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

Fix a format_args span to be expansion #90131

Merged
merged 1 commit into from
Nov 29, 2021

Conversation

camsteffen
Copy link
Contributor

I found this while exploring solutions for rust-lang/rust-clippy#7843.

r? @m-ou-se

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 21, 2021
@camsteffen
Copy link
Contributor Author

Hmm the change to diagnostics is probably not acceptable.

It would be nice to detect the scenario when the rvalue behind the & is a macro input, and then use the span of the rvalue. But it looks like that span is not available in the MIR during borrowck. We have RValue::Ref with a Place that points the the binding, not the binding reference.

@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 28, 2021
@camsteffen
Copy link
Contributor Author

Using Span::with_ctxt as a compromise.

@rust-log-analyzer

This comment has been minimized.

Comment on lines +24 to +25
|
= note: this error originates in the macro `$crate::format_args_nl` (in Nightly builds, run with -Z macro-backtrace for more info)
Copy link
Member

Choose a reason for hiding this comment

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

This new output seems non-ideal, but there's probably not a good way to hide it. I guess it could potentially be useful if/when -Z macro-backtrace is stabilized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not great but there are other instances like this.

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 16, 2021
@camsteffen
Copy link
Contributor Author

@m-ou-se can I bother you for a review? The wrong span context is causing surprising behavior in Clippy.

@m-ou-se
Copy link
Member

m-ou-se commented Nov 23, 2021

Oh, sorry for the delay.

I don't think I'm the right person to review this though.

Maybe someone from the compiler team?

r? rust-lang/compiler

@rust-highfive rust-highfive assigned cjgillot and unassigned m-ou-se Nov 23, 2021
@cjgillot
Copy link
Contributor

Since the AddrOf node is created by the macro, the macro context indeed seems like the right one.
@bors r+

@bors
Copy link
Contributor

bors commented Nov 27, 2021

📌 Commit 4cfb7ad has been approved by cjgillot

@bors 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 Nov 27, 2021
@bors
Copy link
Contributor

bors commented Nov 27, 2021

⌛ Testing commit 4cfb7ad with merge 8d6f1889b234af193a3700ec9b8f2064bcd81f63...

@rust-log-analyzer
Copy link
Collaborator

The job i686-msvc-1 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
* highest error code: E0786
Found 502 error codes
Found 0 error codes with no tests
Done!
thread '<unnamed>' panicked at 'cmd.exec() failed with Error during execution of `cargo metadata`:     Updating git repository `https://github.com/bjorn3/rust-ar.git`
warning: spurious network error (2 tries remaining): request failed with status code: 504; class=Http (34)
warning: spurious network error (1 tries remaining): request failed with status code: 504; class=Http (34)
error: failed to get `ar` as a dependency of package `rustc_codegen_cranelift v0.1.0 (D:\a\rust\rust\compiler\rustc_codegen_cranelift)`
Caused by:
  failed to load source for dependency `ar`

Caused by:
Caused by:
  Unable to update https://github.com/bjorn3/rust-ar.git?branch=do_not_remove_cg_clif_ranlib#de9ab0e5

Caused by:
  failed to clone into: C:\Users\runneradmin\.cargo\git\db\rust-ar-9b35aff8ad678e06
Caused by:
  network failure seems to have happened
  network failure seems to have happened
  if a proxy or similar is necessary `net.git-fetch-with-cli` may help here
  https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli
Caused by:
Caused by:
  request failed with status code: 504; class=Http (34)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Any { .. }', src\tools\tidy\src/main.rs:77:9


command did not execute successfully: "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0-tools-bin\\rust-tidy.exe" "D:\\a\\rust\\rust" "\\\\?\\D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\bin\\cargo.exe" "D:\\a\\rust\\rust\\build" "8"


Build completed unsuccessfully in 0:03:55
Build completed unsuccessfully in 0:03:55
make: *** [Makefile:72: ci-subset-1] Error 1

@bors
Copy link
Contributor

bors commented Nov 27, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 27, 2021
@camelid
Copy link
Member

camelid commented Nov 27, 2021

@bors retry

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 27, 2021
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 27, 2021
@bors
Copy link
Contributor

bors commented Nov 28, 2021

⌛ Testing commit 4cfb7ad with merge c8b3a2be048338a8aff2389252c33c9e01493415...

@bors
Copy link
Contributor

bors commented Nov 28, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 28, 2021
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@cjgillot
Copy link
Contributor

@bors retry

@bors 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 Nov 28, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 29, 2021
…askrgr

Rollup of 3 pull requests

Successful merges:

 - rust-lang#90131 (Fix a format_args span to be expansion)
 - rust-lang#90832 (Add 1.57.0 release notes)
 - rust-lang#90833 (Emit LLVM optimization remarks when enabled with `-Cremark`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9715724 into rust-lang:master Nov 29, 2021
@rustbot rustbot added this to the 1.59.0 milestone Nov 29, 2021
@camsteffen camsteffen deleted the fmt-args-span-fix branch November 29, 2021 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants