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

Fail early when a top level statement fails to resolve #1202

Conversation

DuncanUszkay1
Copy link
Contributor

Issue: #1165

There's more context in the issue itself. Basically we could fail to compile a global top level statement, and we would just keep going compiling the next statement. This caused an assertion failure.

This PR makes the compileTopLevelStatement return a boolean to indicate that it has failed to compile, which is then used to halt compilation early and show an error message. The advantage of this approach is that we don't run into assertion failures later on, and instead show an error that is descriptive to the user.

Example

const INNER: fake = 1;
const OUTER: i32 = INNER;

On master we get:

ERROR: AssertionError: assertion failed
    ...stacktrace

With this branch we get:

ERROR TS2304: Cannot find name 'fake'.

 const INNER: fake = 1;
              ~~~~
 in main.ts(5,14)

ERROR: 1 compile error(s)
    at Object.main (/Users/duncanuszkay/src/github.com/Shopify/as-fork/assemblyscript/cli/asc.js:614:21)
    at /Users/duncanuszkay/src/github.com/Shopify/as-fork/assemblyscript/bin/asc:21:47

@dcodeIO
Copy link
Member

dcodeIO commented Mar 31, 2020

Suggesting to fix the assertion at the place it originates, since otherwise we can't provide proper error highlighting (of all errors to fix in a file) in IDEs. The red markers would just stop at some point.

@DuncanUszkay1
Copy link
Contributor Author

Ah Right, I wasn't thinking about the IDE case. I'll try a different approach then

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

Successfully merging this pull request may close these issues.

2 participants