Skip to content

Conversation

@llvmbot
Copy link
Member

@llvmbot llvmbot commented Nov 8, 2025

Backport 3673cc7

Requested by: @mstorsjo

@llvmbot
Copy link
Member Author

llvmbot commented Nov 8, 2025

@aganea What do you think about merging this PR to the release branch?

@dyung
Copy link
Collaborator

dyung commented Nov 11, 2025

Hi, unfortunately the criteria for fixes to be included in the release branch is that they must be patches for either regressions or major issues, neither of which seems to match this issue. If you think it does fall under one of these categories please let us know and we will reconsider.

@dyung dyung moved this from Needs Triage to Won't Merge in LLVM Release Status Nov 11, 2025
@dyung dyung moved this from Won't Merge to Needs Triage in LLVM Release Status Nov 11, 2025
@mstorsjo
Copy link
Member

Hi, unfortunately the criteria for fixes to be included in the release branch is that they must be patches for either regressions or major issues, neither of which seems to match this issue. If you think it does fall under one of these categories please let us know and we will reconsider.

Yes, this is neither a recent regression or a major issue in itself - but it is a very tightly scoped bugfix, with a clear case of matching the reference (rc.exe), and with very little regression risk; therefore I suggested backporting it.

… mode (llvm#166915)

It turns out that rc.exe doesn't interpret integer literals as octal
numbers - but GNU windres does. Previously, llvm-rc did interpret them
as octal.

Fix the issue by stripping away the leading zeros during tokenization.
The alternative (which would be somewhat cleaner, as visible in
tokenizer.test) would be to retain them in the RCToken object, but strip
them out before calling
StringRef::getAsInteger. Alternatively to handle the radix detection
locally in llvm-rc code and not rely on getAsInteger to autodetect it.
Both of those solutions require propagating the IsWindres flag so that
it is available within RCToken, or at least when calling
RCToken::intValue().

Fixes: llvm#144723
(cherry picked from commit 3673cc7)
@dyung dyung moved this from Needs Triage to Won't Merge in LLVM Release Status Nov 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Won't Merge

Development

Successfully merging this pull request may close these issues.

4 participants