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

lowering: Don't mutate lambda in linearize #57416

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

mlechu
Copy link
Contributor

@mlechu mlechu commented Feb 14, 2025

Fixes #56904.

The associated PR (#55876) compiles a finally block, then compiles a renumbered version of it. This works if compile doesn't mutate its input, but in reality, lambda bodies were being set! when linearized. The "invalid syntax" error was a result of attempting to linearize a lambda twice.

@nsajko nsajko added compiler:lowering Syntax lowering (compiler front end, 2nd stage) bugfix This change fixes an existing bug backport 1.12 Change should be backported to release-1.12 labels Feb 14, 2025
@Keno Keno requested a review from JeffBezanson February 14, 2025 22:52
This was referenced Feb 15, 2025
Copy link
Member

@topolarity topolarity left a comment

Choose a reason for hiding this comment

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

Nice work - Thanks for fixing my bug 🙂

Let's give @JeffBezanson a day to review, and merge this if there are no objections

@nsajko
Copy link
Contributor

nsajko commented Feb 19, 2025

There are conflicts now.

@mlechu mlechu force-pushed the fix-finally-lowering branch from e677990 to cc397d8 Compare February 19, 2025 17:35
@topolarity
Copy link
Member

BTW do you know if this also happens to fix #57153?

That was bisected to the same commit, but may be a separate issue

@mlechu
Copy link
Contributor Author

mlechu commented Feb 19, 2025

BTW do you know if this also happens to fix #57153?

That was bisected to the same commit, but may be a separate issue

Tested, looks like a separate issue.

@oscardssmith oscardssmith added the merge me PR is reviewed. Merge when all tests are passing label Feb 19, 2025
@giordano giordano merged commit 414aca2 into JuliaLang:master Feb 20, 2025
8 checks passed
@giordano giordano removed the merge me PR is reviewed. Merge when all tests are passing label Feb 20, 2025
KristofferC pushed a commit that referenced this pull request Feb 21, 2025
Fixes #56904.

The associated PR (#55876) compiles a finally block, then compiles a
renumbered version of it. This works if `compile` doesn't mutate its
input, but in reality, lambda bodies were being `set!` when linearized.
The "invalid syntax" error was a result of attempting to linearize a
lambda twice.

(cherry picked from commit 414aca2)
KristofferC added a commit that referenced this pull request Feb 26, 2025
Backported PRs:
- [x] #57302 <!-- Add explicit imports for types and fix bugs -->
- [x] #57420 <!-- Compiler: Fix check for IRShow definedness -->
- [x] #57419 <!-- generated: Switch resolution module back to what it
was before -->
- [x] #57421 <!-- bpart: Skip implicit import reval if using'd export
set is unchanged -->
- [x] #57425 <!-- Prohibit binding replacement in closed modules during
precompile -->
- [x] #57426 <!-- Prohibit `import`ing or `using` Main during
incremental compilation -->
- [x] #57433 <!-- bpart: Track whether any binding replacement has
happened in image modules -->
- [x] #57445 <!-- Run all `--sysimage-native-code=no` cmdlineargs tests
single-threaded -->
- [x] #57386 <!-- Only strip invariant.load from special pointers -->
- [x] #57453 <!-- Revert "Make emitted egal code more loopy (#54121)"
-->
- [x] #57389 <!-- Change memory indexing to use the type as index
instead of i8 -->
- [x] #57447 <!-- Don't return null pointer when asking for the type of
a declared global -->
- [x] #57467 <!-- using/import: ensure world update after each
observable operation -->
- [x] #57471 <!-- staticdata: Don't use `newm` pointer after it has been
invalidated -->
- [x] #57416 <!-- lowering: Don't mutate lambda in `linearize` -->
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug compiler:lowering Syntax lowering (compiler front end, 2nd stage)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lowering regression on nightly causing spurious syntax errors
7 participants