Skip to content

Remember failed globals so we don't hit assertions #1321

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

Merged
merged 2 commits into from
Jun 11, 2020
Merged

Conversation

dcodeIO
Copy link
Member

@dcodeIO dcodeIO commented Jun 9, 2020

An alternative to fix #1205 by remembering globals that failed to compile so we properly get a false upon subsequent compileGlobal invocations and don't run into asserts in the first place, as per my latest comment on the above PR.

@DuncanUszkay1: Can you confirm that this solves the issue as well?

If so, also fixes #1165

Now also: fixes #1186, fixes #1190

  • I've read the contributing guidelines

@dcodeIO dcodeIO changed the title Remember failed globals Remember failed globals so we don't hit assertions Jun 9, 2020
@DuncanUszkay1
Copy link
Contributor

LGTM, seems to fix the issue I had

@dcodeIO
Copy link
Member Author

dcodeIO commented Jun 11, 2020

Last commit is an attempt to also fix cases like

let a = (a = 4, 3)

on top of this, by reusing the set of pending globals in compilation to check for this condition. I'm not particularly proud of the dummy local that must be added for this, but it fixes the problem so I guess it's fine.

@dcodeIO dcodeIO requested a review from MaxGraey June 11, 2020 02:20
@dcodeIO dcodeIO merged commit 5ac5841 into master Jun 11, 2020
@dcodeIO
Copy link
Member Author

dcodeIO commented Jun 11, 2020

Merging to get it out of the way, but might not catch all cases yet. Let me know if there's something missing, can add it in a follow-up.

@dcodeIO dcodeIO deleted the issue-1205 branch June 11, 2020 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion failed when modifying a variable from within init Unresolved const type causing assertion failure
2 participants