fix the suggestion of format for asm_sub_register by lyming2007 · Pull Request #101253 · rust-lang/rust

lyming2007 · 2022-08-31T21:32:10Z

modified:   compiler/rustc_typeck/src/check/intrinsicck.rs
modified:   src/test/ui/asm/bad-template.aarch64_mirunsafeck.stderr
modified:   src/test/ui/asm/bad-template.aarch64_thirunsafeck.stderr
modified:   src/test/ui/asm/bad-template.x86_64_mirunsafeck.stderr
modified:   src/test/ui/asm/bad-template.x86_64_thirunsafeck.stderr
modified:   src/test/ui/asm/type-check-1.rs
modified:   src/test/ui/asm/type-check-1.stderr
modified:   src/test/ui/asm/x86_64/type-check-3.stderr

rust-highfive · 2022-08-31T21:32:13Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @davidtwco (or someone else) soon.

Please see the contribution instructions for more information.

davidtwco · 2022-09-01T09:48:57Z

r? @Amanieu

compiler/rustc_typeck/src/check/intrinsicck.rs

Amanieu · 2022-09-01T09:54:05Z

compiler/rustc_typeck/src/check/intrinsicck.rs

Suggested change

"or use the `0:{default_modifier}` modifier to keep the default formatting of `{default_result}`",

"or use `0:{default_modifier}` to keep the default formatting of `{default_result}`",

lyming2007 · 2022-09-06T15:52:12Z

@Amanieu Could you help me take a look at CI?
It always complains no such file:

The job x86_64-gnu-llvm-13 failed! Check out the build log: [(web)](https://github.com/rust-lang/rust/runs/8209338744?check_suite_focus=true) [(plain)](https://github.com/rust-lang/rust/commit/aa662c2ee0ab30df02114622ee3f5e9508814c9e/checks/8209338744/logs)

Click to see the possible cause of the failure (guessed by this bot)
---- [ui] src/test/ui/asm/[type-check-1.rs](http://type-check-1.rs/) stdout ----
diff of stderr:

96    |
97    = note: only integers, floats, SIMD vectors, pointers and function pointers can be used as arguments for inline assembly
- warning: formatting may not be suitable for sub-register argument
-   --> $DIR/type-check-1.rs:67:27
-    |
-    |
- LL |         asm!( "movd xmm0, {0}", in(reg) val, out("xmm0") _,);
-    |                           ^^^           --- for this argument
-    |
-    = note: `#[warn(asm_sub_register)]` on by default
-    = help: use `0:e` modifier to have the register formatted as `eax`
-    = help: or use `0:r` modifier to keep the default formatting of `rax`

But I have checked in the type-check-1.stderr:

 99 warning: formatting may not be suitable for sub-register argument
100   --> $DIR/type-check-1.rs:67:27
101    |
102 LL |         asm!( "movd xmm0, {0}", in(reg) val, out("xmm0") _,);
103    |                           ^^^           --- for this argument
104    |
105    = note: `#[warn(asm_sub_register)]` on by default
106    = help: use `0:e` modifier to have the register formatted as `eax`
107    = help: or use `0:r` modifier to keep the default formatting of `rax`

Amanieu · 2022-09-07T06:29:41Z

Try putting it in type-check-3.rs instead of type-check-1.rs.

Amanieu · 2022-09-07T06:30:39Z

Actually we already have checks for this lint in type-check-3.rs, you don't need to add a new case.

Amanieu · 2022-09-07T13:16:33Z

The stderr file for aarch64 also needs to be updated, otherwise CI will fail.

./x.py test src/test/ui --test-args asm/aarch64/type-check-3.rs --target aarch64-unknown-linux-gnu --bless

lyming2007 · 2022-09-07T13:28:46Z

The stderr file for aarch64 also needs to be updated, otherwise CI will fail.
./x.py test src/test/ui --test-args asm/aarch64/type-check-3.rs --target aarch64-unknown-linux-gnu --bless

Already did. Now CI passed. Thank you for your suggestions.

Amanieu · 2022-09-07T13:30:48Z

No, the file still has the old error messages: https://github.com/lyming2007/rust/blob/issue-101163/src/test/ui/asm/aarch64/type-check-3.stderr#L62

You need to run this command (note the --target aarch64-unknown-linux-gnu):

./x.py test src/test/ui --test-args asm/aarch64/type-check-3.rs --target aarch64-unknown-linux-gnu --bless

lyming2007 · 2022-09-07T13:48:40Z

No, the file still has the old error messages: https://github.com/lyming2007/rust/blob/issue-101163/src/test/ui/asm/aarch64/type-check-3.stderr#L62

You need to run this command (note the --target aarch64-unknown-linux-gnu):
./x.py test src/test/ui --test-args asm/aarch64/type-check-3.rs --target aarch64-unknown-linux-gnu --bless

done

Amanieu

I suggest some minor changes to make the message more readable.

Amanieu · 2022-09-08T03:04:20Z

compiler/rustc_typeck/src/check/intrinsicck.rs

Suggested change

"use `{idx}:{suggested_modifier}` modifier to have the register formatted as `{suggested_result}`",

"use `{{{idx}:{suggested_modifier}}}` to have the register formatted as `{suggested_result}`",

Amanieu · 2022-09-08T03:04:34Z

compiler/rustc_typeck/src/check/intrinsicck.rs

Suggested change

"or use `{idx}:{default_modifier}` modifier to keep the default formatting of `{default_result}`",

"or use `{{{idx}:{default_modifier}}}` to keep the default formatting of `{default_result}`",

Sounds good.
Fix was pushed

modified: compiler/rustc_typeck/src/check/intrinsicck.rs modified: src/test/ui/asm/bad-template.aarch64_mirunsafeck.stderr modified: src/test/ui/asm/bad-template.aarch64_thirunsafeck.stderr modified: src/test/ui/asm/bad-template.x86_64_mirunsafeck.stderr modified: src/test/ui/asm/bad-template.x86_64_thirunsafeck.stderr modified: src/test/ui/asm/type-check-1.rs modified: src/test/ui/asm/type-check-1.stderr modified: src/test/ui/asm/x86_64/type-check-3.stderr

Amanieu · 2022-09-08T11:16:10Z

@bors r+

bors · 2022-09-08T11:16:12Z

📌 Commit 28c62d2 has been approved by Amanieu

It is now in the queue for this repository.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#99207 (Enable eager checks for memory sanitizer) - rust-lang#101253 (fix the suggestion of format for asm_sub_register) - rust-lang#101450 (Add `const_extern_fn` to 1.62 release notes.) - rust-lang#101556 (Tweak future opaque ty pretty printing) - rust-lang#101563 (Link UEFI target documentation from target list) - rust-lang#101593 (Cleanup themes (tooltip)) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned davidtwco Aug 31, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 31, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 31, 2022

This comment has been minimized.

Sign in to view

rust-highfive assigned Amanieu and unassigned davidtwco Sep 1, 2022

Amanieu reviewed Sep 1, 2022

View reviewed changes

lyming2007 force-pushed the issue-101163 branch from 9129ac4 to aa662c2 Compare September 6, 2022 14:32

This comment has been minimized.

Sign in to view

lyming2007 force-pushed the issue-101163 branch from aa662c2 to b912436 Compare September 6, 2022 15:49

This comment has been minimized.

Sign in to view

lyming2007 force-pushed the issue-101163 branch from b912436 to ac117ae Compare September 7, 2022 12:31

lyming2007 force-pushed the issue-101163 branch from ac117ae to c129601 Compare September 7, 2022 13:41

Amanieu reviewed Sep 8, 2022

View reviewed changes

lyming2007 force-pushed the issue-101163 branch from c129601 to 28c62d2 Compare September 8, 2022 11:14

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 8, 2022

matthiaskrgr mentioned this pull request Sep 9, 2022

Rollup of 6 pull requests #101603

Merged

bors merged commit 0354eee into rust-lang:master Sep 9, 2022

rustbot added this to the 1.65.0 milestone Sep 9, 2022

	"or use the `0:{default_modifier}` modifier to keep the default formatting of `{default_result}`",
	"or use `0:{default_modifier}` to keep the default formatting of `{default_result}`",

	"use `{idx}:{suggested_modifier}` modifier to have the register formatted as `{suggested_result}`",
	"use `{{{idx}:{suggested_modifier}}}` to have the register formatted as `{suggested_result}`",

	"or use `{idx}:{default_modifier}` modifier to keep the default formatting of `{default_result}`",
	"or use `{{{idx}:{default_modifier}}}` to keep the default formatting of `{default_result}`",

Uh oh!

Conversation

lyming2007 commented Aug 31, 2022

Uh oh!

rust-highfive commented Aug 31, 2022

Uh oh!

This comment has been minimized.

davidtwco commented Sep 1, 2022

Uh oh!

Uh oh!

Amanieu Sep 1, 2022

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

lyming2007 commented Sep 6, 2022

Uh oh!

This comment has been minimized.

Amanieu commented Sep 7, 2022

Uh oh!

Amanieu commented Sep 7, 2022

Uh oh!

Amanieu commented Sep 7, 2022

Uh oh!

lyming2007 commented Sep 7, 2022

Uh oh!

Amanieu commented Sep 7, 2022

Uh oh!

lyming2007 commented Sep 7, 2022

Uh oh!

Amanieu left a comment

Choose a reason for hiding this comment

Uh oh!

Amanieu Sep 8, 2022

Choose a reason for hiding this comment

Uh oh!

Amanieu Sep 8, 2022

Choose a reason for hiding this comment

Uh oh!

lyming2007 Sep 8, 2022

Choose a reason for hiding this comment

Uh oh!

Amanieu commented Sep 8, 2022

Uh oh!

bors commented Sep 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants