Skip to content

correctly construct _ as Ident rather than Punctuation#1202

Merged
kkysen merged 1 commit intoimmunant:masterfrom
chrysn-pull-requests:more-punctuation
Feb 24, 2025
Merged

correctly construct _ as Ident rather than Punctuation#1202
kkysen merged 1 commit intoimmunant:masterfrom
chrysn-pull-requests:more-punctuation

Conversation

@chrysn
Copy link
Copy Markdown
Contributor

@chrysn chrysn commented Feb 21, 2025

This fixes another bug like #1197: _ is no puctuation -- another case of dtolnay/proc-macro2#475, fall-out of dtolnay/proc-macro2#470.

Testing

This enables building RIOT sources again without --locked. I'm not entirely sure, but I think the offending assembly was

  __ASM ("rev %0, %1" : __CMSIS_GCC_OUT_REG (result) : __CMSIS_GCC_USE_REG (value) );

which gets turned into

    asm!(
        "rev {0}, {1}", lateout(reg) result, inlateout(reg) value => _,
        options(preserves_flags, pure, readonly)
    );

where the _ is the identifier (formerly punctuation) which c2rust trips over.

@chrysn
Copy link
Copy Markdown
Contributor Author

chrysn commented Feb 21, 2025

Note that I did not put much thought into the span that is put in. Looks natural to use this one, could also have used span for less precision, but given this is all just used to generate a file (and not directly compiler input), I'm not sure it matters at all what is in there -- at least I wouldn't know how to trigger anything where it makes a difference and thus can be tested what the span in there means.

Copy link
Copy Markdown
Contributor

@fw-immunant fw-immunant left a comment

Choose a reason for hiding this comment

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

LGTM. I don't think getting the span perfectly minimal is a big deal here.

@kkysen kkysen merged commit 50d1380 into immunant:master Feb 24, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants