Add `deeply_normalize_for_diagnostics`, use it in coherence #118346

compiler-errors · 2023-11-27T03:24:48Z

r? lcnr

Normalize trait refs used for coherence error reporting with -Ztrait-solver=next-coherence.

Two things:

I said before that we can't add this to TyErrCtxt because we compute OverlapResults even if there are no diagnostics being emitted, e.g. for a reservation impl.
I didn't want to add this to an InferCtxtExt trait because I felt it was unnecessary. I don't particularly care about the API though.

rustbot · 2023-11-27T03:25:00Z

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

lcnr · 2023-11-27T11:56:48Z

compiler/rustc_trait_selection/src/solve/normalize.rs

+    param_env: ty::ParamEnv<'tcx>,
+    t: T,
+) -> T {
+    infcx


can you add an assert that this is only used with the new solver or alternatively, make it a noop if it isn't?

I think we already assert if deep_normalize is called outside of the new solver, but I'll copy that assertion here to really make it clear 😃

lcnr · 2023-11-27T11:59:19Z

compiler/rustc_trait_selection/src/solve/normalize.rs

+) -> T {
+    infcx
+        .commit_if_ok(|_| {
+            deeply_normalize(infcx.at(&ObligationCause::dummy(), param_env), t.clone())


this would keep <() as CanNormalize>::Assoc: Trait<<() as Ambig>::Assoc> fully unnormalized. Maybe easiest to write a TypeFolder here where we call deeply_normalize and if that returns an error, super_fold_with?

Yeah that's a good point -- forgot that the deep normalizer uses select_all_or_error.

estebank · 2023-11-28T18:14:27Z

compiler/rustc_trait_selection/src/traits/coherence.rs

@@ -1084,6 +1090,10 @@ impl<'a, 'tcx> ProofTreeVisitor<'tcx> for AmbiguityCausesVisitor<'a, 'tcx> {
                        Ok(Ok(())) => warn!("expected an unknowable trait ref: {trait_ref:?}"),
                        Ok(Err(conflict)) => {
                            if !trait_ref.references_error() {
+                                // Noramlize the trait ref for diagnostics, ignoring any errors if this fails.


Suggested change

// Noramlize the trait ref for diagnostics, ignoring any errors if this fails.

// Normalize the trait ref for diagnostics, ignoring any errors if this fails.

lcnr · 2023-11-28T21:35:53Z

@rustbot author

compiler-errors · 2023-12-05T16:38:22Z

@rustbot ready

lcnr

r=me after nit

compiler/rustc_trait_selection/src/solve/normalize.rs

compiler-errors · 2023-12-05T16:57:56Z

@bors r=lcnr rollup

bors · 2023-12-05T16:57:59Z

📌 Commit 3448284 has been approved by lcnr

It is now in the queue for this repository.

…or-diagnostic, r=lcnr Add `deeply_normalize_for_diagnostics`, use it in coherence r? lcnr Normalize trait refs used for coherence error reporting with `-Ztrait-solver=next-coherence`. Two things: 1. I said before that we can't add this to `TyErrCtxt` because we compute `OverlapResult`s even if there are no diagnostics being emitted, e.g. for a reservation impl. 2. I didn't want to add this to an `InferCtxtExt` trait because I felt it was unnecessary. I don't particularly care about the API though.

…mpiler-errors Rollup of 9 pull requests Successful merges: - rust-lang#117793 (Update variable name to fix `unused_variables` warning) - rust-lang#118123 (Add support for making lib features internal) - rust-lang#118268 (Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always) - rust-lang#118346 (Add `deeply_normalize_for_diagnostics`, use it in coherence) - rust-lang#118350 (Simplify Default for tuples) - rust-lang#118450 (Use OnceCell in cell module documentation) - rust-lang#118585 (Fix parser ICE when recovering `dyn`/`impl` after `for<...>`) - rust-lang#118587 (Cleanup error handlers some more) - rust-lang#118642 (bootstrap(builder.rs): Don't explicitly warn against `semicolon_in_expressions_from_macros`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#118346 - compiler-errors:deeply-normalize-for-diagnostic, r=lcnr Add `deeply_normalize_for_diagnostics`, use it in coherence r? lcnr Normalize trait refs used for coherence error reporting with `-Ztrait-solver=next-coherence`. Two things: 1. I said before that we can't add this to `TyErrCtxt` because we compute `OverlapResult`s even if there are no diagnostics being emitted, e.g. for a reservation impl. 2. I didn't want to add this to an `InferCtxtExt` trait because I felt it was unnecessary. I don't particularly care about the API though.

rustbot assigned lcnr Nov 27, 2023

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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative labels Nov 27, 2023

lcnr reviewed Nov 27, 2023

View reviewed changes

estebank reviewed Nov 28, 2023

View reviewed changes

rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 28, 2023

compiler-errors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 28, 2023

rust-cloud-vms bot force-pushed the deeply-normalize-for-diagnostic branch from a7b3d59 to 01c4e8e Compare November 28, 2023 21:48

This comment has been minimized.

Sign in to view

Add deeply_normalize_for_diagnostics, use it in coherence

334577f

rust-cloud-vms bot force-pushed the deeply-normalize-for-diagnostic branch from 01c4e8e to 8ad9466 Compare December 5, 2023 16:38

lcnr reviewed Dec 5, 2023

View reviewed changes

compiler/rustc_trait_selection/src/solve/normalize.rs Outdated Show resolved Hide resolved

Continue folding if deep normalizer fails

3448284

rust-cloud-vms bot force-pushed the deeply-normalize-for-diagnostic branch from 8ad9466 to 3448284 Compare December 5, 2023 16:55

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 Dec 5, 2023

matthiaskrgr mentioned this pull request Dec 5, 2023

Rollup of 7 pull requests #118651

Closed

compiler-errors mentioned this pull request Dec 5, 2023

Rollup of 8 pull requests #118652

Closed

compiler-errors mentioned this pull request Dec 5, 2023

Rollup of 8 pull requests #118653

Closed

compiler-errors mentioned this pull request Dec 5, 2023

Rollup of 9 pull requests #118655

Merged

bors merged commit 598ca0e into rust-lang:master Dec 6, 2023
11 checks passed

rustbot added this to the 1.76.0 milestone Dec 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `deeply_normalize_for_diagnostics`, use it in coherence #118346

Add `deeply_normalize_for_diagnostics`, use it in coherence #118346

compiler-errors commented Nov 27, 2023

rustbot commented Nov 27, 2023

lcnr Nov 27, 2023

compiler-errors Nov 27, 2023

lcnr Nov 27, 2023

compiler-errors Nov 27, 2023

estebank Nov 28, 2023

lcnr commented Nov 28, 2023

This comment has been minimized.

compiler-errors commented Dec 5, 2023

lcnr left a comment

compiler-errors commented Dec 5, 2023

bors commented Dec 5, 2023

	// Noramlize the trait ref for diagnostics, ignoring any errors if this fails.
	// Normalize the trait ref for diagnostics, ignoring any errors if this fails.

Add deeply_normalize_for_diagnostics, use it in coherence #118346

Add deeply_normalize_for_diagnostics, use it in coherence #118346

Conversation

compiler-errors commented Nov 27, 2023

rustbot commented Nov 27, 2023

lcnr Nov 27, 2023

Choose a reason for hiding this comment

compiler-errors Nov 27, 2023

Choose a reason for hiding this comment

lcnr Nov 27, 2023

Choose a reason for hiding this comment

compiler-errors Nov 27, 2023

Choose a reason for hiding this comment

estebank Nov 28, 2023

Choose a reason for hiding this comment

lcnr commented Nov 28, 2023

This comment has been minimized.

compiler-errors commented Dec 5, 2023

lcnr left a comment

Choose a reason for hiding this comment

compiler-errors commented Dec 5, 2023

bors commented Dec 5, 2023

Add `deeply_normalize_for_diagnostics`, use it in coherence #118346

Add `deeply_normalize_for_diagnostics`, use it in coherence #118346