Attributes

[wojpok]

So far attributes may precede any let statement and may consist of multiple lists of identifiers separated by comma.
I am not really sure what to do with them at the moment, thus all attributes are dropped past de-sugar stage.

[dominik-muc]

Do we only want attributes with let? I can see advantage of annotating a module or data-type.

However, this depends on how good do we want to implement it for now, thus what is really needed for test library (or maybe we can use it somewhere in lexer/parser generators?). It can be changed later on to support other constructions.

Also it might be a good idea to be consistent with list definitions, so maybe consider this syntax:

@{attr1}

@{[attr1, attr2]}

# Or to make things easier to read just list them one after another:

@{attr1}
@{attr2}
let _ = ...

Another question is whether we want attributes to take extra arguments. Should they behave like functions?

# This syntax might be useful for passing additional information to the compiler
@{attr3 "some param" 42}
let _ = ...

[wojpok]

I have added attributes resolving to functions as discussed. As of now they are implemented over Surface Language, not Raw Language and honestly I am not really sure if that is the right way. Anyway, tests are properly toggleable from CLI

[wojpok]

Since last meeting I have added new features to this pull request:

• Tests can be tagged and selected via command line using globs as discussed
• abstr attribute have been implemented
• Record are desugarred to ADT with tag record. Attribute resolver later adds missing record selectors
• Removed visibility from Raw language
• Improved attributes error handling

There is still work to be done, specifically test 89 fails

[ppolesiuk]

I like this direction. Making Desugar smaller always sounds like a good idea. While the most of the work is already done in this PR, it would be nice to take care of some details. See comments below.

[ppolesiuk]

This PR adds dependencies to the interpreter. First, we should ask if it is really needed (I prefer to keep set of dependencies as small as possible). If so, we should update the README.md file.

[Copilot]

Pull Request Overview

This PR introduces an attributes feature for the Fram language, allowing attributes to precede let statements and other definitions. Attributes are specified using the @{...} syntax and can contain multiple comma-separated identifiers with optional arguments. The implementation includes attribute validation, conflict detection, and topological sorting to ensure correct processing order. Additionally, the PR adds command-line options for test filtering using glob patterns.

Key Changes

• Implemented attribute syntax and parsing with @{...} notation
• Added attribute resolution system with conflict detection and topological sorting
• Introduced test filtering functionality via -test and -test-tags command-line options

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
test/ok/ok0139_attributes.fram	Test file demonstrating attribute syntax with various examples including empty attributes, pub, and test tags
src/dune	Added dune-glob library dependency for glob pattern matching
src/dbl.ml	Added -test and -test-tags command-line options for test filtering
src/DblParser/YaccParser.mly	Updated parser to support attribute syntax, modified visibility handling to use attributes internally
src/DblParser/Raw.ml	Added attribute type definition and updated def type to include attribute lists
src/DblParser/Lexer.mll	Added lexer support for @{ token as ATTR_OPEN
src/DblParser/Error.mli	Added error message signatures for attribute-related errors
src/DblParser/Error.ml	Implemented error messages for attribute conflicts, uniqueness violations, and arity mismatches
src/DblParser/Desugar.ml	Refactored definition translation to handle attributes, removed visibility parameters from pattern translation
src/DblParser/Attributes.mli	Interface for attribute resolution and visibility pattern creation
src/DblParser/Attributes.ml	Core attribute resolution logic including validators, resolvers, and topological sorting
src/DblConfig.ml	Added test filtering configuration with glob pattern matching
dune-project	Added dune-glob package dependency with version constraint
README.md	Updated documentation to mention dune-glob requirement
.github/workflows/Test.yml	Added dune-glob to CI installation dependencies

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The error message on line 194 indicates that the #record attribute expects 0 arguments, but the pattern matching on line 182 accepts ["ignoreExistential"] as a valid argument list. This inconsistency suggests that when ignoreExistential is used, the attribute actually expects 1 argument, not 0. The error message should account for this case or the validation logic should be updated.

Suggested change

	(Error.attribute_argument_arity_mismatch args.pos 0 (List.length xs))
	(Error.attribute_argument_arity_mismatch args.pos
	~expected:"0 or 1 (\"ignoreExistential\")"
	~actual:(string_of_int (List.length xs)))
	~expected:"0 or 1 (\"ignoreExistential\")"
	~actual:(string_of_int (List.length xs)))

[wojpok]

This error is not reachable by design. To compensate for that, I changed the error type to attribute_internal_error

[ppolesiuk]

The convention is that errors defined in Error module are programmer's faults, while internal errors are reported by InterpLib.InternalError.report. If the code is really unreachable, it should fall into the latter category.

[ppolesiuk]

Btw, maybe a small comment explaining why this code is unreachable would be helpful. For instance "This code should not be reachable: the function is designed to translate internal #record attribute, for which the user cannot provide own arguments."