Skip to content

Conversation

mlechu
Copy link
Member

@mlechu mlechu commented Jul 30, 2025

Fix #59128

Assignment desugaring of (const (= (|::| x T) rhs)) would pre-expand to, then re-expand (const x ,(convert-for-type-decl rhs T)), but two-arg (IR) const is expected to have a simple RHS---closure conversion doesn't recurse here (should it?), giving us partially-lowered IR, and hence our bug.

Fix: Pre-expand to the one-arg AST const form (const (= x ,(convert-for-type-decl rhs T))) instead. This also gives us a (latestworld) we were missing before, so this lowering may have been originally intended.

@mlechu mlechu requested a review from xal-0 July 30, 2025 15:37
@mlechu mlechu added compiler:lowering Syntax lowering (compiler front end, 2nd stage) bugfix This change fixes an existing bug labels Jul 30, 2025
@giordano giordano added the backport 1.12 Change should be backported to release-1.12 label Jul 30, 2025
@xal-0
Copy link
Member

xal-0 commented Jul 30, 2025

I am excited to revive #58187 if only to remove the lowered const/surface const confusion...

@Keno Keno merged commit 3de5b9a into JuliaLang:master Aug 1, 2025
5 of 7 checks passed
KristofferC pushed a commit that referenced this pull request Aug 6, 2025
Fix #59128

Assignment desugaring of `(const (= (|::| x T) rhs))` would pre-expand
to, then re-expand `(const x ,(convert-for-type-decl rhs T))`, but
two-arg (IR) const is expected to have a simple RHS---closure conversion
doesn't recurse here (should it?), giving us partially-lowered IR, and
hence our bug.

Fix: Pre-expand to the one-arg AST const form `(const (= x
,(convert-for-type-decl rhs T)))` instead. This also gives us a
`(latestworld)` we were missing before, so this lowering may have been
originally intended.

(cherry picked from commit 3de5b9a)
@KristofferC KristofferC mentioned this pull request Aug 6, 2025
38 tasks
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Aug 19, 2025
KristofferC pushed a commit that referenced this pull request Sep 5, 2025
Fix #59128

Assignment desugaring of `(const (= (|::| x T) rhs))` would pre-expand
to, then re-expand `(const x ,(convert-for-type-decl rhs T))`, but
two-arg (IR) const is expected to have a simple RHS---closure conversion
doesn't recurse here (should it?), giving us partially-lowered IR, and
hence our bug.

Fix: Pre-expand to the one-arg AST const form `(const (= x
,(convert-for-type-decl rhs T)))` instead. This also gives us a
`(latestworld)` we were missing before, so this lowering may have been
originally intended.

(cherry picked from commit 3de5b9a)
KristofferC pushed a commit that referenced this pull request Sep 15, 2025
Fix #59128

Assignment desugaring of `(const (= (|::| x T) rhs))` would pre-expand
to, then re-expand `(const x ,(convert-for-type-decl rhs T))`, but
two-arg (IR) const is expected to have a simple RHS---closure conversion
doesn't recurse here (should it?), giving us partially-lowered IR, and
hence our bug.

Fix: Pre-expand to the one-arg AST const form `(const (= x
,(convert-for-type-decl rhs T)))` instead. This also gives us a
`(latestworld)` we were missing before, so this lowering may have been
originally intended.

(cherry picked from commit 3de5b9a)
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.

"Invalid declaration" inside a constant definition when loading a module

5 participants