8000 Fix #4098: Avoid `let`/`const` when building the ASTs for GCC. by sjrd · Pull Request #4110 · scala-js/scala-js · GitHub
[go: up one dir, main page]

Skip to content

Fix #4098: Avoid let/const when building the ASTs for GCC. #4110

New issue

Have a ques 8000 tion 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 1 commit into from
Jul 1, 2020

Conversation

sjrd
Copy link
Member
@sjrd sjrd commented Jul 1, 2020

We now use vars instead, which do not expose the issue.

This is a workaround for an unknown root cause. We should properly fix this in the future.

We now use `var`s instead, which do not expose the issue.

This is a workaround for an unknown root cause. We should properly
fix this in the future.
@sjrd sjrd requested a review from gzm0 July 1, 2020 07:39
val set = new java.util.HashSet[String]()
set.remove("")
set.remove("1") // only if remove is called twice
assertEquals(0, set.size())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this fail in the test suite without the fix? (i.e. does it fail not in isolation?).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it does. I had checked ;)

@gzm0 gzm0 merged commit fba4b7b into scala-js:master Jul 1, 2020
@sjrd sjrd deleted the fix-gcc-crash branch July 1, 2020 09:57
@tbje
Copy link
tbje commented Jul 1, 2020

Just tested v1.1.1 in the initial project and fullOptJS now completes successfully without any workaround 🎉 Thanks for the quick resolution!

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.

3 participants
0