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

Preserve more spans in internal rustc_queries! macro #86123

Merged
merged 1 commit into from Aug 19, 2021

Conversation

Aaron1011
Copy link
Member

We now preserve the span of the various query modifiers, and
use the span of the query's name for the commas that we
generate to separate the modifiers. This makes debugging issues with the
internal query macro infrastructure much nicer - previously, we
would get errors messages pointing at the entire call site
(the rustc_queries! invocation), which isn't very useful.

This should have no effect when compilation succeeds.

A concrete example of an error message produced after this changed:

error: local ambiguity: multiple parsing options: built-in NTs tt ('modifiers') or 1 other option.
    --> /home/aaron/repos/rust/compiler/rustc_middle/src/query/mod.rs:23:11
     |
12   | / rustc_queries! {
13   | |     query trigger_delay_span_bug(key: DefId) -> () {
14   | |         desc { "trigger a delay span bug" }
15   | |     }
...    |
23   | |     query hir_crate(key: ()) -> &'tcx Crate<'tcx> {
     | |           ^^^^^^^^^
...    |
1715 | |     }
1716 | | }
     | |_- in this expansion of `rustc_query_append!`
     |
    ::: compiler/rustc_query_impl/src/lib.rs:51:1
     |
51   |   rustc_query_append! { [define_queries!][<'tcx>] }
     |   ------------------------------------------------- in this macro invocation

The particular bug shown in this error message will be fixed
in a separate PR.

We now preserve the span of the various query modifiers, and
use the span of the query's name for the commas that we
generate to separate the modifiers. This makes debugging issues with the
internal query macro infrastructure much nicer - previously, we
would get errors messages pointing at the entire call site
(the `rustc_queries!` invocation), which isn't very useful.

This should have no effect when compilation succeeds.

A concrete example of an error message produced after this changed:

```
error: local ambiguity: multiple parsing options: built-in NTs tt ('modifiers') or 1 other option.
    --> /home/aaron/repos/rust/compiler/rustc_middle/src/query/mod.rs:23:11
     |
12   | / rustc_queries! {
13   | |     query trigger_delay_span_bug(key: DefId) -> () {
14   | |         desc { "trigger a delay span bug" }
15   | |     }
...    |
23   | |     query hir_crate(key: ()) -> &'tcx Crate<'tcx> {
     | |           ^^^^^^^^^
...    |
1715 | |     }
1716 | | }
     | |_- in this expansion of `rustc_query_append!`
     |
    ::: compiler/rustc_query_impl/src/lib.rs:51:1
     |
51   |   rustc_query_append! { [define_queries!][<'tcx>] }
     |   ------------------------------------------------- in this macro invocation
```

The particular bug shown in this error message will be fixed
in a separate PR.
@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 8, 2021
@Aaron1011
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 8, 2021
@bors
Copy link
Contributor

bors commented Jun 8, 2021

⌛ Trying commit 34f1161 with merge a79bb9d7ed943b0f8fe3fd4c74d833d6ec282050...

@bors
Copy link
Contributor

bors commented Jun 8, 2021

☀️ Try build successful - checks-actions
Build commit: a79bb9d7ed943b0f8fe3fd4c74d833d6ec282050 (a79bb9d7ed943b0f8fe3fd4c74d833d6ec282050)

@rust-timer
Copy link
Collaborator

Queued a79bb9d7ed943b0f8fe3fd4c74d833d6ec282050 with parent a50d721, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (a79bb9d7ed943b0f8fe3fd4c74d833d6ec282050): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 8, 2021
@bjorn3
Copy link
Member

bjorn3 commented Jun 11, 2021

The particular bug shown in this error message will be fixed
in a separate PR.

fb7e483 from #85830 fixed it.

Edit: just saw your a03cf1b from #86119 which fixes it too, but in a different way.

@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 Jun 27, 2021
@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 Jul 12, 2021
@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 Jul 31, 2021
@camelid
Copy link
Member

camelid commented Aug 15, 2021

r? @cjgillot

@cjgillot
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 18, 2021

📌 Commit 34f1161 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 Aug 18, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 19, 2021
…laumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#86123 (Preserve more spans in internal `rustc_queries!` macro)
 - rust-lang#87874 (Add TcpStream type to TcpListener::incoming docs)
 - rust-lang#88034 (rustc_privacy: Replace `HirId`s and `DefId`s with `LocalDefId`s where possible)
 - rust-lang#88050 (Remove `HashStable` impls for `FileName` and `RealFileName`)
 - rust-lang#88093 ([rustdoc] Wrap code blocks in `<code>` tag)
 - rust-lang#88146 (Add tests for some `feature(const_evaluatable_checked)` incr comp issues)
 - rust-lang#88153 (Update .mailmap)
 - rust-lang#88159 (Use a trait instead of the now disallowed missing trait there)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit def6393 into rust-lang:master Aug 19, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 19, 2021
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants