Skip to content

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
Collaborator

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
Collaborator

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
Collaborator

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
Collaborator

bors commented Nov 28, 2021

⌛ Testing commit 4cfb7ad with merge c8b3a2be048338a8aff2389252c33c9e01493415...

@bors
Copy link
Collaborator

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.

10 participants