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 span when evaluating mir::ConstOperand #122471

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

RalfJung
Copy link
Member

This lets us show to the user where they were using the faulty const (which can be quite relevant when generics are involved).

I wonder if we should change "erroneous constant encountered" to something like "the above error was encountered while evaluating this constant" or so, to make this more similar to what the collector emits when showing a "backtrace" of where things get monomorphized? It seems a bit strange to rely on the order of emitted diagnostics for that but it seems the collector already does that.

@rustbot
Copy link
Collaborator

rustbot commented Mar 14, 2024

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 14, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 14, 2024

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@RalfJung
Copy link
Member Author

r? @oli-obk

@rustbot rustbot assigned oli-obk and unassigned Nadrieril Mar 14, 2024
--> $DIR/issue-50814-2.rs:20:5
|
LL | &<A<T> as Foo<T>>::BAR
| ^^^^^^^^^^^^^^^^^^^^^^
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be further improved by taking into account that promoteds are defined where they are used... but the emit_note logic doesn't know whether it is decorating a promoted. I'd leave that to a future PR.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 14, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 14, 2024

📌 Commit da8a8c9 has been approved by oli-obk

It is now in the queue for this repository.

@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 Mar 14, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 14, 2024
preserve span when evaluating mir::ConstOperand

This lets us show to the user where they were using the faulty const (which can be quite relevant when generics are involved).

I wonder if we should change "erroneous constant encountered" to something like "the above error was encountered while evaluating this constant" or so, to make this more similar to what the collector emits when showing a "backtrace" of where things get monomorphized? It seems a bit strange to rely on the order of emitted diagnostics for that but it seems the collector already [does that](https://github.com/rust-lang/rust/blob/da8a8c9223722e17cc0173ce9490076b4a6d263d/compiler/rustc_monomorphize/src/collector.rs#L472-L475).
@RalfJung
Copy link
Member Author

@oli-obk what do you think about this?

I wonder if we should change "erroneous constant encountered" to something like "the above error was encountered while evaluating this constant" or so, to make this more similar to what the collector emits when showing a "backtrace" of where things get monomorphized? It seems a bit strange to rely on the order of emitted diagnostics for that but it seems the collector already does that.

@rust-log-analyzer

This comment has been minimized.

@RalfJung
Copy link
Member Author

RalfJung commented Mar 14, 2024 via email

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 14, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Mar 14, 2024

I wonder if we should change "erroneous constant encountered" to something like "the above error was encountered while evaluating this constant"

I'm not sure that is reliable unless we use the collector scheme of counting global errors that were reported. The constant may have gotten evaluated much earlier with lots of other errors in between

@RalfJung
Copy link
Member Author

Hm, yeah okay that makes sense. Let's land as-is then, I'll open an issue to track further possible improvements.

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Mar 14, 2024

📌 Commit f24f055 has been approved by oli-obk

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 14, 2024
@RalfJung
Copy link
Member Author

Filed #122486 for further possible improvements.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 14, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 14, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#119029 (Avoid closing invalid handles)
 - rust-lang#122238 (Document some builtin impls in the next solver)
 - rust-lang#122247 (rustdoc-search: depth limit `T<U>` -> `U` unboxing)
 - rust-lang#122287 (add test ensuring simd codegen checks don't run when a static assertion failed)
 - rust-lang#122368 (chore: remove repetitive words)
 - rust-lang#122397 (Various cleanups around the const eval query providers)
 - rust-lang#122406 (Fix WF for `AsyncFnKindHelper` in new trait solver)
 - rust-lang#122477 (Change some attribute to only_local)
 - rust-lang#122482 (Ungate the `UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES` lint)
 - rust-lang#122490 (Update build instructions for OpenHarmony)

Failed merges:

 - rust-lang#122471 (preserve span when evaluating mir::ConstOperand)

r? `@ghost`
`@rustbot` modify labels: rollup
@RalfJung
Copy link
Member Author

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Mar 14, 2024

📌 Commit 2fdc37f has been approved by oli-obk

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 14, 2024
@rust-log-analyzer

This comment has been minimized.

@RalfJung
Copy link
Member Author

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 14, 2024
@RalfJung
Copy link
Member Author

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Mar 14, 2024

📌 Commit 48f2f0d has been approved by oli-obk

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 14, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 15, 2024
preserve span when evaluating mir::ConstOperand

This lets us show to the user where they were using the faulty const (which can be quite relevant when generics are involved).

I wonder if we should change "erroneous constant encountered" to something like "the above error was encountered while evaluating this constant" or so, to make this more similar to what the collector emits when showing a "backtrace" of where things get monomorphized? It seems a bit strange to rely on the order of emitted diagnostics for that but it seems the collector already [does that](https://github.com/rust-lang/rust/blob/da8a8c9223722e17cc0173ce9490076b4a6d263d/compiler/rustc_monomorphize/src/collector.rs#L472-L475).
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 15, 2024
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#121885 (Move generic `NonZero` `rustc_layout_scalar_valid_range_start` attribute to inner type.)
 - rust-lang#122471 (preserve span when evaluating mir::ConstOperand)
 - rust-lang#122515 (Pass the correct DefId when suggesting writing the aliased Self type out)
 - rust-lang#122523 (Ensure RPITITs are created before def-id freezing)
 - rust-lang#122527 (Clean up AstConv)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 15, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#121207 (Add `-Z external-clangrt`)
 - rust-lang#122174 (diagnostics: suggest `Clone` bounds when noop `clone()`)
 - rust-lang#122471 (preserve span when evaluating mir::ConstOperand)
 - rust-lang#122515 (Pass the correct DefId when suggesting writing the aliased Self type out)
 - rust-lang#122523 (Ensure RPITITs are created before def-id freezing)
 - rust-lang#122526 (Docs for `thir::ExprKind::Use` and `thir::ExprKind::Let`)
 - rust-lang#122527 (Clean up AstConv)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f4afbe1 into rust-lang:master Mar 15, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 15, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 15, 2024
Rollup merge of rust-lang#122471 - RalfJung:const-eval-span, r=oli-obk

preserve span when evaluating mir::ConstOperand

This lets us show to the user where they were using the faulty const (which can be quite relevant when generics are involved).

I wonder if we should change "erroneous constant encountered" to something like "the above error was encountered while evaluating this constant" or so, to make this more similar to what the collector emits when showing a "backtrace" of where things get monomorphized? It seems a bit strange to rely on the order of emitted diagnostics for that but it seems the collector already [does that](https://github.com/rust-lang/rust/blob/da8a8c9223722e17cc0173ce9490076b4a6d263d/compiler/rustc_monomorphize/src/collector.rs#L472-L475).
@RalfJung RalfJung deleted the const-eval-span branch March 15, 2024 20:30
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 18, 2024
…RalfJung

Avoid various uses of `Option<Span>` in favor of using `DUMMY_SP` in the few cases that used `None`

based on rust-lang#122471

`DUMMY_SP` is already the sentinel value we have that says "no span". We don't need to wrap these `Span`s in a separate `Option`.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 18, 2024
…RalfJung

Avoid various uses of `Option<Span>` in favor of using `DUMMY_SP` in the few cases that used `None`

based on rust-lang#122471

`DUMMY_SP` is already the sentinel value we have that says "no span". We don't need to wrap these `Span`s in a separate `Option`.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 18, 2024
Rollup merge of rust-lang#122480 - oli-obk:const-eval-span-no-opt, r=RalfJung

Avoid various uses of `Option<Span>` in favor of using `DUMMY_SP` in the few cases that used `None`

based on rust-lang#122471

`DUMMY_SP` is already the sentinel value we have that says "no span". We don't need to wrap these `Span`s in a separate `Option`.
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.

6 participants