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

Internal Compiler Error when evaluating nonsensical const #5616

Closed
IGI-111 opened this issue Feb 15, 2024 · 2 comments · Fixed by #5631
Closed

Internal Compiler Error when evaluating nonsensical const #5616

IGI-111 opened this issue Feb 15, 2024 · 2 comments · Fixed by #5631
Assignees
Labels
bug Something isn't working compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen

Comments

@IGI-111
Copy link
Contributor

IGI-111 commented Feb 15, 2024

Swapping let for const in places can lead to an ICE among other errors.

The following minimal example:

contract;

abi Abi1 {
    fn foo() -> b256;
}

abi Abi2{
    fn bar() -> u64;
}

pub fn main() {
    let contract_1 = abi(Abi1, b256::min());
    const invalid_const = contract_1.foo();
    let contract_2 = abi(Abi2, invalid_const);
    let invalid = contract_2.bar();
    log(invalid);
}

Produces:

error: Internal compiler error: Unable to resolve variable 'invalid'.
Please file an issue on the repository and include the code that triggered this error.
@IGI-111 IGI-111 added bug Something isn't working compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen labels Feb 15, 2024
@IGI-111 IGI-111 assigned tritao and jjcnn and unassigned tritao Feb 15, 2024
@jjcnn
Copy link
Contributor

jjcnn commented Feb 16, 2024

I 've been looking at this one, and I can't reproduce the ICE neither on master nor any of the tagged branches since 0.48.0.

The example code gives me a warning about how the const variable should be in all caps, but other than that everything compiles just fine.

On the playground I'm getting

There was an unexpected error compiling your contract. Please try again.

This seems unrelated, though, because I'm getting it even if I change const to let.

Can you walk me through the exact steps I need to go through to reproduce the issue?

@IGI-111
Copy link
Contributor Author

IGI-111 commented Feb 16, 2024

Sorry I didn't paste this correctly, it should be a script not a contract.

This should fail correctly when built:

script;

abi Abi1 {
    fn foo() -> b256;
}

abi Abi2{
    fn bar() -> u64;
}

pub fn main() {
    let contract_1 = abi(Abi1, b256::min());
    const invalid_const = contract_1.foo();
    let contract_2 = abi(Abi2, invalid_const);
    let invalid = contract_2.bar();
    log(invalid);
}

IGI-111 pushed a commit that referenced this issue Feb 27, 2024
…#5631)

## Description

Fixes #5616 and #5622.

When generating the IR for constant and variable declarations the name
being declared is not added to the local environment if the initializer
cannot be compiled. This can happen when initializing a constant using a
non-constant expression.

Since the name isn't added to the environment, and since we attempt to
continue compilation after an error, any attempt to use the name later
in the code will cause the name to not be found, which in turn causes
the IR converter to throw an ICE because it assumes that all used names
are in scope.

This PR fixes the issue by not throwing the initializer error until the
name has been added to the environment. Note that
1. The initializer must be compiled before the name is added to the
environment. The name should not be in scope during its own
initialization.
2. #5602 is blocking a more elegant solution to this problem. The lack
of a `Never` type means that the call to `convert_resolved_typeid` may
fail if the initializer diverges, and since that call must be made
before the new name can be added to the environment we check for
termination first. The order of operations is therefore currently a)
compile the initializer, then b) check that the initializer did not
result in an error and did not diverge, then c) resolve the type of the
name being declared, then d) add the name to the environment, and
finally e) throw an error if the initializer resulted in an error. Once
the `Never` type is introduced we can move the terminator check to the
end of that sequence.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants