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][compiler-v2] ability missing warning comes too late #14490

Closed
meng-xu-cs opened this issue Aug 31, 2024 · 1 comment · Fixed by #15006
Closed

[Bug][compiler-v2] ability missing warning comes too late #14490

meng-xu-cs opened this issue Aug 31, 2024 · 1 comment · Fixed by #15006

Comments

@meng-xu-cs
Copy link
Collaborator

🐛 Bug

Compiling the following Move code with V2 compiler

module demo::demo {
    struct S<T: store> has store {
        field: T,
    }

    struct E<T: store> has store, drop {
        entry: S<T>,
    }
}

yields an error at the File Format generation phase:

bug: bytecode verification failed with unexpected status code `FIELD_MISSING_TYPE_ABILITY`. This is a compiler bug, consider reporting it.
  ┌─ /<redacted>/test/sources/test.move:1:1
  │  
1 │ ╭ module demo::demo {
2 │ │     struct S<T: store> has store {
3 │ │         field: T,
4 │ │     }
  · │
8 │ │     }
9 │ │ }
  │ ╰─^

Which is a bit late in the process.

With V1 compiler, the error message is more dev-friendly:

error[E05001]: ability constraint not satisfied
  ┌─ /<redacted>/test/sources/test.move:7:16
  │
2 │     struct S<T: store> has store {
  │            - To satisfy the constraint, the 'drop' ability would need to be added here
  ·
7 │         entry: S<T>,
  │                ^^^^
  │                │
  │                Invalid field type. The struct was declared with the ability 'drop' so all fields require the ability 'drop'
  │                The type '(demo=0x97751FCABB9A35CABA546D45C30E91CEB8B4622386C2DBF81B9B6D64A9F53AEF)::demo::S<_>' does not have the ability 'drop'
@meng-xu-cs meng-xu-cs added the bug Something isn't working label Aug 31, 2024
@meng-xu-cs meng-xu-cs changed the title [Bug][compiler-v2] ability missing warnings comes too late [Bug][compiler-v2] ability missing warning comes too late Aug 31, 2024
@wrwg
Copy link
Contributor

wrwg commented Aug 31, 2024

This is the bytecode verifier as a verification step after code generation, and any error like this is definitely an unintended bug, though safe because the code can't be deployed.

@brmataptos brmataptos moved this from 🆕 New to For Grabs in Move Language and Runtime Sep 5, 2024
wrwg added a commit that referenced this issue Oct 18, 2024
Closes #14490

Move has some unusual semantics for ability checking of type parameters

- In most contexts, a type parameter is assumed to have all abilities during checking. Then when the parameter is instantiated, checking takes place.
- However, there is one exemption, namely when a type parameter is used to instantiate another type parameter which has an abilitie constraint, as in `f<T:drop>`. If we instantiate `f<R>` then `R` needs to be checked for `drop`.

This PR solves this by adding a field `type_param_exempted` to `HasAbilities(abilities, type_param_exempted)` to the ability constraint.
wrwg added a commit that referenced this issue Oct 18, 2024
Closes #14490

Move has some unusual semantics for ability checking of type parameters

- In most contexts, a type parameter is assumed to have all abilities during checking. Then when the parameter is instantiated, checking takes place.
- However, there is one exemption, namely when a type parameter is used to instantiate another type parameter which has an abilitie constraint, as in `f<T:drop>`. If we instantiate `f<R>` then `R` needs to be checked for `drop`.

This PR solves this by adding a field `type_param_exempted` to `HasAbilities(abilities, type_param_exempted)` to the ability constraint.
@wrwg wrwg closed this as completed in 87348fc Oct 18, 2024
@github-project-automation github-project-automation bot moved this from For Grabs to ✅ Done in Move Language and Runtime Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants