Do not (ab)use blocks for top level trees #4909
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CI only for now. While working on #4906, I have realized that we hold on to a lot of Javascript trees in the back end for the mere purpose of using their identity to find the already printed tree in the downstream cache. I want to attempt to print the trees eagerly so we can release that memory. However, for this I need to clean up the block stuff first. My plan is to make Emitter type parametric on the resulting tree so the BasicLinkerBackend can request a |
|
Is our CLA check broken? https://www.lightbend.com/contribute/cla/scala/check/gzm0 returns: {"user":"gzm0","signed":true,"version":"1.0","currentVersion":"1.0"} |
|
Well: |
|
Looks like certificates might be missing in the chain sent from the sever: |
|
I have the USERTrust RSA CA in my cert store (/etc/ssl) but not the Sectigo RSA Domain Validation Secure Server CA. Probably Chrome has that one built in 🤷 |
|
It's on Lightbend's side apparently: scala/scala-dev#855 |
0d49e7d to
83b69fb
Compare
sjrd
approved these changes
Oct 15, 2023
Comment on lines
214
to
216
| } yield ( | ||
| // Real constructor |
There was a problem hiding this comment.
Hum this goes a bit outside the style guide. If the body of a yield spans multiple lines, it should have {}:
Suggested change
| } yield ( | |
| // Real constructor | |
| } yield { | |
| // Real constructor |
There was a problem hiding this comment.
Just that by itself will not compile because of the blank line before // Inheritable Constructor. The :: before the blank line is then interpreted as postfix operator.
Options I see:
- Leave it like this.
- Curly braces without the blank line
- Parens inside the curly braces (either with nesting or on the same line).
I have gone with the last option for now.
There was a problem hiding this comment.
Hum I see. In that case ideally it should be:
} yield {
(
...
...
)
}
linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/ClassEmitter.scala
Outdated
Show resolved
Hide resolved
sjrd
approved these changes
Oct 15, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.