-
Notifications
You must be signed in to change notification settings - Fork 570
[WIP][go1.20] Working on go1.20 native overrides #1283
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
Closed
Closed
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
b5034ee
to
6e6162d
Compare
We haven't been using it since we switched to esbuild for prelude minification.
Since Node 12 it supports the --enable-source-maps flag, which has been considered stable since Node 14. Given we are now on 18, we can drop the unneeded dependency and use the built-in flag.
Remove npm dependencies we don't need anymore
The main change is that we assign explicit names to all function objects that correspond to Go functions (named and literals). Function name is declared as `var f = function nameHere() { ... }` and is visible inside the function scope only. Doing so serves two purposes: - It is an identifier which we can use when saving state of a blocked function to know which function to call upon resumption. - It shows up in the stack trace, which helps distinguish similarly-named functions. For methods, we include the receiver type in the identifier to make A.String and B.String easily distinguishable. The main trick is that we synthesize names for the function literals, which are anonymous as far as go/types is concerned. The upstream Go compiler does something very similar. (based on commit 4d24395)
JS function names are subtly different from what vanilla Go may expect, unless gopherjs#1085 is implemented. It turns out that a combination of d5771cc and 22c65b8 subtly changes how node outputs stack trace in a way that breaks my workarounds in the reflect package. Instead of further fumbling, I am going to disable the offending tests temporarily, and I have a proper fix for gopherjs#1085 in the works, which will allow us to re-enable them along with a few other tests.
Final round of refactoring ported from the original generics branch.
The branch is no longer needed since I'm switching to help with generics which is being added to master. When we're getting closer to go1.20 being finished I may open up a similar branch like this then.... until then... bye! |
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.
WIP - Do Not Merge
🚧 This is a temporary PR that I'm using to work on some of the updates to natives and anything else for go1.20. This branch contains many ideas that may or may not be cut out into it's own PR. This allows for fast iteration whilst getting CI testing and feedback. Once I get a self-contained idea working, I'll then cut a PR containing one commit to add that idea to branch go1.20.