Skip to content
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

global_asm expands arguments in comments #85944

Closed
gz opened this issue Jun 2, 2021 · 11 comments
Closed

global_asm expands arguments in comments #85944

gz opened this issue Jun 2, 2021 · 11 comments
Labels
A-inline-assembly Area: inline asm!(..) C-discussion Category: Discussion or questions that doesn't represent real issues. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@gz
Copy link
Contributor

gz commented Jun 2, 2021

I tried this code:

#![feature(global_asm)]
global_asm!{r#"
// ${num}
/* ${num} */
"#}

I expected to see this happen: Compiles without error (on nightly)

Instead, this happened (not sure if this is intended behavior or a bug):

error: there is no argument named `num`
 --> src/lib.rs:3:5
  |
3 | // ${num}
  |     ^^^^^

error: there is no argument named `num`
 --> src/lib.rs:4:5
  |
4 | /* ${num} */
  |     ^^^^^
@gz gz added the C-bug Category: This is a bug. label Jun 2, 2021
@klensy
Copy link
Contributor

klensy commented Jun 2, 2021

Until raw string closed, all content interpreted as string, so that not a comments inside, but raw string. Or i misunderstand something.

@gz
Copy link
Contributor Author

gz commented Jun 2, 2021

I ran into this because my global_asm!{} typically includes a .S file (with include_str!("file.S")), now that .S file is documented with comments and the comments also contained a ${num} string. This used to be fine with nightly-2021-03-25 or earlier but now throws this error after I upgraded to nightly-2021-06-01 -- so the behavior changed somewhere between these versions.

@klensy
Copy link
Contributor

klensy commented Jun 2, 2021

Perhaps #84107, try may-11 nightly.

@gz
Copy link
Contributor Author

gz commented Jun 2, 2021

May 11th nightly works (does not produce the error)

rustc --version
rustc 1.45.0-nightly (9912925c2 2020-05-10)

@klensy
Copy link
Contributor

klensy commented Jun 2, 2021

That pr get merged May 14, 2021, 1:17 AM GMT+3, so you can try closest nightly.

rustc 1.45.0-nightly (9912925 2020-05-10)

2021-05-11, not 2020 )

@gz
Copy link
Contributor Author

gz commented Jun 2, 2021

Ups -- sorry about that, I guess I'm still stuck in 2020!

works:

rustc --version
rustc 1.54.0-nightly (6d395a1c2 2021-05-13)

does not work:

rustc --version
rustc 1.54.0-nightly (1025db84a 2021-05-14)

@klensy
Copy link
Contributor

klensy commented Jun 2, 2021

$ git log 6d395a1..1025db8 --author=bors --oneline
1025db8 Auto merge of #85211 - Aaron1011:metadata-invalid-span, r=michaelwoerister
75da570 Auto merge of #83640 - bjorn3:shared_metadata_reader, r=nagisa
36a4d14 Auto merge of #85236 - nikic:update-llvm-submodule, r=cuviper
69b352e Auto merge of #85233 - FabianWolff:issue-85227, r=petrochenkov
91f2e2d Auto merge of #85190 - mati865:update-cc, r=Mark-Simulacrum
754d171 Auto merge of #85195 - Mark-Simulacrum:variant-by-idx, r=petrochenkov
17f30e5 Auto merge of #84107 - Amanieu:global_asm2, r=nagisa

So i guess i picked that one pr @Amanieu

@Amanieu
Copy link
Member

Amanieu commented Jun 3, 2021

#84107 changed global_asm! to accept arguments just like asm!. However this means that the string is now parsed the same way as a format string, which means {}, {1} and {name} are treated as placeholders for asm arguments.

The current workaround is to escape { and } using {{ and }} just like with format strings. One possibility would be to add options(raw) which would force treating the asm as a raw string instead of as a format string.

@gz
Copy link
Contributor Author

gz commented Jun 3, 2021

Thanks the explanation. This isn't a problem for me in practice and looks like it's intentional after all, feel free to close this.

@ChrisDenton ChrisDenton added the needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. label Jul 16, 2023
@fmease fmease added A-inline-assembly Area: inline asm!(..) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-discussion Category: Discussion or questions that doesn't represent real issues. and removed C-bug Category: This is a bug. needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. labels Jan 25, 2024
@fmease
Copy link
Member

fmease commented Jan 25, 2024

Closing as works-as-intended. However, I can reopen this issue if you'd like to pursue a potential options(raw) (alternatively you can open a new issue for that for the sake of clarity):

One possibility would be to add options(raw) which would force treating the asm as a raw string instead of as a format string.

@fmease fmease closed this as not planned Won't fix, can't repro, duplicate, stale Jan 25, 2024
@Amanieu
Copy link
Member

Amanieu commented Jan 26, 2024

options(raw) already exists.

@Amanieu Amanieu closed this as completed Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inline-assembly Area: inline asm!(..) C-discussion Category: Discussion or questions that doesn't represent real issues. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants