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

[Bug] error out in compilation when encounter an unknown annotation/attribute #8875

Closed
lightmark opened this issue Jun 27, 2023 · 4 comments
Closed
Assignees
Labels
bug Something isn't working enhancement New feature or request stale-exempt Prevents issues from being automatically marked and closed as stale

Comments

@lightmark
Copy link
Contributor

🚀 Feature Request

Compiler ignores unknown annotations but actually it should just error out. Otherwise, typo must be introduced while the author doesn't know. For example -> #8866 for examples.

Motivation

#[testonly] in #8866 for examples.

Pitch

Additional context

@lightmark lightmark added the enhancement New feature or request label Jun 27, 2023
@lightmark lightmark changed the title [Feature Request] error out in compilation when encounter an unknown annotation [Bug] error out in compilation when encounter an unknown annotation Jul 5, 2023
@lightmark lightmark added the bug Something isn't working label Jul 5, 2023
@wrwg wrwg changed the title [Bug] error out in compilation when encounter an unknown annotation [Bug] error out in compilation when encounter an unknown annotation/attribute Jul 5, 2023
@brmataptos brmataptos self-assigned this Jul 6, 2023
@brmataptos
Copy link
Contributor

list of allowed attributes is implicitly in the constants defined here.

Oops, extended_checks.rs also has a few known attributes ("legacy_entry_fun", "view", "group", "scope", "resource_group", "resource_group_member", "group"), with an odd variety of constant names associated with them.

We’re also missing #[deprecated{


More from Slack discussion:

Kevin Hoang
Another thing to note is we need a good error message in case users already use some custom attributes

Wolfgang Grieskamp
The cli needs a flag to skip those checks

Kevin Hoang
We might also consider giving a harsher warning for variants of test_only such as testonly as those are the most risky ones (accidentally releasing test code)

@brmataptos
Copy link
Contributor

Specific Requirements:

  • mechanism to register and check known attribute names from various code
  • register attributes
    • third_party/move/move-compiler/src/shared/mod.rs#L518
    • aptos-move/framework/src/extended_checks.rs#L24
    • #[deprecated]
  • good error message in case users use custom attributes
    • CLI needs a flag to skip those checks
    • maybe a flag to allow registering more attributes
  • also consider harsher warning for variants of test_only such as testonly
  • tests

@brmataptos brmataptos moved this from 🆕 New to 🏗 In progress in Move Language and Runtime Jul 11, 2023
@brmataptos
Copy link
Contributor

Wolfgang is apparently greatly proposed to a flag registering more attributes, so we'll just have a flag to skip checks.

@wrwg
Copy link
Contributor

wrwg commented Jul 15, 2023

Yes, I do think we should leave user registerable attributes to the compiler v2 and language extensions. Specifically, I propose to do attributes similar as in C#/Java, so they can be declared in the language, together with typing of their parameters, as in:

#[define_attribute] public fun my_attribute(param1: u64, param2: String)

Besides this being more general, the flag approach doesn't seem to cover what people expect from compiler command line options. A user would have to every single time specify this options, if he forgets, then compilation fails. Moreover, the allowed parameters of the attributes cannot be easily defined this way.

I assume at this point not many people are out there which actually use there own attributes. Those few can be grandfathered by the the single flag --skip-attribute-checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request stale-exempt Prevents issues from being automatically marked and closed as stale
Projects
Status: Done
Development

No branches or pull requests

4 participants