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

Scala.js trait super call code generation #7962

Merged
merged 1 commit into from Jan 17, 2020

Conversation

@molikto
Copy link
Contributor

molikto commented Jan 11, 2020

In this commit scala.js will not rewrite super calls into scala2x traits to static methods. so <init> trait constructors is not rewritten, but although they are non-static in scala2x, they are actually named $init$.

Alternative fix will be changing encodeMethodSym (I only thought of this alternative afterwards, not sure it works actually), not sure which one is preferred...

Copy link

dotty-bot left a comment

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

@molikto molikto force-pushed the molikto:try-fix-trait-init branch from e950981 to 8fbb824 Jan 11, 2020
Copy link
Member

sjrd left a comment

Thanks for the PR, and sorry for the delay. Here are a few comments. The overall idea looks sound.

compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala Outdated Show resolved Hide resolved
compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala Outdated Show resolved Hide resolved
compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala Outdated Show resolved Hide resolved
compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala Outdated Show resolved Hide resolved
@molikto

This comment has been minimized.

Copy link
Contributor Author

molikto commented Jan 15, 2020

@sjrd Hello! After getting more understanding of the compiler code, I got an alternative fix, which is smaller code change, although it adds one more bit of code path difference between JVM and js

@molikto molikto requested a review from sjrd Jan 15, 2020
Copy link
Member

sjrd left a comment

Thanks. The new fix looks good.

Could you address the two whitespace-related comments below, then squash everything together in one commit, please? Given that the strategy has been completely replaced, it makes no sense to keep in the intermediate commits.

project/Build.scala Outdated Show resolved Hide resolved
compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala Outdated Show resolved Hide resolved
@molikto molikto force-pushed the molikto:try-fix-trait-init branch from 0f91417 to 43b9d12 Jan 17, 2020
@molikto molikto requested a review from sjrd Jan 17, 2020
@sjrd
sjrd approved these changes Jan 17, 2020
Copy link
Member

sjrd left a comment

Thanks!

@sjrd sjrd merged commit 1cff48f into lampepfl:master Jan 17, 2020
2 checks passed
2 checks passed
CLA User signed CLA
Details
continuous-integration/drone/pr Build is passing
Details
@molikto molikto deleted the molikto:try-fix-trait-init branch Jan 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.