-
Notifications
You must be signed in to change notification settings - Fork 355
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
automatic loop variables in for loops ("loop var"), consistent with go 1.22 behavior #1645
Conversation
…1 for reasons I can't yet figure out.
…t was going infinite. must apply to all for7 cases. also updated tests to skip: now doing everything in 1.22 and skipping closures for 1.21
note that this one is ready to go now. it has the mysterious error on the "main / checks code and generated code" test which doesn't replicate locally, and the test doesn't report any specific issues to fix, so I'm not sure how to proceed on that. |
In the logs from CI, the tests fail with the following: === RUN TestExportClosureArg
interp_issue_1634_test.go:57:
Got: "0\n0\n0\n",
want: "0\n1\n2\n"
--- FAIL: TestExportClosureArg (0.00s) I have not investigated further. |
it appears that the closure PR interacts with this one, wrt the definition of shadow local variables: for i := 0; i < 3; i++ {
i := i // eliminate this or use a variable with a different name here and it works
tmp.Func(&fs, func() { println(i) })
} will investiage. |
actually it looks like all of the for loop closure tests used a new variable name (e.g., |
…re 1.22 fix for loopvar) with the same name as the original. all the other tests were using a new variable name, so we missed this case.
The test case It's possible to blacklist the test (see |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this contribution. This is an amazing piece of work to keep the interpreter up-to-date with the latest evolutions of the Go language.
Well done!
🚫 the milestone is missing |
Thanks and thank you so much for writing this amazing system! Incidentally I had written something similar to this in overall structure, for C++ (which I called "C Super Script" or CSS, just before the internet started...). It is so much nicer to be working in Go, and the ability to bidirectionally interoperate with compiled code is so powerful. |
This builds on #1644 and adds automatic per-loop variables that are consistent with go 1.22 behavior. See #1643 for discussion.
This is still a draft because the for7 version ends up capturing the per-loop var values when they are +1 relative to what they should be. Maybe somehow the incrementing and conditional code is somehow capturing the within loop variables and incrementing them? not sure how that would work. anyway, need to investigate further before this is ready to go, but pushing it here in case there are other issues or someone might figure out this bug before I do..