Skip to content

Add formatting support for the asm! macro #6526

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

Open
folkertdev opened this issue Mar 29, 2025 · 4 comments
Open

Add formatting support for the asm! macro #6526

folkertdev opened this issue Mar 29, 2025 · 4 comments
Labels
blocked Blocked on rustc, an RFC, etc. feature-request

Comments

@folkertdev
Copy link
Contributor

Given a snippet like this

unsafe fn foobar() {
    core::arch::asm!("{}", 
        const X)
}

I'd expect that to format to

unsafe fn foobar() {
    core::arch::asm!("{}", const X)
}

but instead nothing happens. Given that these operands are quite new, I suspect this is an oversight.

I'd happily look into fixing this, but I'm not 100% clear on whether the formatting behavior for existing code can be change? Though here I'd clearly consider it a bug.

@ytmimi
Copy link
Contributor

ytmimi commented Mar 29, 2025

rustfmt has never had dedicated support for the asm! macro. I put something together a while ago (#5191), but that open PR is fairly out of date and I'm sure there's a lot of room for improvement.

It's still an open ended question on how the asm! macro should be formatted rust-lang/style-team#152 (at the very least no one has added the rules to the style guide).

@folkertdev
Copy link
Contributor Author

Looks like no progress for the last 3 years then? And there is no real process that would make sure a decision is eventually made?

Also, while maybe it's not been decided exactly what the formatting should look like, it is more functional for certain syntax components that are older, e.g. this

unsafe fn foobar() {
    core::arch::asm!("{}", 
        options(noreturn))
}

does format to the expected

unsafe fn foobar() {
    core::arch::asm!("{}", options(noreturn))
}

It would be nice if sym, const and now label (stable on nightly) would at least do that rather than freezing formatting altogether.

@ytmimi
Copy link
Contributor

ytmimi commented Mar 29, 2025

Also, while maybe it's not been decided exactly what the formatting should look like, it is more functional for certain syntax components that are older, e.g. this

unsafe fn foobar() {
    core::arch::asm!("{}", 
        options(noreturn))
}

does format to the expected

unsafe fn foobar() {
    core::arch::asm!("{}", options(noreturn))
}

It would be nice if sym, const and now label (stable on nightly) would at least do that rather than freezing formatting altogether.

That's because options(noreturn) looks like a function call. When it comes to macro calls, rustfmt can only format them if it can parse all of it's input tokens as valid rust syntax. sym <path>, const <expr>, and label <block> don't parse as valid rust and prevent the macro from being formatted.

There's no way around that until we add dedicated support for the asm! macro.

@ytmimi ytmimi changed the title asm! operands const and sym are not formatted Add formatting support for the asm! macro Mar 29, 2025
@ytmimi ytmimi added feature-request blocked Blocked on rustc, an RFC, etc. labels Mar 29, 2025
@ytmimi
Copy link
Contributor

ytmimi commented Mar 29, 2025

Marking this as blocked on rust-lang/style-team#152

fmease added a commit to fmease/rust that referenced this issue May 18, 2025
…manieu,traviscross

split `asm!` parsing and validation

This PR splits `asm!` parsing and validation into two separate steps.

The parser constructs a `Vec<RawAsmArg>`, with each element corresponding to an argument to one of the `asm!` macros.
The validation then checks things like ordering of arguments or that options are not provided twice.

The motivation is rust-lang#140279, which wants to add `#[cfg(...)]` support to these arguments. This support can now be added in a straightforward way by adding an `attributes: ast::AttrVec` field to `RawAsmArg`.

An extra reason for this split is that `rustfmt` probably wants to format the assembly at some point (currently that appears to be stubbed out, and the formatting is unstable rust-lang/style-team#152).

r? `@ghost` (just want to look at CI for now)

cc `@ytmimi` we discussed asm formatting a little while ago in rust-lang/rustfmt#6526. Am I correct in assuming that `AsmArgs` does not give enough information for formatting, but that `RawAsmArgs` would (it e.g. does not join information from multiple lines). This must have been an issue before?
fmease added a commit to fmease/rust that referenced this issue May 18, 2025
…manieu,traviscross

split `asm!` parsing and validation

This PR splits `asm!` parsing and validation into two separate steps.

The parser constructs a `Vec<RawAsmArg>`, with each element corresponding to an argument to one of the `asm!` macros.
The validation then checks things like ordering of arguments or that options are not provided twice.

The motivation is rust-lang#140279, which wants to add `#[cfg(...)]` support to these arguments. This support can now be added in a straightforward way by adding an `attributes: ast::AttrVec` field to `RawAsmArg`.

An extra reason for this split is that `rustfmt` probably wants to format the assembly at some point (currently that appears to be stubbed out, and the formatting is unstable rust-lang/style-team#152).

r? ``@ghost`` (just want to look at CI for now)

cc ``@ytmimi`` we discussed asm formatting a little while ago in rust-lang/rustfmt#6526. Am I correct in assuming that `AsmArgs` does not give enough information for formatting, but that `RawAsmArgs` would (it e.g. does not join information from multiple lines). This must have been an issue before?
bors added a commit to rust-lang-ci/rust that referenced this issue May 18, 2025
split `asm!` parsing and validation

This PR splits `asm!` parsing and validation into two separate steps.

The parser constructs a `Vec<RawAsmArg>`, with each element corresponding to an argument to one of the `asm!` macros.
The validation then checks things like ordering of arguments or that options are not provided twice.

The motivation is rust-lang#140279, which wants to add `#[cfg(...)]` support to these arguments. This support can now be added in a straightforward way by adding an `attributes: ast::AttrVec` field to `RawAsmArg`.

An extra reason for this split is that `rustfmt` probably wants to format the assembly at some point (currently that appears to be stubbed out, and the formatting is unstable rust-lang/style-team#152).

r? `@ghost` (just want to look at CI for now)

cc `@ytmimi` we discussed asm formatting a little while ago in rust-lang/rustfmt#6526. Am I correct in assuming that `AsmArgs` does not give enough information for formatting, but that `RawAsmArgs` would (it e.g. does not join information from multiple lines). This must have been an issue before?

try-job: aarch64-apple
fmease added a commit to fmease/rust that referenced this issue May 18, 2025
…manieu,traviscross

split `asm!` parsing and validation

This PR splits `asm!` parsing and validation into two separate steps.

The parser constructs a `Vec<RawAsmArg>`, with each element corresponding to an argument to one of the `asm!` macros.
The validation then checks things like ordering of arguments or that options are not provided twice.

The motivation is rust-lang#140279, which wants to add `#[cfg(...)]` support to these arguments. This support can now be added in a straightforward way by adding an `attributes: ast::AttrVec` field to `RawAsmArg`.

An extra reason for this split is that `rustfmt` probably wants to format the assembly at some point (currently that appears to be stubbed out, and the formatting is unstable rust-lang/style-team#152).

r? `@ghost` (just want to look at CI for now)

cc `@ytmimi` we discussed asm formatting a little while ago in rust-lang/rustfmt#6526. Am I correct in assuming that `AsmArgs` does not give enough information for formatting, but that `RawAsmArgs` would (it e.g. does not join information from multiple lines). This must have been an issue before?

try-job: aarch64-apple
fmease added a commit to fmease/rust that referenced this issue May 18, 2025
…manieu,traviscross

split `asm!` parsing and validation

This PR splits `asm!` parsing and validation into two separate steps.

The parser constructs a `Vec<RawAsmArg>`, with each element corresponding to an argument to one of the `asm!` macros.
The validation then checks things like ordering of arguments or that options are not provided twice.

The motivation is rust-lang#140279, which wants to add `#[cfg(...)]` support to these arguments. This support can now be added in a straightforward way by adding an `attributes: ast::AttrVec` field to `RawAsmArg`.

An extra reason for this split is that `rustfmt` probably wants to format the assembly at some point (currently that appears to be stubbed out, and the formatting is unstable rust-lang/style-team#152).

r? ``@ghost`` (just want to look at CI for now)

cc ``@ytmimi`` we discussed asm formatting a little while ago in rust-lang/rustfmt#6526. Am I correct in assuming that `AsmArgs` does not give enough information for formatting, but that `RawAsmArgs` would (it e.g. does not join information from multiple lines). This must have been an issue before?

try-job: aarch64-apple
rust-timer added a commit to rust-lang-ci/rust that referenced this issue May 18, 2025
Rollup merge of rust-lang#140490 - folkertdev:asm-parser-changes, r=Amanieu,traviscross

split `asm!` parsing and validation

This PR splits `asm!` parsing and validation into two separate steps.

The parser constructs a `Vec<RawAsmArg>`, with each element corresponding to an argument to one of the `asm!` macros.
The validation then checks things like ordering of arguments or that options are not provided twice.

The motivation is rust-lang#140279, which wants to add `#[cfg(...)]` support to these arguments. This support can now be added in a straightforward way by adding an `attributes: ast::AttrVec` field to `RawAsmArg`.

An extra reason for this split is that `rustfmt` probably wants to format the assembly at some point (currently that appears to be stubbed out, and the formatting is unstable rust-lang/style-team#152).

r? ``@ghost`` (just want to look at CI for now)

cc ``@ytmimi`` we discussed asm formatting a little while ago in rust-lang/rustfmt#6526. Am I correct in assuming that `AsmArgs` does not give enough information for formatting, but that `RawAsmArgs` would (it e.g. does not join information from multiple lines). This must have been an issue before?

try-job: aarch64-apple
github-actions bot pushed a commit to rust-lang/miri that referenced this issue May 19, 2025
…aviscross

split `asm!` parsing and validation

This PR splits `asm!` parsing and validation into two separate steps.

The parser constructs a `Vec<RawAsmArg>`, with each element corresponding to an argument to one of the `asm!` macros.
The validation then checks things like ordering of arguments or that options are not provided twice.

The motivation is rust-lang/rust#140279, which wants to add `#[cfg(...)]` support to these arguments. This support can now be added in a straightforward way by adding an `attributes: ast::AttrVec` field to `RawAsmArg`.

An extra reason for this split is that `rustfmt` probably wants to format the assembly at some point (currently that appears to be stubbed out, and the formatting is unstable rust-lang/style-team#152).

r? ``@ghost`` (just want to look at CI for now)

cc ``@ytmimi`` we discussed asm formatting a little while ago in rust-lang/rustfmt#6526. Am I correct in assuming that `AsmArgs` does not give enough information for formatting, but that `RawAsmArgs` would (it e.g. does not join information from multiple lines). This must have been an issue before?

try-job: aarch64-apple
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Blocked on rustc, an RFC, etc. feature-request
Projects
None yet
Development

No branches or pull requests

2 participants