Skip to content

[JuliaLowering] Restrict K"VERSION" to module arg#60255

Merged
topolarity merged 2 commits into
JuliaLang:masterfrom
mlechu:jl-version-conversion
Dec 1, 2025
Merged

[JuliaLowering] Restrict K"VERSION" to module arg#60255
topolarity merged 2 commits into
JuliaLang:masterfrom
mlechu:jl-version-conversion

Conversation

@mlechu

@mlechu mlechu commented Nov 26, 2025

Copy link
Copy Markdown
Member

Fixes stdlib precompilation after #60018: the v_str macro produces a
VersionNumber, which becomes K"VERSION", which is unhandled syntax. I
think the intent was to only give it special treatment as the child of a
module expression (not make VersionNumber special syntax).

Fixes stdlib precompilation after JuliaLang#60018: the `v_str` macro produces a
     `VersionNumber`, which becomes `K"VERSION"`, which is unhandled syntax.  I
     think the intent was to only give it special treatment as the child of a
     module expression (not make VersionNumber special syntax).
@mlechu
mlechu requested a review from Keno November 26, 2025 19:55
@mlechu mlechu added the compiler:lowering Syntax lowering (compiler front end, 2nd stage) label Nov 26, 2025
Comment thread JuliaLowering/src/compat.jl
@Keno

Keno commented Nov 26, 2025

Copy link
Copy Markdown
Member

Yes, I did not intend a generic VersionNumber <-> K"VERSION" embedding. However, I don't think this PR works as is. First there are two places, this shows up, one is in the module, the other is in @VERSION. The second issue as inline, I don't think the runtime likes this. Can we do this conversion in the appropriate head for the :module or :macrocall heads?

@mlechu

mlechu commented Nov 26, 2025

Copy link
Copy Markdown
Member Author

Instead of introducing K"VERSION" in converting the macrocall, would it make sense to just handle it in macro expansion (symmetric to your change in jl_invoke_julia_macro)?

@Keno

Keno commented Nov 26, 2025

Copy link
Copy Markdown
Member

That sounds reasonable, but I don't know JuliaLowering well enough to really think it through.

@mlechu

mlechu commented Nov 26, 2025

Copy link
Copy Markdown
Member Author

I'll give it a shot. What is the reason for having the K"VERSION" node instead of just a K"Value" anyway? Is it a request for JuliaLowering to fill in its version info?

@Keno

Keno commented Nov 26, 2025

Copy link
Copy Markdown
Member

There's no place for the parser to store a VersionNumber, so it needs to be encoded into the flags. I would be fine it was a K"Value" in lowering.

@mlechu

mlechu commented Nov 26, 2025

Copy link
Copy Markdown
Member Author

(update after trying it:) Using K"Value" in module would be simpler, but would require differences between SyntaxNode and SyntaxTree (I do want this, but that starts with #60162).

The macrocall should work now, though I'm not sure how to reason about it. Meta.parse("@VERSION") and Expr(JS.parse("@VERSION")) produce different answers, and in general I think we're treating the SyntaxNode->Expr conversion as an assumed post-processing step, where JuliaLowering's input doesn't necessarily go through this step.

@mlechu
mlechu requested a review from Keno December 1, 2025 17:20

@topolarity topolarity left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is important to get stdlibs green again on JuliaLowering, so I'm merging this as-is, but follow-up feedback is welcome

@topolarity
topolarity merged commit 68553c5 into JuliaLang:master Dec 1, 2025
8 checks passed
topolarity pushed a commit that referenced this pull request Dec 1, 2025
- `ccall` with a `lib,sym` tuple argument was being desugared to a call
to `Core.Tuple` when we actually want an `Expr(:tuple)`

- `cfunction` only worked in simple cases, since the check for local
variables in scope resolution will fail with any nontrivial function
body. I know that using the new `static_eval` head could be considered a
bugfix (see discussion at
JuliaLang/JuliaLowering.jl#36), but this PR just
uses `inert` to match Base.

stdlib status: 38/51 (with #60255)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compiler:lowering Syntax lowering (compiler front end, 2nd stage)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants