Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move attribute checks to type checking phase #6987

Open
ironcev opened this issue Mar 4, 2025 · 1 comment
Open

Move attribute checks to type checking phase #6987

ironcev opened this issue Mar 4, 2025 · 1 comment

Comments

@ironcev
Copy link
Member

ironcev commented Mar 4, 2025

#6986 defines a single place to check attribute's properties, in the convert_parse_tree. This means that modules with attribute issues will now consistently not reach the type checking phase. This was previously the case for some of the issues with attributes where custom checks where done during the type checking phase.

Checking attribute properties can be moved after the tree conversion phase, except the checks for #[cfg]. The benefit is the continuation of compilation until the end of module.

ironcev added a commit that referenced this issue Mar 23, 2025
## Description

This PR reimplements how `#[attribute]`s are parsed, defined, and
checked. It fixes the following issues we had with attributes:
- attributes were in general not checked for their expected and allowed
usage.
- when checked, checks were fixing particular reported issues and were
scattered through various compilation phases: lexing, parsing, and tree
conversion.
- when checked, reported errors in some cases had bugs and were
overalapping with other errors and warnings.
- attribute handling was not centralized, but rahter scattered throught
the whole code base, and mostly done at use sites.

For concrete examples of these issues, see the list of closed issues
below.

The PR:
- encapsulates the attributes handling within a single abstraction:
`Attributes`.
- assigns the following properties to every attribute:
  - _target_: what kind of elements an attribute can annotate.
- _multiplicity_: if more then one attribute of the same kind can be
applied to an element.
- _arguments multiplicity_: a range defining how many arguments an
attribute can or must have.
- _arguments value expectation_: if attribute arguments are expected to
have values.
- validates those properties in a single place, during the
`convert_parse_tree`. Note that this means that modules with attribute
issues will now consistently not reach the type checking phase. This was
previously the case for some of the issues with attributes where custom
checks where done during the type checking phase. #6987 proposes to move
those checks after the tree conversion phase, allowing the continuation
of compilation.
- adds the `AttributeKind::Unknow` to preserve unknown attributes in the
typed tree.
- removes redundant code related to attributes: specific warnings and
errors, specialized parsing, repetitive and error-prone at use site
checks, etc.
- removes the unused `Doc` attribute.

The PR also introduces a new pattern in emitting errors. The
`attr_decls_to_attributes` function will always return `Attributes` even
if they have errors, because:
- we expect the compilation to continue even in the case of errors.
- we expect those errors to be ignored if a valid `#[cfg]` attribute
among those evaluates to false.
- we expect them to be emitted with eventual other errors, or alone, at
the end of the tree conversion of the annotated element.

For more details, see the comment on `attr_decls_to_attributes` and
`cfg_eval`.

Closes #6880; closes #6913; closes #6914; closes #6915; closes #6916;
closes #6917; closes #6918; closes #6931; closes #6981; closes #6983;
closes #6984; closes #6985.

## Breaking changes

Strictly seen, this PR can cause breaking changes, but only in the case
of invalid existing attribute usage. We treat those breaking changes as
bug fixes in the compiler and, thus, do not have them behind a feature
flag.

E.g., this kind of code was possible before, but will now emit and
error:
```sway
#[storage(read, write, read, write)]
struct Struct {}
```

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
@Oluebube01
Copy link

May I pick this up?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants