From 3d50334314cb41f2334f201458d4be0aa15ae203 Mon Sep 17 00:00:00 2001 From: yukang Date: Thu, 16 Jan 2025 10:25:59 +0800 Subject: [PATCH 01/23] Add ignore value suggestion in closure body --- compiler/rustc_hir_typeck/src/coercion.rs | 7 ++--- compiler/rustc_hir_typeck/src/expr.rs | 2 +- .../src/fn_ctxt/suggestions.rs | 13 ++++++++++ .../closure-ty-mismatch-issue-128561.rs | 10 +++++++ .../closure-ty-mismatch-issue-128561.stderr | 26 +++++++++++++++++++ 5 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 tests/ui/typeck/closure-ty-mismatch-issue-128561.rs create mode 100644 tests/ui/typeck/closure-ty-mismatch-issue-128561.stderr diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index 625c7f38fbb40..2217eb33062d7 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -1891,9 +1891,10 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { kind: hir::ExprKind::Closure(&hir::Closure { body, .. }), .. }) = parent - && !matches!(fcx.tcx.hir_body(body).value.kind, hir::ExprKind::Block(..)) { - fcx.suggest_missing_semicolon(&mut err, expr, expected, true); + let needs_block = + !matches!(fcx.tcx.hir_body(body).value.kind, hir::ExprKind::Block(..)); + fcx.suggest_missing_semicolon(&mut err, expr, expected, needs_block, true); } // Verify that this is a tail expression of a function, otherwise the // label pointing out the cause for the type coercion will be wrong @@ -1901,7 +1902,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { if let Some(expr) = expression && due_to_block { - fcx.suggest_missing_semicolon(&mut err, expr, expected, false); + fcx.suggest_missing_semicolon(&mut err, expr, expected, false, false); let pointing_at_return_type = fcx.suggest_mismatched_types_on_tail( &mut err, expr, diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 277396da19c13..25b58c7fd9d44 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -921,7 +921,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self, &cause, |mut err| { - self.suggest_missing_semicolon(&mut err, expr, e_ty, false); + self.suggest_missing_semicolon(&mut err, expr, e_ty, false, false); self.suggest_mismatched_types_on_tail( &mut err, expr, ty, e_ty, target_id, ); diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index f9fc121593637..f8d860cf7b7ac 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -754,6 +754,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expression: &'tcx hir::Expr<'tcx>, expected: Ty<'tcx>, needs_block: bool, + parent_is_closure: bool, ) { if expected.is_unit() { // `BlockTailExpression` only relevant if the tail expr would be @@ -789,6 +790,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } } + ExprKind::Path(..) | ExprKind::Lit(_) if parent_is_closure => { + err.span_suggestion( + expression.span.shrink_to_lo(), + "consider ignore the value here", + "_ = ", + if in_external_macro(self.tcx.sess, expression.span) { + Applicability::MaybeIncorrect + } else { + Applicability::MachineApplicable + }, + ); + } _ => (), } } diff --git a/tests/ui/typeck/closure-ty-mismatch-issue-128561.rs b/tests/ui/typeck/closure-ty-mismatch-issue-128561.rs new file mode 100644 index 0000000000000..589a90e71d6ec --- /dev/null +++ b/tests/ui/typeck/closure-ty-mismatch-issue-128561.rs @@ -0,0 +1,10 @@ +fn main() { + b"abc".iter().for_each(|x| x); //~ ERROR: mismatched types + + b"abc".iter().for_each(|x| dbg!(x)); //~ ERROR: mismatched types + + b"abc".iter().for_each(|x| { + println!("{}", x); + x //~ ERROR: mismatched types + }) +} diff --git a/tests/ui/typeck/closure-ty-mismatch-issue-128561.stderr b/tests/ui/typeck/closure-ty-mismatch-issue-128561.stderr new file mode 100644 index 0000000000000..f9b606cd52da7 --- /dev/null +++ b/tests/ui/typeck/closure-ty-mismatch-issue-128561.stderr @@ -0,0 +1,26 @@ +error[E0308]: mismatched types + --> $DIR/closure-ty-mismatch-issue-128561.rs:2:32 + | +LL | b"abc".iter().for_each(|x| x); + | ^ + | | + | expected `()`, found `&u8` + | help: consider ignore the value here: `_ =` + +error[E0308]: mismatched types + --> $DIR/closure-ty-mismatch-issue-128561.rs:4:32 + | +LL | b"abc".iter().for_each(|x| dbg!(x)); + | ^^^^^^^ expected `()`, found `&u8` + | + = note: this error originates in the macro `dbg` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0308]: mismatched types + --> $DIR/closure-ty-mismatch-issue-128561.rs:8:9 + | +LL | x + | ^ expected `()`, found `&u8` + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0308`. From 8cddffb74ef7fa75767c710e9f85bd4389159adb Mon Sep 17 00:00:00 2001 From: Yukang Date: Fri, 17 Jan 2025 08:19:17 +0800 Subject: [PATCH 02/23] Update compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs Co-authored-by: Esteban Kuber --- compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index f8d860cf7b7ac..a34f3d27fb6c5 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -793,7 +793,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ExprKind::Path(..) | ExprKind::Lit(_) if parent_is_closure => { err.span_suggestion( expression.span.shrink_to_lo(), - "consider ignore the value here", + "consider ignoring the value", "_ = ", if in_external_macro(self.tcx.sess, expression.span) { Applicability::MaybeIncorrect From d6d9c2e7516c687553480a4e171f89dfc22009dd Mon Sep 17 00:00:00 2001 From: Yukang Date: Fri, 17 Jan 2025 08:19:23 +0800 Subject: [PATCH 03/23] Update compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs Co-authored-by: Esteban Kuber --- compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs | 2 +- .../ui/typeck/closure-ty-mismatch-issue-128561.stderr | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index a34f3d27fb6c5..a31abef43757b 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -791,7 +791,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } ExprKind::Path(..) | ExprKind::Lit(_) if parent_is_closure => { - err.span_suggestion( + err.span_suggestion_verbose( expression.span.shrink_to_lo(), "consider ignoring the value", "_ = ", diff --git a/tests/ui/typeck/closure-ty-mismatch-issue-128561.stderr b/tests/ui/typeck/closure-ty-mismatch-issue-128561.stderr index f9b606cd52da7..31acc5bb10ec0 100644 --- a/tests/ui/typeck/closure-ty-mismatch-issue-128561.stderr +++ b/tests/ui/typeck/closure-ty-mismatch-issue-128561.stderr @@ -2,10 +2,12 @@ error[E0308]: mismatched types --> $DIR/closure-ty-mismatch-issue-128561.rs:2:32 | LL | b"abc".iter().for_each(|x| x); - | ^ - | | - | expected `()`, found `&u8` - | help: consider ignore the value here: `_ =` + | ^ expected `()`, found `&u8` + | +help: consider ignoring the value + | +LL | b"abc".iter().for_each(|x| _ = x); + | +++ error[E0308]: mismatched types --> $DIR/closure-ty-mismatch-issue-128561.rs:4:32 From 774ab462d692615d65dc79bb36685ac5cfebd7d4 Mon Sep 17 00:00:00 2001 From: yukang Date: Fri, 17 Jan 2025 12:15:25 +0800 Subject: [PATCH 04/23] code cleanup and do not suggest for external macro except we get better solution --- compiler/rustc_hir_typeck/src/coercion.rs | 4 +--- compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs | 11 +++++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index 2217eb33062d7..afa7175238c6b 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -1883,9 +1883,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { fcx.err_ctxt().report_mismatched_types(cause, fcx.param_env, expected, found, ty_err); let due_to_block = matches!(fcx.tcx.hir_node(block_or_return_id), hir::Node::Block(..)); - - let parent_id = fcx.tcx.parent_hir_id(block_or_return_id); - let parent = fcx.tcx.hir_node(parent_id); + let parent = fcx.tcx.parent_hir_node(block_or_return_id); if let Some(expr) = expression && let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(&hir::Closure { body, .. }), diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index a31abef43757b..8f2665212b86b 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -790,16 +790,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } } - ExprKind::Path(..) | ExprKind::Lit(_) if parent_is_closure => { + ExprKind::Path(..) | ExprKind::Lit(_) + if parent_is_closure + && !expression.span.in_external_macro(self.tcx.sess.source_map()) => + { err.span_suggestion_verbose( expression.span.shrink_to_lo(), "consider ignoring the value", "_ = ", - if in_external_macro(self.tcx.sess, expression.span) { - Applicability::MaybeIncorrect - } else { - Applicability::MachineApplicable - }, + Applicability::MachineApplicable, ); } _ => (), From 2d5e80b8cb89e9d809e569426d948e4f1fa6002d Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 10 Apr 2025 10:57:55 +0000 Subject: [PATCH 05/23] Handle regions equivalent to 'static in non_local_bounds `non_local_bounds` would only find non local bounds that strictly bound a given region, but it's possible that a local region is equated to 'static when showing a type referencing a locally bound lifetime, such as `dyn Any + 'a` in the tests added, is well-formed. In this case we should return 'static. --- .../src/type_check/free_region_relations.rs | 3 +- .../src/transitive_relation.rs | 14 +++ tests/crashes/122704.rs | 14 --- .../unconstrained-closure-lifetime-generic.rs | 22 ++++ ...onstrained-closure-lifetime-generic.stderr | 119 ++++++++++++++++++ ...nstrained-closure-lifetime-trait-object.rs | 11 ++ ...ained-closure-lifetime-trait-object.stderr | 17 +++ 7 files changed, 185 insertions(+), 15 deletions(-) delete mode 100644 tests/crashes/122704.rs create mode 100644 tests/ui/borrowck/unconstrained-closure-lifetime-generic.rs create mode 100644 tests/ui/borrowck/unconstrained-closure-lifetime-generic.stderr create mode 100644 tests/ui/borrowck/unconstrained-closure-lifetime-trait-object.rs create mode 100644 tests/ui/borrowck/unconstrained-closure-lifetime-trait-object.stderr diff --git a/compiler/rustc_borrowck/src/type_check/free_region_relations.rs b/compiler/rustc_borrowck/src/type_check/free_region_relations.rs index eaac633b512d6..aad10e3d6dc03 100644 --- a/compiler/rustc_borrowck/src/type_check/free_region_relations.rs +++ b/compiler/rustc_borrowck/src/type_check/free_region_relations.rs @@ -133,7 +133,8 @@ impl UniversalRegionRelations<'_> { assert!(self.universal_regions.is_universal_region(fr0)); let mut external_parents = vec![]; - let mut queue = vec![fr0]; + + let mut queue = vec![relation.minimal_scc_representative(fr0)]; // Keep expanding `fr` into its parents until we reach // non-local regions. diff --git a/compiler/rustc_data_structures/src/transitive_relation.rs b/compiler/rustc_data_structures/src/transitive_relation.rs index 33ac279f3e0ae..31abea938196c 100644 --- a/compiler/rustc_data_structures/src/transitive_relation.rs +++ b/compiler/rustc_data_structures/src/transitive_relation.rs @@ -354,6 +354,20 @@ impl TransitiveRelation { .collect() } + /// Given an element A, elements B with the lowest index such that `A R B` + /// and `B R A`, or `A` if no such element exists. + pub fn minimal_scc_representative(&self, a: T) -> T { + match self.index(a) { + Some(a_i) => self.with_closure(|closure| { + closure + .iter(a_i.0) + .find(|i| closure.contains(*i, a_i.0)) + .map_or(a, |i| self.elements[i]) + }), + None => a, + } + } + fn with_closure(&self, op: OP) -> R where OP: FnOnce(&BitMatrix) -> R, diff --git a/tests/crashes/122704.rs b/tests/crashes/122704.rs deleted file mode 100644 index d6c07be831885..0000000000000 --- a/tests/crashes/122704.rs +++ /dev/null @@ -1,14 +0,0 @@ -//@ known-bug: #122704 -use std::any::Any; - -pub struct Foo { - bar: Box Fn(&'a usize) -> Box>, -} - -impl Foo { - pub fn ack(&mut self, f: impl for<'a> Fn(&'a usize) -> Box) { - self.bar = Box::new(|baz| Box::new(f(baz))); - } -} - -fn main() {} diff --git a/tests/ui/borrowck/unconstrained-closure-lifetime-generic.rs b/tests/ui/borrowck/unconstrained-closure-lifetime-generic.rs new file mode 100644 index 0000000000000..4fdf5470feac6 --- /dev/null +++ b/tests/ui/borrowck/unconstrained-closure-lifetime-generic.rs @@ -0,0 +1,22 @@ +// Regression test for #122704 +use std::any::Any; + +pub struct Foo { + bar: Box Fn(&'a usize) -> Box>, +} + +impl Foo { + pub fn ack(&mut self, f: impl for<'a> Fn(&'a usize) -> Box) { + self.bar = Box::new(|baz| Box::new(f(baz))); + //~^ ERROR the parameter type `impl for<'a> Fn(&'a usize) -> Box` may not live long enough + //~| ERROR the parameter type `impl for<'a> Fn(&'a usize) -> Box` may not live long enough + //~| ERROR the parameter type `impl for<'a> Fn(&'a usize) -> Box` may not live long enough + //~| ERROR the parameter type `impl for<'a> Fn(&'a usize) -> Box` may not live long enough + //~| ERROR the parameter type `I` may not live long enough + //~| ERROR the parameter type `I` may not live long enough + //~| ERROR the parameter type `I` may not live long enough + //~| ERROR `f` does not live long enough + } +} + +fn main() {} diff --git a/tests/ui/borrowck/unconstrained-closure-lifetime-generic.stderr b/tests/ui/borrowck/unconstrained-closure-lifetime-generic.stderr new file mode 100644 index 0000000000000..df86ce79f09c7 --- /dev/null +++ b/tests/ui/borrowck/unconstrained-closure-lifetime-generic.stderr @@ -0,0 +1,119 @@ +error[E0310]: the parameter type `impl for<'a> Fn(&'a usize) -> Box` may not live long enough + --> $DIR/unconstrained-closure-lifetime-generic.rs:10:9 + | +LL | self.bar = Box::new(|baz| Box::new(f(baz))); + | ^^^^^^^^ + | | + | the parameter type `impl for<'a> Fn(&'a usize) -> Box` must be valid for the static lifetime... + | ...so that the type `impl for<'a> Fn(&'a usize) -> Box` will meet its required lifetime bounds + | +help: consider adding an explicit lifetime bound + | +LL | pub fn ack(&mut self, f: impl for<'a> Fn(&'a usize) -> Box + 'static) { + | +++++++++ + +error[E0310]: the parameter type `impl for<'a> Fn(&'a usize) -> Box` may not live long enough + --> $DIR/unconstrained-closure-lifetime-generic.rs:10:9 + | +LL | self.bar = Box::new(|baz| Box::new(f(baz))); + | ^^^^^^^^ + | | + | the parameter type `impl for<'a> Fn(&'a usize) -> Box` must be valid for the static lifetime... + | ...so that the type `impl for<'a> Fn(&'a usize) -> Box` will meet its required lifetime bounds + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider adding an explicit lifetime bound + | +LL | pub fn ack(&mut self, f: impl for<'a> Fn(&'a usize) -> Box + 'static) { + | +++++++++ + +error[E0310]: the parameter type `impl for<'a> Fn(&'a usize) -> Box` may not live long enough + --> $DIR/unconstrained-closure-lifetime-generic.rs:10:20 + | +LL | self.bar = Box::new(|baz| Box::new(f(baz))); + | ^^^^^^^^ + | | + | the parameter type `impl for<'a> Fn(&'a usize) -> Box` must be valid for the static lifetime... + | ...so that the type `impl for<'a> Fn(&'a usize) -> Box` will meet its required lifetime bounds + | +help: consider adding an explicit lifetime bound + | +LL | pub fn ack(&mut self, f: impl for<'a> Fn(&'a usize) -> Box + 'static) { + | +++++++++ + +error[E0310]: the parameter type `impl for<'a> Fn(&'a usize) -> Box` may not live long enough + --> $DIR/unconstrained-closure-lifetime-generic.rs:10:20 + | +LL | self.bar = Box::new(|baz| Box::new(f(baz))); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | the parameter type `impl for<'a> Fn(&'a usize) -> Box` must be valid for the static lifetime... + | ...so that the type `impl for<'a> Fn(&'a usize) -> Box` will meet its required lifetime bounds + | +help: consider adding an explicit lifetime bound + | +LL | pub fn ack(&mut self, f: impl for<'a> Fn(&'a usize) -> Box + 'static) { + | +++++++++ + +error[E0310]: the parameter type `I` may not live long enough + --> $DIR/unconstrained-closure-lifetime-generic.rs:10:35 + | +LL | self.bar = Box::new(|baz| Box::new(f(baz))); + | ^^^^^^^^^^^^^^^^ + | | + | the parameter type `I` must be valid for the static lifetime... + | ...so that the type `I` will meet its required lifetime bounds + | +help: consider adding an explicit lifetime bound + | +LL | pub fn ack(&mut self, f: impl for<'a> Fn(&'a usize) -> Box) { + | +++++++++ + +error[E0310]: the parameter type `I` may not live long enough + --> $DIR/unconstrained-closure-lifetime-generic.rs:10:35 + | +LL | self.bar = Box::new(|baz| Box::new(f(baz))); + | ^^^^^^^^^^^^^^^^ + | | + | the parameter type `I` must be valid for the static lifetime... + | ...so that the type `I` will meet its required lifetime bounds + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider adding an explicit lifetime bound + | +LL | pub fn ack(&mut self, f: impl for<'a> Fn(&'a usize) -> Box) { + | +++++++++ + +error[E0311]: the parameter type `I` may not live long enough + --> $DIR/unconstrained-closure-lifetime-generic.rs:10:35 + | +LL | pub fn ack(&mut self, f: impl for<'a> Fn(&'a usize) -> Box) { + | --------- the parameter type `I` must be valid for the anonymous lifetime defined here... +LL | self.bar = Box::new(|baz| Box::new(f(baz))); + | ^^^^^^^^^^^^^^^^ ...so that the type `I` will meet its required lifetime bounds + | +help: consider adding an explicit lifetime bound + | +LL | pub fn ack<'a, I: 'a>(&'a mut self, f: impl for<'a> Fn(&'a usize) -> Box) { + | +++ ++++ ++ + +error[E0597]: `f` does not live long enough + --> $DIR/unconstrained-closure-lifetime-generic.rs:10:44 + | +LL | pub fn ack(&mut self, f: impl for<'a> Fn(&'a usize) -> Box) { + | - binding `f` declared here +LL | self.bar = Box::new(|baz| Box::new(f(baz))); + | -------- ----- ^ borrowed value does not live long enough + | | | + | | value captured here + | coercion requires that `f` is borrowed for `'static` +... +LL | } + | - `f` dropped here while still borrowed + | + = note: due to object lifetime defaults, `Box Fn(&'a usize) -> Box<(dyn Any + 'a)>>` actually means `Box<(dyn for<'a> Fn(&'a usize) -> Box<(dyn Any + 'a)> + 'static)>` + +error: aborting due to 8 previous errors + +Some errors have detailed explanations: E0310, E0311, E0597. +For more information about an error, try `rustc --explain E0310`. diff --git a/tests/ui/borrowck/unconstrained-closure-lifetime-trait-object.rs b/tests/ui/borrowck/unconstrained-closure-lifetime-trait-object.rs new file mode 100644 index 0000000000000..3eee98d9bdb21 --- /dev/null +++ b/tests/ui/borrowck/unconstrained-closure-lifetime-trait-object.rs @@ -0,0 +1,11 @@ +// Regression test for #139004 +use std::any::Any; + +type B = Box Fn(&(dyn Any + 'a)) -> Box>; + +fn foo() -> B { + Box::new(|e| Box::new(e.is::())) + //~^ ERROR the parameter type `E` may not live long enough +} + +fn main() {} diff --git a/tests/ui/borrowck/unconstrained-closure-lifetime-trait-object.stderr b/tests/ui/borrowck/unconstrained-closure-lifetime-trait-object.stderr new file mode 100644 index 0000000000000..c9d5f78828d44 --- /dev/null +++ b/tests/ui/borrowck/unconstrained-closure-lifetime-trait-object.stderr @@ -0,0 +1,17 @@ +error[E0310]: the parameter type `E` may not live long enough + --> $DIR/unconstrained-closure-lifetime-trait-object.rs:7:29 + | +LL | Box::new(|e| Box::new(e.is::())) + | ^^ + | | + | the parameter type `E` must be valid for the static lifetime... + | ...so that the type `E` will meet its required lifetime bounds + | +help: consider adding an explicit lifetime bound + | +LL | fn foo() -> B { + | +++++++++ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0310`. From c57ef293eb5c628b5a96253868c4ca171416780a Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 14 Apr 2025 10:40:44 +0000 Subject: [PATCH 06/23] Add unit tests for minimal_scc_representative --- .../src/transitive_relation/tests.rs | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/compiler/rustc_data_structures/src/transitive_relation/tests.rs b/compiler/rustc_data_structures/src/transitive_relation/tests.rs index e756c546e41ba..cba14b5b64bc2 100644 --- a/compiler/rustc_data_structures/src/transitive_relation/tests.rs +++ b/compiler/rustc_data_structures/src/transitive_relation/tests.rs @@ -376,3 +376,44 @@ fn parent() { let p = relation.postdom_parent(3); assert_eq!(p, Some(0)); } + +#[test] +fn minimal_scc_representative_1() { + // +---------+ + // v | + // a -> c -> d -> e + // ^ ^ + // | | + // b ---+ + + // "digraph { a -> c -> d -> e -> c; b -> d; b -> e; }", + let mut relation = TransitiveRelationBuilder::default(); + relation.add("a", "c"); + relation.add("c", "d"); + relation.add("d", "e"); + relation.add("e", "c"); + relation.add("b", "d"); + relation.add("b", "e"); + let relation = relation.freeze(); + + assert_eq!(relation.minimal_scc_representative("a"), "a"); + assert_eq!(relation.minimal_scc_representative("b"), "b"); + assert_eq!(relation.minimal_scc_representative("c"), "c"); + assert_eq!(relation.minimal_scc_representative("d"), "c"); + assert_eq!(relation.minimal_scc_representative("e"), "c"); +} + +#[test] +fn minimal_scc_representative_2() { + // "digraph { a -> b; a -> a; b -> a; c -> c}", + let mut relation = TransitiveRelationBuilder::default(); + relation.add("a", "b"); + relation.add("b", "a"); + relation.add("a", "a"); + relation.add("c", "c"); + let relation = relation.freeze(); + + assert_eq!(relation.minimal_scc_representative("a"), "a"); + assert_eq!(relation.minimal_scc_representative("b"), "a"); + assert_eq!(relation.minimal_scc_representative("c"), "c"); +} From 1d9d30f35ab715f1b9fcf0f7ee6ce8d61216b253 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Tue, 22 Apr 2025 18:53:23 +0200 Subject: [PATCH 07/23] Consistently use the DiagCtxtHandle of HirTyLowerer instead of the one of TyCtxt They are not the same. --- .../src/hir_ty_lowering/bounds.rs | 18 ++++++++---------- .../src/hir_ty_lowering/cmse.rs | 2 +- .../src/hir_ty_lowering/dyn_compatibility.rs | 2 +- .../src/hir_ty_lowering/lint.rs | 2 +- .../src/hir_ty_lowering/mod.rs | 8 ++++---- 5 files changed, 15 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs index bf91eb1b8fdac..f412ac34834d0 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs @@ -309,7 +309,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { false => "`?Sized`", }; // There was a `?Trait` bound, but it was neither `?Sized` nor `experimental_default_bounds`. - tcx.dcx().span_err( + self.dcx().span_err( unbound.span, format!( "relaxing a default bound only does something for {}; \ @@ -810,15 +810,13 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { // `resolve_bound_vars`, since we'd need to introduce those as elided // bound vars on the where clause too. if bound.has_bound_vars() { - return Err(self.tcx().dcx().emit_err( - errors::AssociatedItemTraitUninferredGenericParams { - span, - inferred_sugg: Some(span.with_hi(item_segment.ident.span.lo())), - bound: format!("{}::", tcx.anonymize_bound_vars(bound).skip_binder(),), - mpart_sugg: None, - what: "function", - }, - )); + return Err(self.dcx().emit_err(errors::AssociatedItemTraitUninferredGenericParams { + span, + inferred_sugg: Some(span.with_hi(item_segment.ident.span.lo())), + bound: format!("{}::", tcx.anonymize_bound_vars(bound).skip_binder(),), + mpart_sugg: None, + what: "function", + })); } let trait_def_id = bound.def_id(); diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs index d1ee5a5494c00..ebeb3b58208eb 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs @@ -35,7 +35,7 @@ pub(crate) fn validate_cmse_abi<'tcx>( _ => tcx.hir_span(hir_id), }; struct_span_code_err!( - tcx.dcx(), + dcx, span, E0781, "the `\"C-cmse-nonsecure-call\"` ABI is only allowed on function pointers" diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs index 88f745892048c..3af439eb2fd52 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs @@ -132,7 +132,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { if references_self { // With trait alias and type alias combined, type resolver // may not be able to catch all illegal `Self` usages (issue 139082) - let guar = tcx.dcx().emit_err(SelfInTypeAlias { span }); + let guar = self.dcx().emit_err(SelfInTypeAlias { span }); b.term = replace_dummy_self_with_error(tcx, b.term, guar); } } diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs index 483b61add3380..1f4692b19f1ad 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs @@ -103,7 +103,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { // In case there is an associated type with the same name // Add the suggestion to this error if let Some(mut sugg) = - tcx.dcx().steal_non_err(self_ty.span, StashKey::AssociatedTypeSuggestion) + self.dcx().steal_non_err(self_ty.span, StashKey::AssociatedTypeSuggestion) && let Suggestions::Enabled(ref mut s1) = diag.suggestions && let Suggestions::Enabled(ref mut s2) = sugg.suggestions { diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index 5e79e93201531..4163a2028e40a 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -1202,7 +1202,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { )? { LoweredAssoc::Term(def_id, args) => { if !tcx.associated_item(def_id).is_type_const_capable(tcx) { - let mut err = tcx.dcx().struct_span_err( + let mut err = self.dcx().struct_span_err( span, "use of trait associated const without `#[type_const]`", ); @@ -2323,7 +2323,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { if tcx.features().generic_const_parameter_types() && (anon_const_type.has_free_regions() || anon_const_type.has_erased_regions()) { - let e = tcx.dcx().span_err( + let e = self.dcx().span_err( const_arg.span(), "anonymous constants with lifetimes in their type are not yet supported", ); @@ -2334,7 +2334,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { // use this type to feed the `type_of` and query results must not contain inference // variables otherwise we will ICE. if anon_const_type.has_non_region_infer() { - let e = tcx.dcx().span_err( + let e = self.dcx().span_err( const_arg.span(), "anonymous constants with inferred types are not yet supported", ); @@ -2344,7 +2344,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { // We error when the type contains unsubstituted generics since we do not currently // give the anon const any of the generics from the parent. if anon_const_type.has_non_region_param() { - let e = tcx.dcx().span_err( + let e = self.dcx().span_err( const_arg.span(), "anonymous constants referencing generics are not yet supported", ); From 5fdc0de28c0c843dd9a3d107484d044f9c08b969 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Tue, 22 Apr 2025 19:17:28 +0200 Subject: [PATCH 08/23] Eliminate unnecessary parameter --- compiler/rustc_hir_analysis/messages.ftl | 2 +- compiler/rustc_hir_analysis/src/collect.rs | 4 +--- .../src/hir_ty_lowering/bounds.rs | 14 +++++++------- .../rustc_hir_analysis/src/hir_ty_lowering/mod.rs | 10 ++-------- compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs | 3 +-- 5 files changed, 12 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index 277bb7bd3e15c..1138e9774d510 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -37,7 +37,7 @@ hir_analysis_assoc_kind_mismatch = expected {$expected}, found {$got} hir_analysis_assoc_kind_mismatch_wrap_in_braces_sugg = consider adding braces here -hir_analysis_associated_type_trait_uninferred_generic_params = cannot use the associated {$what} of a trait with uninferred generic parameters +hir_analysis_associated_type_trait_uninferred_generic_params = cannot use the {$what} of a trait with uninferred generic parameters .suggestion = use a fully qualified path with inferred lifetimes hir_analysis_associated_type_trait_uninferred_generic_params_multipart_suggestion = use a fully qualified path with explicit lifetimes diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index 4520fbe352cea..14fd265baa6b0 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -44,7 +44,6 @@ use rustc_trait_selection::traits::ObligationCtxt; use tracing::{debug, instrument}; use crate::errors; -use crate::hir_ty_lowering::errors::assoc_tag_str; use crate::hir_ty_lowering::{FeedConstTy, HirTyLowerer, RegionInferReason}; pub(crate) mod dump; @@ -450,7 +449,6 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> { item_def_id: DefId, item_segment: &rustc_hir::PathSegment<'tcx>, poly_trait_ref: ty::PolyTraitRef<'tcx>, - assoc_tag: ty::AssocTag, ) -> Result<(DefId, ty::GenericArgsRef<'tcx>), ErrorGuaranteed> { if let Some(trait_ref) = poly_trait_ref.no_bound_vars() { let item_args = self.lowerer().lower_generic_args_of_assoc_item( @@ -525,7 +523,7 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> { inferred_sugg, bound, mpart_sugg, - what: assoc_tag_str(assoc_tag), + what: self.tcx.def_descr(item_def_id), })) } } diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs index f412ac34834d0..800bf5e1b3022 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs @@ -803,6 +803,11 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } }; + let trait_def_id = bound.def_id(); + let assoc_fn = self + .probe_assoc_item(assoc_ident, ty::AssocTag::Fn, qpath_hir_id, span, trait_def_id) + .expect("failed to find associated fn"); + // Don't let `T::method` resolve to some `for<'a> >::method`, // which may happen via a higher-ranked where clause or supertrait. // This is the same restrictions as associated types; even though we could @@ -815,16 +820,11 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { inferred_sugg: Some(span.with_hi(item_segment.ident.span.lo())), bound: format!("{}::", tcx.anonymize_bound_vars(bound).skip_binder(),), mpart_sugg: None, - what: "function", + what: assoc_fn.descr(), })); } - let trait_def_id = bound.def_id(); - let assoc_ty = self - .probe_assoc_item(assoc_ident, ty::AssocTag::Fn, qpath_hir_id, span, trait_def_id) - .expect("failed to find associated type"); - - Ok((bound, assoc_ty.def_id)) + Ok((bound, assoc_fn.def_id)) } /// Do the common parts of lowering an RTN type. This involves extending the diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index 4163a2028e40a..533499ed3447a 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -168,7 +168,6 @@ pub trait HirTyLowerer<'tcx> { item_def_id: DefId, item_segment: &hir::PathSegment<'tcx>, poly_trait_ref: ty::PolyTraitRef<'tcx>, - assoc_tag: ty::AssocTag, ) -> Result<(DefId, GenericArgsRef<'tcx>), ErrorGuaranteed>; fn lower_fn_sig( @@ -1433,13 +1432,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { let assoc_item = self .probe_assoc_item(assoc_ident, mode.assoc_tag(), hir_ref_id, span, trait_did) .expect("failed to find associated item"); - let (def_id, args) = self.lower_assoc_shared( - span, - assoc_item.def_id, - assoc_segment, - bound, - mode.assoc_tag(), - )?; + let (def_id, args) = + self.lower_assoc_shared(span, assoc_item.def_id, assoc_segment, bound)?; let result = LoweredAssoc::Term(def_id, args); if let Some(variant_def_id) = variant_resolution { diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs index de189b301092c..fb557555237e5 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs @@ -314,11 +314,10 @@ impl<'tcx> HirTyLowerer<'tcx> for FnCtxt<'_, 'tcx> { item_def_id: DefId, item_segment: &rustc_hir::PathSegment<'tcx>, poly_trait_ref: ty::PolyTraitRef<'tcx>, - _assoc_tag: ty::AssocTag, ) -> Result<(DefId, ty::GenericArgsRef<'tcx>), ErrorGuaranteed> { let trait_ref = self.instantiate_binder_with_fresh_vars( span, - // FIXME(mgca): this should be assoc const if that is the `kind` + // FIXME(mgca): `item_def_id` can be an AssocConst; rename this variant. infer::BoundRegionConversionTime::AssocTypeProjection(item_def_id), poly_trait_ref, ); From 3fd047d164c09cc3c0d5a8c6833e6021c5f5934e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Wed, 23 Apr 2025 15:58:20 +0200 Subject: [PATCH 09/23] Name methods pertaining to HIR ty lowering of paths more appropriately --- compiler/rustc_hir_analysis/src/collect.rs | 2 +- .../src/hir_ty_lowering/bounds.rs | 2 +- .../src/hir_ty_lowering/mod.rs | 280 +++++++++--------- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 17 +- compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs | 2 +- 5 files changed, 162 insertions(+), 141 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index 14fd265baa6b0..350bdc7821d5b 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -443,7 +443,7 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> { self.tcx.at(span).type_param_predicates((self.item_def_id, def_id, assoc_ident)) } - fn lower_assoc_shared( + fn lower_assoc_item_path( &self, span: Span, item_def_id: DefId, diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs index 800bf5e1b3022..9544f9b0c2bd1 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs @@ -737,7 +737,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } /// Perform type-dependent lookup for a *method* for return type notation. - /// This generally mirrors `::lower_assoc_path`. + /// This generally mirrors `::lower_type_relative_path`. fn resolve_type_relative_return_type_notation( &self, qself: &'tcx hir::Ty<'tcx>, diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index 533499ed3447a..5b587bb99ee4d 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -150,7 +150,7 @@ pub trait HirTyLowerer<'tcx> { assoc_ident: Ident, ) -> ty::EarlyBinder<'tcx, &'tcx [(ty::Clause<'tcx>, Span)]>; - /// Lower an associated type/const (from a trait) to a projection. + /// Lower a path to an associated item (of a trait) to a projection. /// /// This method has to be defined by the concrete lowering context because /// dealing with higher-ranked trait references depends on its capabilities: @@ -162,7 +162,7 @@ pub trait HirTyLowerer<'tcx> { /// /// The canonical example of this is associated type `T::P` where `T` is a type /// param constrained by `T: for<'a> Trait<'a>` and where `Trait` defines `P`. - fn lower_assoc_shared( + fn lower_assoc_item_path( &self, span: Span, item_def_id: DefId, @@ -244,39 +244,39 @@ pub enum FeedConstTy<'a, 'tcx> { } #[derive(Debug, Clone, Copy)] -enum LowerAssocMode { +enum LowerTypeRelativePathMode { Type { permit_variants: bool }, Const, } -impl LowerAssocMode { +impl LowerTypeRelativePathMode { fn assoc_tag(self) -> ty::AssocTag { match self { - LowerAssocMode::Type { .. } => ty::AssocTag::Type, - LowerAssocMode::Const => ty::AssocTag::Const, + Self::Type { .. } => ty::AssocTag::Type, + Self::Const => ty::AssocTag::Const, } } fn def_kind(self) -> DefKind { match self { - LowerAssocMode::Type { .. } => DefKind::AssocTy, - LowerAssocMode::Const => DefKind::AssocConst, + Self::Type { .. } => DefKind::AssocTy, + Self::Const => DefKind::AssocConst, } } fn permit_variants(self) -> bool { match self { - LowerAssocMode::Type { permit_variants } => permit_variants, + Self::Type { permit_variants } => permit_variants, // FIXME(mgca): Support paths like `Option::::None` or `Option::::Some` which // resolve to const ctors/fn items respectively. - LowerAssocMode::Const => false, + Self::Const => false, } } } #[derive(Debug, Clone, Copy)] -enum LoweredAssoc<'tcx> { - Term(DefId, GenericArgsRef<'tcx>), +enum TypeRelativePath<'tcx> { + AssocItem(DefId, GenericArgsRef<'tcx>), Variant { adt: Ty<'tcx>, variant_did: DefId }, } @@ -1126,7 +1126,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { Ok(bound) } - /// Lower a [type-relative] path referring to an associated type or to an enum variant. + /// Lower a [type-relative](hir::QPath::TypeRelative) path in type position to a type. /// /// If the path refers to an enum variant and `permit_variants` holds, /// the returned type is simply the provided self type `qself_ty`. @@ -1147,59 +1147,61 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { /// described in the previous paragraph and their modeling of projections would likely be /// very similar in nature. /// - /// [type-relative]: hir::QPath::TypeRelative /// [#22519]: https://github.com/rust-lang/rust/issues/22519 /// [iat]: https://github.com/rust-lang/rust/issues/8995#issuecomment-1569208403 // // NOTE: When this function starts resolving `Trait::AssocTy` successfully // it should also start reporting the `BARE_TRAIT_OBJECTS` lint. #[instrument(level = "debug", skip_all, ret)] - pub fn lower_assoc_path_ty( + pub fn lower_type_relative_ty_path( &self, - hir_ref_id: HirId, + self_ty: Ty<'tcx>, + hir_self_ty: &'tcx hir::Ty<'tcx>, + segment: &'tcx hir::PathSegment<'tcx>, + qpath_hir_id: HirId, span: Span, - qself_ty: Ty<'tcx>, - qself: &'tcx hir::Ty<'tcx>, - assoc_segment: &'tcx hir::PathSegment<'tcx>, permit_variants: bool, ) -> Result<(Ty<'tcx>, DefKind, DefId), ErrorGuaranteed> { let tcx = self.tcx(); - match self.lower_assoc_path_shared( - hir_ref_id, + match self.lower_type_relative_path( + self_ty, + hir_self_ty, + segment, + qpath_hir_id, span, - qself_ty, - qself, - assoc_segment, - LowerAssocMode::Type { permit_variants }, + LowerTypeRelativePathMode::Type { permit_variants }, )? { - LoweredAssoc::Term(def_id, args) => { + TypeRelativePath::AssocItem(def_id, args) => { let alias_ty = ty::AliasTy::new_from_args(tcx, def_id, args); let ty = Ty::new_alias(tcx, alias_ty.kind(tcx), alias_ty); Ok((ty, tcx.def_kind(def_id), def_id)) } - LoweredAssoc::Variant { adt, variant_did } => Ok((adt, DefKind::Variant, variant_did)), + TypeRelativePath::Variant { adt, variant_did } => { + Ok((adt, DefKind::Variant, variant_did)) + } } } + /// Lower a [type-relative][hir::QPath::TypeRelative] path to a (type-level) constant. #[instrument(level = "debug", skip_all, ret)] - fn lower_assoc_path_const( + fn lower_type_relative_const_path( &self, - hir_ref_id: HirId, + self_ty: Ty<'tcx>, + hir_self_ty: &'tcx hir::Ty<'tcx>, + segment: &'tcx hir::PathSegment<'tcx>, + qpath_hir_id: HirId, span: Span, - qself_ty: Ty<'tcx>, - qself: &'tcx hir::Ty<'tcx>, - assoc_segment: &'tcx hir::PathSegment<'tcx>, ) -> Result, ErrorGuaranteed> { let tcx = self.tcx(); - let (def_id, args) = match self.lower_assoc_path_shared( - hir_ref_id, + let (def_id, args) = match self.lower_type_relative_path( + self_ty, + hir_self_ty, + segment, + qpath_hir_id, span, - qself_ty, - qself, - assoc_segment, - LowerAssocMode::Const, + LowerTypeRelativePathMode::Const, )? { - LoweredAssoc::Term(def_id, args) => { + TypeRelativePath::AssocItem(def_id, args) => { if !tcx.associated_item(def_id).is_type_const_capable(tcx) { let mut err = self.dcx().struct_span_err( span, @@ -1212,75 +1214,78 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } // FIXME(mgca): implement support for this once ready to support all adt ctor expressions, // not just const ctors - LoweredAssoc::Variant { .. } => { + TypeRelativePath::Variant { .. } => { span_bug!(span, "unexpected variant res for type associated const path") } }; Ok(Const::new_unevaluated(tcx, ty::UnevaluatedConst::new(def_id, args))) } + /// Lower a [type-relative][hir::QPath::TypeRelative] (and type-level) path. #[instrument(level = "debug", skip_all, ret)] - fn lower_assoc_path_shared( + fn lower_type_relative_path( &self, - hir_ref_id: HirId, + self_ty: Ty<'tcx>, + hir_self_ty: &'tcx hir::Ty<'tcx>, + segment: &'tcx hir::PathSegment<'tcx>, + qpath_hir_id: HirId, span: Span, - qself_ty: Ty<'tcx>, - qself: &'tcx hir::Ty<'tcx>, - assoc_segment: &'tcx hir::PathSegment<'tcx>, - mode: LowerAssocMode, - ) -> Result, ErrorGuaranteed> { - debug!(%qself_ty, ?assoc_segment.ident); + mode: LowerTypeRelativePathMode, + ) -> Result, ErrorGuaranteed> { + debug!(%self_ty, ?segment.ident); let tcx = self.tcx(); - - let assoc_ident = assoc_segment.ident; + let ident = segment.ident; // Check if we have an enum variant or an inherent associated type. - let mut variant_resolution = None; - if let Some(adt_def) = self.probe_adt(span, qself_ty) { + let mut variant_def_id = None; + if let Some(adt_def) = self.probe_adt(span, self_ty) { if adt_def.is_enum() { let variant_def = adt_def .variants() .iter() - .find(|vd| tcx.hygienic_eq(assoc_ident, vd.ident(tcx), adt_def.did())); + .find(|vd| tcx.hygienic_eq(ident, vd.ident(tcx), adt_def.did())); if let Some(variant_def) = variant_def { if mode.permit_variants() { - tcx.check_stability(variant_def.def_id, Some(hir_ref_id), span, None); + tcx.check_stability(variant_def.def_id, Some(qpath_hir_id), span, None); let _ = self.prohibit_generic_args( - slice::from_ref(assoc_segment).iter(), - GenericsArgsErrExtend::EnumVariant { qself, assoc_segment, adt_def }, + slice::from_ref(segment).iter(), + GenericsArgsErrExtend::EnumVariant { + qself: hir_self_ty, + assoc_segment: segment, + adt_def, + }, ); - return Ok(LoweredAssoc::Variant { - adt: qself_ty, + return Ok(TypeRelativePath::Variant { + adt: self_ty, variant_did: variant_def.def_id, }); } else { - variant_resolution = Some(variant_def.def_id); + variant_def_id = Some(variant_def.def_id); } } } // FIXME(inherent_associated_types, #106719): Support self types other than ADTs. - if let Some((did, args)) = self.probe_inherent_assoc_shared( - assoc_segment, + if let Some((did, args)) = self.probe_inherent_assoc_item( + segment, adt_def.did(), - qself_ty, - hir_ref_id, + self_ty, + qpath_hir_id, span, mode.assoc_tag(), )? { - return Ok(LoweredAssoc::Term(did, args)); + return Ok(TypeRelativePath::AssocItem(did, args)); } } - let qself_res = if let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = &qself.kind { - path.res - } else { - Res::Err + let self_ty_res = match hir_self_ty.kind { + hir::TyKind::Path(hir::QPath::Resolved(_, path)) => path.res, + _ => Res::Err, }; // Find the type of the associated item, and the trait where the associated // item is declared. - let bound = match (qself_ty.kind(), qself_res) { + let bound = match (self_ty.kind(), self_ty_res) { (_, Res::SelfTyAlias { alias_to: impl_def_id, is_trait_impl: true, .. }) => { // `Self` in an impl of a trait -- we have a concrete self type and a // trait reference. @@ -1298,7 +1303,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { }, AssocItemQSelf::SelfTyAlias, mode.assoc_tag(), - assoc_ident, + ident, span, None, )? @@ -1308,48 +1313,48 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { Res::SelfTyParam { trait_: param_did } | Res::Def(DefKind::TyParam, param_did), ) => self.probe_single_ty_param_bound_for_assoc_item( param_did.expect_local(), - qself.span, + hir_self_ty.span, mode.assoc_tag(), - assoc_ident, + ident, span, )?, _ => { let kind_str = assoc_tag_str(mode.assoc_tag()); - let reported = if variant_resolution.is_some() { + let reported = if variant_def_id.is_some() { // Variant in type position - let msg = format!("expected {kind_str}, found variant `{assoc_ident}`"); + let msg = format!("expected {kind_str}, found variant `{ident}`"); self.dcx().span_err(span, msg) - } else if qself_ty.is_enum() { + } else if self_ty.is_enum() { let mut err = self.dcx().create_err(NoVariantNamed { - span: assoc_ident.span, - ident: assoc_ident, - ty: qself_ty, + span: ident.span, + ident, + ty: self_ty, }); - let adt_def = qself_ty.ty_adt_def().expect("enum is not an ADT"); + let adt_def = self_ty.ty_adt_def().expect("enum is not an ADT"); if let Some(variant_name) = find_best_match_for_name( &adt_def .variants() .iter() .map(|variant| variant.name) .collect::>(), - assoc_ident.name, + ident.name, None, ) && let Some(variant) = adt_def.variants().iter().find(|s| s.name == variant_name) { - let mut suggestion = vec![(assoc_ident.span, variant_name.to_string())]; + let mut suggestion = vec![(ident.span, variant_name.to_string())]; if let hir::Node::Stmt(&hir::Stmt { kind: hir::StmtKind::Semi(expr), .. }) - | hir::Node::Expr(expr) = tcx.parent_hir_node(hir_ref_id) + | hir::Node::Expr(expr) = tcx.parent_hir_node(qpath_hir_id) && let hir::ExprKind::Struct(..) = expr.kind { match variant.ctor { None => { // struct suggestion = vec![( - assoc_ident.span.with_hi(expr.span.hi()), + ident.span.with_hi(expr.span.hi()), if variant.fields.is_empty() { format!("{variant_name} {{}}") } else { @@ -1370,7 +1375,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { let fn_sig = tcx.fn_sig(def_id).instantiate_identity(); let inputs = fn_sig.inputs().skip_binder(); suggestion = vec![( - assoc_ident.span.with_hi(expr.span.hi()), + ident.span.with_hi(expr.span.hi()), format!( "{variant_name}({})", inputs @@ -1384,7 +1389,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { Some((hir::def::CtorKind::Const, _)) => { // unit suggestion = vec![( - assoc_ident.span.with_hi(expr.span.hi()), + ident.span.with_hi(expr.span.hi()), variant_name.to_string(), )]; } @@ -1396,31 +1401,27 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { Applicability::HasPlaceholders, ); } else { - err.span_label( - assoc_ident.span, - format!("variant not found in `{qself_ty}`"), - ); + err.span_label(ident.span, format!("variant not found in `{self_ty}`")); } if let Some(sp) = tcx.hir_span_if_local(adt_def.did()) { - err.span_label(sp, format!("variant `{assoc_ident}` not found here")); + err.span_label(sp, format!("variant `{ident}` not found here")); } err.emit() - } else if let Err(reported) = qself_ty.error_reported() { + } else if let Err(reported) = self_ty.error_reported() { reported } else { - self.maybe_report_similar_assoc_fn(span, qself_ty, qself)?; + self.maybe_report_similar_assoc_fn(span, self_ty, hir_self_ty)?; - let traits: Vec<_> = - self.probe_traits_that_match_assoc_ty(qself_ty, assoc_ident); + let traits: Vec<_> = self.probe_traits_that_match_assoc_ty(self_ty, ident); // Don't print `ty::Error` to the user. self.report_ambiguous_assoc( span, - &[qself_ty.to_string()], + &[self_ty.to_string()], &traits, - assoc_ident.name, + ident.name, mode.assoc_tag(), ) }; @@ -1428,21 +1429,20 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } }; - let trait_did = bound.def_id(); + let trait_def_id = bound.def_id(); let assoc_item = self - .probe_assoc_item(assoc_ident, mode.assoc_tag(), hir_ref_id, span, trait_did) + .probe_assoc_item(ident, mode.assoc_tag(), qpath_hir_id, span, trait_def_id) .expect("failed to find associated item"); - let (def_id, args) = - self.lower_assoc_shared(span, assoc_item.def_id, assoc_segment, bound)?; - let result = LoweredAssoc::Term(def_id, args); + let (def_id, args) = self.lower_assoc_item_path(span, assoc_item.def_id, segment, bound)?; + let result = TypeRelativePath::AssocItem(def_id, args); - if let Some(variant_def_id) = variant_resolution { - tcx.node_span_lint(AMBIGUOUS_ASSOCIATED_ITEMS, hir_ref_id, span, |lint| { + if let Some(variant_def_id) = variant_def_id { + tcx.node_span_lint(AMBIGUOUS_ASSOCIATED_ITEMS, qpath_hir_id, span, |lint| { lint.primary_message("ambiguous associated item"); let mut could_refer_to = |kind: DefKind, def_id, also| { let note_msg = format!( "`{}` could{} refer to the {} defined here", - assoc_ident, + ident, also, tcx.def_kind_descr(kind, def_id) ); @@ -1455,7 +1455,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { lint.span_suggestion( span, "use fully-qualified syntax", - format!("<{} as {}>::{}", qself_ty, tcx.item_name(trait_did), assoc_ident), + format!("<{} as {}>::{}", self_ty, tcx.item_name(trait_def_id), ident), Applicability::MachineApplicable, ); }); @@ -1463,7 +1463,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { Ok(result) } - fn probe_inherent_assoc_shared( + /// Search for inherent associated items for use at the type level. + fn probe_inherent_assoc_item( &self, segment: &hir::PathSegment<'tcx>, adt_did: DefId, @@ -1757,9 +1758,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { .collect() } - /// Lower a qualified path to a type. + /// Lower a [resolved][hir::QPath::Resolved] associated type path to a projection. #[instrument(level = "debug", skip_all)] - fn lower_qpath_ty( + fn lower_resolved_assoc_ty_path( &self, span: Span, opt_self_ty: Option>, @@ -1767,7 +1768,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { trait_segment: Option<&hir::PathSegment<'tcx>>, item_segment: &hir::PathSegment<'tcx>, ) -> Ty<'tcx> { - match self.lower_qpath_shared( + match self.lower_resolved_assoc_item_path( span, opt_self_ty, item_def_id, @@ -1782,9 +1783,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } } - /// Lower a qualified path to a const. + /// Lower a [resolved][hir::QPath::Resolved] associated const path to a (type-level) constant. #[instrument(level = "debug", skip_all)] - fn lower_qpath_const( + fn lower_resolved_assoc_const_path( &self, span: Span, opt_self_ty: Option>, @@ -1792,7 +1793,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { trait_segment: Option<&hir::PathSegment<'tcx>>, item_segment: &hir::PathSegment<'tcx>, ) -> Const<'tcx> { - match self.lower_qpath_shared( + match self.lower_resolved_assoc_item_path( span, opt_self_ty, item_def_id, @@ -1808,8 +1809,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } } + /// Lower a [resolved][hir::QPath::Resolved] (type-level) associated item path. #[instrument(level = "debug", skip_all)] - fn lower_qpath_shared( + fn lower_resolved_assoc_item_path( &self, span: Span, opt_self_ty: Option>, @@ -2063,9 +2065,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { generic_segments } - /// Lower a type `Path` to a type. + /// Lower a [resolved][hir::QPath::Resolved] path to a type. #[instrument(level = "debug", skip_all)] - pub fn lower_path( + pub fn lower_resolved_ty_path( &self, opt_self_ty: Option>, path: &hir::Path<'tcx>, @@ -2196,7 +2198,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } else { None }; - self.lower_qpath_ty( + self.lower_resolved_assoc_ty_path( span, opt_self_ty, def_id, @@ -2292,7 +2294,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } } - /// Convert a [`hir::ConstArg`] to a [`ty::Const`](Const). + /// Lower a [`hir::ConstArg`] to a (type-level) [`ty::Const`](Const). #[instrument(skip(self), level = "debug")] pub fn lower_const_arg( &self, @@ -2357,13 +2359,19 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { hir::ConstArgKind::Path(hir::QPath::Resolved(maybe_qself, path)) => { debug!(?maybe_qself, ?path); let opt_self_ty = maybe_qself.as_ref().map(|qself| self.lower_ty(qself)); - self.lower_const_path_resolved(opt_self_ty, path, hir_id) + self.lower_resolved_const_path(opt_self_ty, path, hir_id) } - hir::ConstArgKind::Path(hir::QPath::TypeRelative(qself, segment)) => { - debug!(?qself, ?segment); - let ty = self.lower_ty(qself); - self.lower_assoc_path_const(hir_id, const_arg.span(), ty, qself, segment) - .unwrap_or_else(|guar| Const::new_error(tcx, guar)) + hir::ConstArgKind::Path(hir::QPath::TypeRelative(hir_self_ty, segment)) => { + debug!(?hir_self_ty, ?segment); + let self_ty = self.lower_ty(hir_self_ty); + self.lower_type_relative_const_path( + self_ty, + hir_self_ty, + segment, + hir_id, + const_arg.span(), + ) + .unwrap_or_else(|guar| Const::new_error(tcx, guar)) } hir::ConstArgKind::Path(qpath @ hir::QPath::LangItem(..)) => { ty::Const::new_error_with_message( @@ -2377,7 +2385,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } } - fn lower_const_path_resolved( + /// Lower a [resolved][hir::QPath::Resolved] path to a (type-level) constant. + fn lower_resolved_const_path( &self, opt_self_ty: Option>, path: &hir::Path<'tcx>, @@ -2414,7 +2423,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } else { None }; - self.lower_qpath_const( + self.lower_resolved_assoc_const_path( span, opt_self_ty, did, @@ -2624,7 +2633,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { hir::TyKind::Path(hir::QPath::Resolved(maybe_qself, path)) => { debug!(?maybe_qself, ?path); let opt_self_ty = maybe_qself.as_ref().map(|qself| self.lower_ty(qself)); - self.lower_path(opt_self_ty, path, hir_ty.hir_id, false) + self.lower_resolved_ty_path(opt_self_ty, path, hir_ty.hir_id, false) } &hir::TyKind::OpaqueDef(opaque_ty) => { // If this is an RPITIT and we are using the new RPITIT lowering scheme, we @@ -2678,12 +2687,19 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { let guar = self.dcx().emit_err(BadReturnTypeNotation { span: hir_ty.span }); Ty::new_error(tcx, guar) } - hir::TyKind::Path(hir::QPath::TypeRelative(qself, segment)) => { - debug!(?qself, ?segment); - let ty = self.lower_ty(qself); - self.lower_assoc_path_ty(hir_ty.hir_id, hir_ty.span, ty, qself, segment, false) - .map(|(ty, _, _)| ty) - .unwrap_or_else(|guar| Ty::new_error(tcx, guar)) + hir::TyKind::Path(hir::QPath::TypeRelative(hir_self_ty, segment)) => { + debug!(?hir_self_ty, ?segment); + let self_ty = self.lower_ty(hir_self_ty); + self.lower_type_relative_ty_path( + self_ty, + hir_self_ty, + segment, + hir_ty.hir_id, + hir_ty.span, + false, + ) + .map(|(ty, _, _)| ty) + .unwrap_or_else(|guar| Ty::new_error(tcx, guar)) } &hir::TyKind::Path(hir::QPath::LangItem(lang_item, span)) => { let def_id = tcx.require_lang_item(lang_item, Some(span)); diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 6cc7e82bbf735..05b00f65cfda8 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -2105,15 +2105,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { match *qpath { QPath::Resolved(ref maybe_qself, path) => { let self_ty = maybe_qself.as_ref().map(|qself| self.lower_ty(qself).raw); - let ty = self.lowerer().lower_path(self_ty, path, hir_id, true); + let ty = self.lowerer().lower_resolved_ty_path(self_ty, path, hir_id, true); (path.res, LoweredTy::from_raw(self, path_span, ty)) } - QPath::TypeRelative(qself, segment) => { - let ty = self.lower_ty(qself); + QPath::TypeRelative(hir_self_ty, segment) => { + let self_ty = self.lower_ty(hir_self_ty); - let result = self - .lowerer() - .lower_assoc_path_ty(hir_id, path_span, ty.raw, qself, segment, true); + let result = self.lowerer().lower_type_relative_ty_path( + self_ty.raw, + hir_self_ty, + segment, + hir_id, + path_span, + true, + ); let ty = result .map(|(ty, _, _)| ty) .unwrap_or_else(|guar| Ty::new_error(self.tcx(), guar)); diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs index fb557555237e5..ea0adf16b1a36 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs @@ -308,7 +308,7 @@ impl<'tcx> HirTyLowerer<'tcx> for FnCtxt<'_, 'tcx> { )) } - fn lower_assoc_shared( + fn lower_assoc_item_path( &self, span: Span, item_def_id: DefId, From bda903ed8ab07fe1e67b138f191a37c1b61a6914 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Wed, 23 Apr 2025 02:40:23 +0200 Subject: [PATCH 10/23] Introduce Boolean type `PermitVariants` for legibility --- .../src/hir_ty_lowering/mod.rs | 35 ++++++++++++------- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 11 ++++-- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index 5b587bb99ee4d..06756bd3ee683 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -245,35 +245,42 @@ pub enum FeedConstTy<'a, 'tcx> { #[derive(Debug, Clone, Copy)] enum LowerTypeRelativePathMode { - Type { permit_variants: bool }, + Type(PermitVariants), Const, } impl LowerTypeRelativePathMode { fn assoc_tag(self) -> ty::AssocTag { match self { - Self::Type { .. } => ty::AssocTag::Type, + Self::Type(_) => ty::AssocTag::Type, Self::Const => ty::AssocTag::Const, } } fn def_kind(self) -> DefKind { match self { - Self::Type { .. } => DefKind::AssocTy, + Self::Type(_) => DefKind::AssocTy, Self::Const => DefKind::AssocConst, } } - fn permit_variants(self) -> bool { + fn permit_variants(self) -> PermitVariants { match self { - Self::Type { permit_variants } => permit_variants, + Self::Type(permit_variants) => permit_variants, // FIXME(mgca): Support paths like `Option::::None` or `Option::::Some` which // resolve to const ctors/fn items respectively. - Self::Const => false, + Self::Const => PermitVariants::No, } } } +/// Whether to permit a path to resolve to an enum variant. +#[derive(Debug, Clone, Copy)] +pub enum PermitVariants { + Yes, + No, +} + #[derive(Debug, Clone, Copy)] enum TypeRelativePath<'tcx> { AssocItem(DefId, GenericArgsRef<'tcx>), @@ -1160,7 +1167,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { segment: &'tcx hir::PathSegment<'tcx>, qpath_hir_id: HirId, span: Span, - permit_variants: bool, + permit_variants: PermitVariants, ) -> Result<(Ty<'tcx>, DefKind, DefId), ErrorGuaranteed> { let tcx = self.tcx(); match self.lower_type_relative_path( @@ -1169,7 +1176,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { segment, qpath_hir_id, span, - LowerTypeRelativePathMode::Type { permit_variants }, + LowerTypeRelativePathMode::Type(permit_variants), )? { TypeRelativePath::AssocItem(def_id, args) => { let alias_ty = ty::AliasTy::new_from_args(tcx, def_id, args); @@ -1245,7 +1252,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { .iter() .find(|vd| tcx.hygienic_eq(ident, vd.ident(tcx), adt_def.did())); if let Some(variant_def) = variant_def { - if mode.permit_variants() { + if let PermitVariants::Yes = mode.permit_variants() { tcx.check_stability(variant_def.def_id, Some(qpath_hir_id), span, None); let _ = self.prohibit_generic_args( slice::from_ref(segment).iter(), @@ -2072,7 +2079,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { opt_self_ty: Option>, path: &hir::Path<'tcx>, hir_id: HirId, - permit_variants: bool, + permit_variants: PermitVariants, ) -> Ty<'tcx> { debug!(?path.res, ?opt_self_ty, ?path.segments); let tcx = self.tcx(); @@ -2103,7 +2110,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { ); self.lower_path_segment(span, did, path.segments.last().unwrap()) } - Res::Def(kind @ DefKind::Variant, def_id) if permit_variants => { + Res::Def(kind @ DefKind::Variant, def_id) + if let PermitVariants::Yes = permit_variants => + { // Lower "variant type" as if it were a real type. // The resulting `Ty` is type of the variant's enum for now. assert_eq!(opt_self_ty, None); @@ -2633,7 +2642,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { hir::TyKind::Path(hir::QPath::Resolved(maybe_qself, path)) => { debug!(?maybe_qself, ?path); let opt_self_ty = maybe_qself.as_ref().map(|qself| self.lower_ty(qself)); - self.lower_resolved_ty_path(opt_self_ty, path, hir_ty.hir_id, false) + self.lower_resolved_ty_path(opt_self_ty, path, hir_ty.hir_id, PermitVariants::No) } &hir::TyKind::OpaqueDef(opaque_ty) => { // If this is an RPITIT and we are using the new RPITIT lowering scheme, we @@ -2696,7 +2705,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { segment, hir_ty.hir_id, hir_ty.span, - false, + PermitVariants::No, ) .map(|(ty, _, _)| ty) .unwrap_or_else(|guar| Ty::new_error(tcx, guar)) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 05b00f65cfda8..54c7819fc7b13 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -9,7 +9,7 @@ use rustc_hir::def_id::DefId; use rustc_hir::intravisit::Visitor; use rustc_hir::{ExprKind, HirId, Node, QPath}; use rustc_hir_analysis::check::potentially_plural_count; -use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer; +use rustc_hir_analysis::hir_ty_lowering::{HirTyLowerer, PermitVariants}; use rustc_index::IndexVec; use rustc_infer::infer::{DefineOpaqueTypes, InferOk, TypeTrace}; use rustc_middle::ty::adjustment::AllowTwoPhase; @@ -2105,7 +2105,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { match *qpath { QPath::Resolved(ref maybe_qself, path) => { let self_ty = maybe_qself.as_ref().map(|qself| self.lower_ty(qself).raw); - let ty = self.lowerer().lower_resolved_ty_path(self_ty, path, hir_id, true); + let ty = self.lowerer().lower_resolved_ty_path( + self_ty, + path, + hir_id, + PermitVariants::Yes, + ); (path.res, LoweredTy::from_raw(self, path_span, ty)) } QPath::TypeRelative(hir_self_ty, segment) => { @@ -2117,7 +2122,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { segment, hir_id, path_span, - true, + PermitVariants::Yes, ); let ty = result .map(|(ty, _, _)| ty) From 7cd1da4a1646d1db3d409d6c2ff682b8f03ba14b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Wed, 23 Apr 2025 16:16:24 +0200 Subject: [PATCH 11/23] Rename and move several error reporting methods Name them more consistently, descriptively and appropriately. Move large error reporting methods into the dedicated error module to make the happy paths in HIR ty lowering more legible. --- .../src/hir_ty_lowering/bounds.rs | 2 +- .../src/hir_ty_lowering/dyn_compatibility.rs | 10 +- .../src/hir_ty_lowering/errors.rs | 195 ++++++++++++++++-- .../src/hir_ty_lowering/mod.rs | 190 ++--------------- 4 files changed, 205 insertions(+), 192 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs index 9544f9b0c2bd1..6aadefc30274c 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs @@ -675,7 +675,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { // Good error for `where Trait::method(..): Send`. let Some(self_ty) = opt_self_ty else { - let guar = self.error_missing_qpath_self_ty( + let guar = self.report_missing_self_ty_for_resolved_path( trait_def_id, hir_ty.span, item_segment, diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs index 3af439eb2fd52..f411bf4d6102d 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs @@ -78,15 +78,13 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { // We don't support empty trait objects. if regular_traits.is_empty() && auto_traits.is_empty() { - let guar = self.report_trait_object_with_no_traits_error( - span, - user_written_bounds.iter().copied(), - ); + let guar = + self.report_trait_object_with_no_traits(span, user_written_bounds.iter().copied()); return Ty::new_error(tcx, guar); } // We don't support >1 principal if regular_traits.len() > 1 { - let guar = self.report_trait_object_addition_traits_error(®ular_traits); + let guar = self.report_trait_object_addition_traits(®ular_traits); return Ty::new_error(tcx, guar); } // Don't create a dyn trait if we have errors in the principal. @@ -344,7 +342,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { hir_bound.trait_ref.path.res == Res::Def(DefKind::Trait, trait_ref.def_id) && hir_bound.span.contains(span) }); - self.complain_about_missing_type_params( + self.report_missing_type_params( missing_type_params, trait_ref.def_id, span, diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs index 3759a224ff75b..60dc84716c73b 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs @@ -5,9 +5,9 @@ use rustc_errors::codes::*; use rustc_errors::{ Applicability, Diag, ErrorGuaranteed, MultiSpan, listify, pluralize, struct_span_code_err, }; -use rustc_hir as hir; use rustc_hir::def::{CtorOf, DefKind, Res}; use rustc_hir::def_id::DefId; +use rustc_hir::{self as hir, HirId}; use rustc_middle::bug; use rustc_middle::ty::fast_reject::{TreatParams, simplify_type}; use rustc_middle::ty::print::{PrintPolyTraitRefExt as _, PrintTraitRefExt as _}; @@ -23,6 +23,7 @@ use rustc_trait_selection::traits::{ FulfillmentError, dyn_compatibility_violations_for_assoc_item, }; use smallvec::SmallVec; +use tracing::debug; use crate::errors::{ self, AssocItemConstraintsNotAllowedHere, ManualImplementation, MissingTypeParams, @@ -34,7 +35,7 @@ use crate::hir_ty_lowering::{AssocItemQSelf, HirTyLowerer}; impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { /// On missing type parameters, emit an E0393 error and provide a structured suggestion using /// the type parameter's name as a placeholder. - pub(crate) fn complain_about_missing_type_params( + pub(crate) fn report_missing_type_params( &self, missing_type_params: Vec, def_id: DefId, @@ -56,7 +57,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { /// When the code is using the `Fn` traits directly, instead of the `Fn(A) -> B` syntax, emit /// an error and attempt to build a reasonable structured suggestion. - pub(crate) fn complain_about_internal_fn_trait( + pub(crate) fn report_internal_fn_trait( &self, span: Span, trait_def_id: DefId, @@ -112,7 +113,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } } - pub(super) fn complain_about_assoc_item_not_found( + pub(super) fn report_unresolved_assoc_item( &self, all_candidates: impl Fn() -> I, qself: AssocItemQSelf, @@ -132,7 +133,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { .filter_by_name_unhygienic(assoc_ident.name) .find(|item| tcx.hygienic_eq(assoc_ident, item.ident(tcx), r.def_id())) }) { - return self.complain_about_assoc_kind_mismatch( + return self.report_assoc_kind_mismatch( assoc_item, assoc_tag, assoc_ident, @@ -331,7 +332,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { self.dcx().emit_err(err) } - fn complain_about_assoc_kind_mismatch( + fn report_assoc_kind_mismatch( &self, assoc_item: &ty::AssocItem, assoc_tag: ty::AssocTag, @@ -396,7 +397,173 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { }) } - pub(super) fn report_ambiguous_assoc( + pub(crate) fn report_missing_self_ty_for_resolved_path( + &self, + trait_def_id: DefId, + span: Span, + item_segment: &hir::PathSegment<'tcx>, + assoc_tag: ty::AssocTag, + ) -> ErrorGuaranteed { + let tcx = self.tcx(); + let path_str = tcx.def_path_str(trait_def_id); + + let def_id = self.item_def_id(); + debug!(item_def_id = ?def_id); + + // FIXME: document why/how this is different from `tcx.local_parent(def_id)` + let parent_def_id = tcx.hir_get_parent_item(tcx.local_def_id_to_hir_id(def_id)).to_def_id(); + debug!(?parent_def_id); + + // If the trait in segment is the same as the trait defining the item, + // use the `` syntax in the error. + let is_part_of_self_trait_constraints = def_id.to_def_id() == trait_def_id; + let is_part_of_fn_in_self_trait = parent_def_id == trait_def_id; + + let type_names = if is_part_of_self_trait_constraints || is_part_of_fn_in_self_trait { + vec!["Self".to_string()] + } else { + // Find all the types that have an `impl` for the trait. + tcx.all_impls(trait_def_id) + .filter_map(|impl_def_id| tcx.impl_trait_header(impl_def_id)) + .filter(|header| { + // Consider only accessible traits + tcx.visibility(trait_def_id).is_accessible_from(self.item_def_id(), tcx) + && header.polarity != ty::ImplPolarity::Negative + }) + .map(|header| header.trait_ref.instantiate_identity().self_ty()) + // We don't care about blanket impls. + .filter(|self_ty| !self_ty.has_non_region_param()) + .map(|self_ty| tcx.erase_regions(self_ty).to_string()) + .collect() + }; + // FIXME: also look at `tcx.generics_of(self.item_def_id()).params` any that + // references the trait. Relevant for the first case in + // `src/test/ui/associated-types/associated-types-in-ambiguous-context.rs` + self.report_ambiguous_assoc_item_path( + span, + &type_names, + &[path_str], + item_segment.ident.name, + assoc_tag, + ) + } + + pub(super) fn report_unresolved_type_relative_path( + &self, + self_ty: Ty<'tcx>, + hir_self_ty: &hir::Ty<'_>, + assoc_tag: ty::AssocTag, + ident: Ident, + qpath_hir_id: HirId, + span: Span, + variant_def_id: Option, + ) -> ErrorGuaranteed { + let tcx = self.tcx(); + let kind_str = assoc_tag_str(assoc_tag); + if variant_def_id.is_some() { + // Variant in type position + let msg = format!("expected {kind_str}, found variant `{ident}`"); + self.dcx().span_err(span, msg) + } else if self_ty.is_enum() { + let mut err = self.dcx().create_err(errors::NoVariantNamed { + span: ident.span, + ident, + ty: self_ty, + }); + + let adt_def = self_ty.ty_adt_def().expect("enum is not an ADT"); + if let Some(variant_name) = find_best_match_for_name( + &adt_def.variants().iter().map(|variant| variant.name).collect::>(), + ident.name, + None, + ) && let Some(variant) = adt_def.variants().iter().find(|s| s.name == variant_name) + { + let mut suggestion = vec![(ident.span, variant_name.to_string())]; + if let hir::Node::Stmt(&hir::Stmt { kind: hir::StmtKind::Semi(expr), .. }) + | hir::Node::Expr(expr) = tcx.parent_hir_node(qpath_hir_id) + && let hir::ExprKind::Struct(..) = expr.kind + { + match variant.ctor { + None => { + // struct + suggestion = vec![( + ident.span.with_hi(expr.span.hi()), + if variant.fields.is_empty() { + format!("{variant_name} {{}}") + } else { + format!( + "{variant_name} {{ {} }}", + variant + .fields + .iter() + .map(|f| format!("{}: /* value */", f.name)) + .collect::>() + .join(", ") + ) + }, + )]; + } + Some((hir::def::CtorKind::Fn, def_id)) => { + // tuple + let fn_sig = tcx.fn_sig(def_id).instantiate_identity(); + let inputs = fn_sig.inputs().skip_binder(); + suggestion = vec![( + ident.span.with_hi(expr.span.hi()), + format!( + "{variant_name}({})", + inputs + .iter() + .map(|i| format!("/* {i} */")) + .collect::>() + .join(", ") + ), + )]; + } + Some((hir::def::CtorKind::Const, _)) => { + // unit + suggestion = vec![( + ident.span.with_hi(expr.span.hi()), + variant_name.to_string(), + )]; + } + } + } + err.multipart_suggestion_verbose( + "there is a variant with a similar name", + suggestion, + Applicability::HasPlaceholders, + ); + } else { + err.span_label(ident.span, format!("variant not found in `{self_ty}`")); + } + + if let Some(sp) = tcx.hir_span_if_local(adt_def.did()) { + err.span_label(sp, format!("variant `{ident}` not found here")); + } + + err.emit() + } else if let Err(reported) = self_ty.error_reported() { + reported + } else { + match self.maybe_report_similar_assoc_fn(span, self_ty, hir_self_ty) { + Ok(()) => {} + Err(reported) => return reported, + } + + let traits: Vec<_> = self.probe_traits_that_match_assoc_ty(self_ty, ident); + + // Don't print `ty::Error` to the user. + self.report_ambiguous_assoc_item_path( + span, + &[self_ty.to_string()], + &traits, + ident.name, + assoc_tag, + ) + } + } + + pub(super) fn report_ambiguous_assoc_item_path( &self, span: Span, types: &[String], @@ -505,7 +672,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { err.emit() } - pub(crate) fn complain_about_ambiguous_inherent_assoc( + pub(crate) fn report_ambiguous_inherent_assoc_item( &self, name: Ident, candidates: Vec, @@ -518,12 +685,12 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { "multiple applicable items in scope" ); err.span_label(name.span, format!("multiple `{name}` found")); - self.note_ambiguous_inherent_assoc_ty(&mut err, candidates, span); + self.note_ambiguous_inherent_assoc_item(&mut err, candidates, span); err.emit() } // FIXME(fmease): Heavily adapted from `rustc_hir_typeck::method::suggest`. Deduplicate. - fn note_ambiguous_inherent_assoc_ty( + fn note_ambiguous_inherent_assoc_item( &self, err: &mut Diag<'_>, candidates: Vec, @@ -566,7 +733,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } // FIXME(inherent_associated_types): Find similarly named associated types and suggest them. - pub(crate) fn complain_about_inherent_assoc_not_found( + pub(crate) fn report_unresolved_inherent_assoc_item( &self, name: Ident, self_ty: Ty<'tcx>, @@ -1046,7 +1213,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } } - pub fn report_prohibit_generics_error<'a>( + pub fn report_prohibited_generic_args<'a>( &self, segments: impl Iterator> + Clone, args_visitors: impl Iterator> + Clone, @@ -1128,7 +1295,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { err.emit() } - pub fn report_trait_object_addition_traits_error( + pub fn report_trait_object_addition_traits( &self, regular_traits: &Vec<(ty::PolyTraitPredicate<'tcx>, SmallVec<[Span; 1]>)>, ) -> ErrorGuaranteed { @@ -1171,7 +1338,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { err.emit() } - pub fn report_trait_object_with_no_traits_error( + pub fn report_trait_object_with_no_traits( &self, span: Span, user_written_clauses: impl IntoIterator, Span)>, diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index 06756bd3ee683..bb5c1543441dc 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -44,16 +44,14 @@ use rustc_middle::ty::{ use rustc_middle::{bug, span_bug}; use rustc_session::lint::builtin::AMBIGUOUS_ASSOCIATED_ITEMS; use rustc_session::parse::feature_err; -use rustc_span::edit_distance::find_best_match_for_name; -use rustc_span::{DUMMY_SP, Ident, Span, Symbol, kw, sym}; +use rustc_span::{DUMMY_SP, Ident, Span, kw, sym}; use rustc_trait_selection::infer::InferCtxtExt; use rustc_trait_selection::traits::wf::object_region_bounds; use rustc_trait_selection::traits::{self, ObligationCtxt}; use tracing::{debug, instrument}; -use self::errors::assoc_tag_str; use crate::check::check_abi_fn_ptr; -use crate::errors::{AmbiguousLifetimeBound, BadReturnTypeNotation, NoVariantNamed}; +use crate::errors::{AmbiguousLifetimeBound, BadReturnTypeNotation}; use crate::hir_ty_lowering::errors::{GenericsArgsErrExtend, prohibit_assoc_item_constraint}; use crate::hir_ty_lowering::generics::{check_generic_arg_count, lower_generic_args}; use crate::middle::resolve_bound_vars as rbv; @@ -751,7 +749,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { trait_ref.path.segments.split_last().unwrap().1.iter(), GenericsArgsErrExtend::None, ); - self.complain_about_internal_fn_trait(span, trait_def_id, trait_segment, false); + self.report_internal_fn_trait(span, trait_def_id, trait_segment, false); let (generic_args, arg_count) = self.lower_generic_args_of_path( trait_ref.path.span, @@ -926,7 +924,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { trait_segment: &hir::PathSegment<'tcx>, is_impl: bool, ) -> ty::TraitRef<'tcx> { - self.complain_about_internal_fn_trait(span, trait_def_id, trait_segment, is_impl); + self.report_internal_fn_trait(span, trait_def_id, trait_segment, is_impl); let (generic_args, _) = self.lower_generic_args_of_path(span, trait_def_id, &[], trait_segment, Some(self_ty)); @@ -1032,15 +1030,14 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { }); let Some(bound) = matching_candidates.next() else { - let reported = self.complain_about_assoc_item_not_found( + return Err(self.report_unresolved_assoc_item( all_candidates, qself, assoc_tag, assoc_ident, span, constraint, - ); - return Err(reported); + )); }; debug!(?bound); @@ -1326,113 +1323,15 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { span, )?, _ => { - let kind_str = assoc_tag_str(mode.assoc_tag()); - let reported = if variant_def_id.is_some() { - // Variant in type position - let msg = format!("expected {kind_str}, found variant `{ident}`"); - self.dcx().span_err(span, msg) - } else if self_ty.is_enum() { - let mut err = self.dcx().create_err(NoVariantNamed { - span: ident.span, - ident, - ty: self_ty, - }); - - let adt_def = self_ty.ty_adt_def().expect("enum is not an ADT"); - if let Some(variant_name) = find_best_match_for_name( - &adt_def - .variants() - .iter() - .map(|variant| variant.name) - .collect::>(), - ident.name, - None, - ) && let Some(variant) = - adt_def.variants().iter().find(|s| s.name == variant_name) - { - let mut suggestion = vec![(ident.span, variant_name.to_string())]; - if let hir::Node::Stmt(&hir::Stmt { - kind: hir::StmtKind::Semi(expr), .. - }) - | hir::Node::Expr(expr) = tcx.parent_hir_node(qpath_hir_id) - && let hir::ExprKind::Struct(..) = expr.kind - { - match variant.ctor { - None => { - // struct - suggestion = vec![( - ident.span.with_hi(expr.span.hi()), - if variant.fields.is_empty() { - format!("{variant_name} {{}}") - } else { - format!( - "{variant_name} {{ {} }}", - variant - .fields - .iter() - .map(|f| format!("{}: /* value */", f.name)) - .collect::>() - .join(", ") - ) - }, - )]; - } - Some((hir::def::CtorKind::Fn, def_id)) => { - // tuple - let fn_sig = tcx.fn_sig(def_id).instantiate_identity(); - let inputs = fn_sig.inputs().skip_binder(); - suggestion = vec![( - ident.span.with_hi(expr.span.hi()), - format!( - "{variant_name}({})", - inputs - .iter() - .map(|i| format!("/* {i} */")) - .collect::>() - .join(", ") - ), - )]; - } - Some((hir::def::CtorKind::Const, _)) => { - // unit - suggestion = vec![( - ident.span.with_hi(expr.span.hi()), - variant_name.to_string(), - )]; - } - } - } - err.multipart_suggestion_verbose( - "there is a variant with a similar name", - suggestion, - Applicability::HasPlaceholders, - ); - } else { - err.span_label(ident.span, format!("variant not found in `{self_ty}`")); - } - - if let Some(sp) = tcx.hir_span_if_local(adt_def.did()) { - err.span_label(sp, format!("variant `{ident}` not found here")); - } - - err.emit() - } else if let Err(reported) = self_ty.error_reported() { - reported - } else { - self.maybe_report_similar_assoc_fn(span, self_ty, hir_self_ty)?; - - let traits: Vec<_> = self.probe_traits_that_match_assoc_ty(self_ty, ident); - - // Don't print `ty::Error` to the user. - self.report_ambiguous_assoc( - span, - &[self_ty.to_string()], - &traits, - ident.name, - mode.assoc_tag(), - ) - }; - return Err(reported); + return Err(self.report_unresolved_type_relative_path( + self_ty, + hir_self_ty, + mode.assoc_tag(), + ident, + qpath_hir_id, + span, + variant_def_id, + )); } }; @@ -1626,7 +1525,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { .collect(); match &applicable_candidates[..] { - &[] => Err(self.complain_about_inherent_assoc_not_found( + &[] => Err(self.report_unresolved_inherent_assoc_item( name, self_ty, candidates, @@ -1637,7 +1536,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { &[applicable_candidate] => Ok(applicable_candidate), - &[_, ..] => Err(self.complain_about_ambiguous_inherent_assoc( + &[_, ..] => Err(self.report_ambiguous_inherent_assoc_item( name, applicable_candidates.into_iter().map(|(_, (candidate, _))| candidate).collect(), span, @@ -1833,7 +1732,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { debug!(?trait_def_id); let Some(self_ty) = opt_self_ty else { - return Err(self.error_missing_qpath_self_ty( + return Err(self.report_missing_self_ty_for_resolved_path( trait_def_id, span, item_segment, @@ -1852,57 +1751,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { Ok((item_def_id, item_args)) } - fn error_missing_qpath_self_ty( - &self, - trait_def_id: DefId, - span: Span, - item_segment: &hir::PathSegment<'tcx>, - assoc_tag: ty::AssocTag, - ) -> ErrorGuaranteed { - let tcx = self.tcx(); - let path_str = tcx.def_path_str(trait_def_id); - - let def_id = self.item_def_id(); - debug!(item_def_id = ?def_id); - - // FIXME: document why/how this is different from `tcx.local_parent(def_id)` - let parent_def_id = tcx.hir_get_parent_item(tcx.local_def_id_to_hir_id(def_id)).to_def_id(); - debug!(?parent_def_id); - - // If the trait in segment is the same as the trait defining the item, - // use the `` syntax in the error. - let is_part_of_self_trait_constraints = def_id.to_def_id() == trait_def_id; - let is_part_of_fn_in_self_trait = parent_def_id == trait_def_id; - - let type_names = if is_part_of_self_trait_constraints || is_part_of_fn_in_self_trait { - vec!["Self".to_string()] - } else { - // Find all the types that have an `impl` for the trait. - tcx.all_impls(trait_def_id) - .filter_map(|impl_def_id| tcx.impl_trait_header(impl_def_id)) - .filter(|header| { - // Consider only accessible traits - tcx.visibility(trait_def_id).is_accessible_from(self.item_def_id(), tcx) - && header.polarity != ty::ImplPolarity::Negative - }) - .map(|header| header.trait_ref.instantiate_identity().self_ty()) - // We don't care about blanket impls. - .filter(|self_ty| !self_ty.has_non_region_param()) - .map(|self_ty| tcx.erase_regions(self_ty).to_string()) - .collect() - }; - // FIXME: also look at `tcx.generics_of(self.item_def_id()).params` any that - // references the trait. Relevant for the first case in - // `src/test/ui/associated-types/associated-types-in-ambiguous-context.rs` - self.report_ambiguous_assoc( - span, - &type_names, - &[path_str], - item_segment.ident.name, - assoc_tag, - ) - } - pub fn prohibit_generic_args<'a>( &self, segments: impl Iterator> + Clone, @@ -1911,7 +1759,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { let args_visitors = segments.clone().flat_map(|segment| segment.args().args); let mut result = Ok(()); if let Some(_) = args_visitors.clone().next() { - result = Err(self.report_prohibit_generics_error( + result = Err(self.report_prohibited_generic_args( segments.clone(), args_visitors, err_extend, From 9e1832998d0c47ee11cd28e0458e68d9e7186b0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Wed, 23 Apr 2025 20:43:59 +0200 Subject: [PATCH 12/23] Factor out `resolve_type_relative_path` IMPORTANT: This leads to a tiny diagnostic regression that will be fixed in the next commit! --- .../src/hir_ty_lowering/bounds.rs | 141 +++++------------- .../src/hir_ty_lowering/mod.rs | 123 +++++++++------ .../path-non-param-qself.stderr | 18 +++ 3 files changed, 130 insertions(+), 152 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs index 6aadefc30274c..106420faa4c01 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs @@ -4,9 +4,9 @@ use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; use rustc_errors::codes::*; use rustc_errors::struct_span_code_err; use rustc_hir as hir; +use rustc_hir::AmbigArg; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId}; -use rustc_hir::{AmbigArg, HirId}; use rustc_middle::bug; use rustc_middle::ty::{ self as ty, IsSuggestable, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, @@ -713,118 +713,51 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { Err(guar) => Ty::new_error(tcx, guar), } } - hir::QPath::TypeRelative(qself, item_segment) - if item_segment.args.is_some_and(|args| { + hir::QPath::TypeRelative(hir_self_ty, segment) + if segment.args.is_some_and(|args| { matches!(args.parenthesized, hir::GenericArgsParentheses::ReturnTypeNotation) }) => { - match self - .resolve_type_relative_return_type_notation( - qself, - item_segment, - hir_ty.hir_id, - hir_ty.span, - ) - .and_then(|(candidate, item_def_id)| { - self.lower_return_type_notation_ty(candidate, item_def_id, hir_ty.span) - }) { - Ok(ty) => Ty::new_alias(tcx, ty::Projection, ty), - Err(guar) => Ty::new_error(tcx, guar), - } - } - _ => self.lower_ty(hir_ty), - } - } - - /// Perform type-dependent lookup for a *method* for return type notation. - /// This generally mirrors `::lower_type_relative_path`. - fn resolve_type_relative_return_type_notation( - &self, - qself: &'tcx hir::Ty<'tcx>, - item_segment: &'tcx hir::PathSegment<'tcx>, - qpath_hir_id: HirId, - span: Span, - ) -> Result<(ty::PolyTraitRef<'tcx>, DefId), ErrorGuaranteed> { - let tcx = self.tcx(); - let qself_ty = self.lower_ty(qself); - let assoc_ident = item_segment.ident; - let qself_res = if let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = &qself.kind { - path.res - } else { - Res::Err - }; - - let bound = match (qself_ty.kind(), qself_res) { - (_, Res::SelfTyAlias { alias_to: impl_def_id, is_trait_impl: true, .. }) => { - // `Self` in an impl of a trait -- we have a concrete self type and a - // trait reference. - let Some(trait_ref) = tcx.impl_trait_ref(impl_def_id) else { - // A cycle error occurred, most likely. - self.dcx().span_bug(span, "expected cycle error"); - }; - - self.probe_single_bound_for_assoc_item( - || { - traits::supertraits( - tcx, - ty::Binder::dummy(trait_ref.instantiate_identity()), - ) - }, - AssocItemQSelf::SelfTyAlias, + let self_ty = self.lower_ty(hir_self_ty); + let (item_def_id, bound) = match self.resolve_type_relative_path( + self_ty, + hir_self_ty, ty::AssocTag::Fn, - assoc_ident, - span, + segment, + hir_ty.hir_id, + hir_ty.span, None, - )? - } - ( - &ty::Param(_), - Res::SelfTyParam { trait_: param_did } | Res::Def(DefKind::TyParam, param_did), - ) => self.probe_single_ty_param_bound_for_assoc_item( - param_did.expect_local(), - qself.span, - ty::AssocTag::Fn, - assoc_ident, - span, - )?, - _ => { - if let Err(reported) = qself_ty.error_reported() { - return Err(reported); - } else { - // FIXME(return_type_notation): Provide some structured suggestion here. - let err = struct_span_code_err!( - self.dcx(), - span, - E0223, - "ambiguous associated function" + ) { + Ok(result) => result, + Err(guar) => return Ty::new_error(tcx, guar), + }; + + // Don't let `T::method` resolve to some `for<'a> >::method`, + // which may happen via a higher-ranked where clause or supertrait. + // This is the same restrictions as associated types; even though we could + // support it, it just makes things a lot more difficult to support in + // `resolve_bound_vars`, since we'd need to introduce those as elided + // bound vars on the where clause too. + if bound.has_bound_vars() { + return Ty::new_error( + tcx, + self.dcx().emit_err(errors::AssociatedItemTraitUninferredGenericParams { + span: hir_ty.span, + inferred_sugg: Some(hir_ty.span.with_hi(segment.ident.span.lo())), + bound: format!("{}::", tcx.anonymize_bound_vars(bound).skip_binder()), + mpart_sugg: None, + what: tcx.def_descr(item_def_id), + }), ); - return Err(err.emit()); } - } - }; - - let trait_def_id = bound.def_id(); - let assoc_fn = self - .probe_assoc_item(assoc_ident, ty::AssocTag::Fn, qpath_hir_id, span, trait_def_id) - .expect("failed to find associated fn"); - // Don't let `T::method` resolve to some `for<'a> >::method`, - // which may happen via a higher-ranked where clause or supertrait. - // This is the same restrictions as associated types; even though we could - // support it, it just makes things a lot more difficult to support in - // `resolve_bound_vars`, since we'd need to introduce those as elided - // bound vars on the where clause too. - if bound.has_bound_vars() { - return Err(self.dcx().emit_err(errors::AssociatedItemTraitUninferredGenericParams { - span, - inferred_sugg: Some(span.with_hi(item_segment.ident.span.lo())), - bound: format!("{}::", tcx.anonymize_bound_vars(bound).skip_binder(),), - mpart_sugg: None, - what: assoc_fn.descr(), - })); + match self.lower_return_type_notation_ty(bound, item_def_id, hir_ty.span) { + Ok(ty) => Ty::new_alias(tcx, ty::Projection, ty), + Err(guar) => Ty::new_error(tcx, guar), + } + } + _ => self.lower_ty(hir_ty), } - - Ok((bound, assoc_fn.def_id)) } /// Do the common parts of lowering an RTN type. This involves extending the diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index bb5c1543441dc..6b21bbbfcd809 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -38,8 +38,8 @@ use rustc_middle::middle::stability::AllowUnstable; use rustc_middle::mir::interpret::LitToConstInput; use rustc_middle::ty::print::PrintPolyTraitRefExt as _; use rustc_middle::ty::{ - self, AssocTag, Const, GenericArgKind, GenericArgsRef, GenericParamDefKind, ParamEnv, Ty, - TyCtxt, TypeVisitableExt, TypingMode, Upcast, fold_regions, + self, Const, GenericArgKind, GenericArgsRef, GenericParamDefKind, ParamEnv, Ty, TyCtxt, + TypeVisitableExt, TypingMode, Upcast, fold_regions, }; use rustc_middle::{bug, span_bug}; use rustc_session::lint::builtin::AMBIGUOUS_ASSOCIATED_ITEMS; @@ -937,7 +937,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { fn probe_trait_that_defines_assoc_item( &self, trait_def_id: DefId, - assoc_tag: AssocTag, + assoc_tag: ty::AssocTag, assoc_ident: Ident, ) -> bool { self.tcx() @@ -980,7 +980,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { &self, ty_param_def_id: LocalDefId, ty_param_span: Span, - assoc_tag: AssocTag, + assoc_tag: ty::AssocTag, assoc_ident: Ident, span: Span, ) -> Result, ErrorGuaranteed> { @@ -1015,7 +1015,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { &self, all_candidates: impl Fn() -> I, qself: AssocItemQSelf, - assoc_tag: AssocTag, + assoc_tag: ty::AssocTag, assoc_ident: Ident, span: Span, constraint: Option<&hir::AssocItemConstraint<'tcx>>, @@ -1238,7 +1238,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { ) -> Result, ErrorGuaranteed> { debug!(%self_ty, ?segment.ident); let tcx = self.tcx(); - let ident = segment.ident; // Check if we have an enum variant or an inherent associated type. let mut variant_def_id = None; @@ -1247,7 +1246,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { let variant_def = adt_def .variants() .iter() - .find(|vd| tcx.hygienic_eq(ident, vd.ident(tcx), adt_def.did())); + .find(|vd| tcx.hygienic_eq(segment.ident, vd.ident(tcx), adt_def.did())); if let Some(variant_def) = variant_def { if let PermitVariants::Yes = mode.permit_variants() { tcx.check_stability(variant_def.def_id, Some(qpath_hir_id), span, None); @@ -1282,13 +1281,70 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } } + let (item_def_id, bound) = self.resolve_type_relative_path( + self_ty, + hir_self_ty, + mode.assoc_tag(), + segment, + qpath_hir_id, + span, + variant_def_id, + )?; + + let (item_def_id, args) = self.lower_assoc_item_path(span, item_def_id, segment, bound)?; + + if let Some(variant_def_id) = variant_def_id { + tcx.node_span_lint(AMBIGUOUS_ASSOCIATED_ITEMS, qpath_hir_id, span, |lint| { + lint.primary_message("ambiguous associated item"); + let mut could_refer_to = |kind: DefKind, def_id, also| { + let note_msg = format!( + "`{}` could{} refer to the {} defined here", + segment.ident, + also, + tcx.def_kind_descr(kind, def_id) + ); + lint.span_note(tcx.def_span(def_id), note_msg); + }; + + could_refer_to(DefKind::Variant, variant_def_id, ""); + could_refer_to(mode.def_kind(), item_def_id, " also"); + + lint.span_suggestion( + span, + "use fully-qualified syntax", + format!( + "<{} as {}>::{}", + self_ty, + tcx.item_name(bound.def_id()), + segment.ident + ), + Applicability::MachineApplicable, + ); + }); + } + + Ok(TypeRelativePath::AssocItem(item_def_id, args)) + } + + /// Resolve a [type-relative](hir::QPath::TypeRelative) (and type-level) path. + fn resolve_type_relative_path( + &self, + self_ty: Ty<'tcx>, + hir_self_ty: &'tcx hir::Ty<'tcx>, + assoc_tag: ty::AssocTag, + segment: &'tcx hir::PathSegment<'tcx>, + qpath_hir_id: HirId, + span: Span, + variant_def_id: Option, + ) -> Result<(DefId, ty::PolyTraitRef<'tcx>), ErrorGuaranteed> { + let tcx = self.tcx(); + let self_ty_res = match hir_self_ty.kind { hir::TyKind::Path(hir::QPath::Resolved(_, path)) => path.res, _ => Res::Err, }; - // Find the type of the associated item, and the trait where the associated - // item is declared. + // Find the type of the assoc item, and the trait where the associated item is declared. let bound = match (self_ty.kind(), self_ty_res) { (_, Res::SelfTyAlias { alias_to: impl_def_id, is_trait_impl: true, .. }) => { // `Self` in an impl of a trait -- we have a concrete self type and a @@ -1300,14 +1356,12 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { self.probe_single_bound_for_assoc_item( || { - traits::supertraits( - tcx, - ty::Binder::dummy(trait_ref.instantiate_identity()), - ) + let trait_ref = ty::Binder::dummy(trait_ref.instantiate_identity()); + traits::supertraits(tcx, trait_ref) }, AssocItemQSelf::SelfTyAlias, - mode.assoc_tag(), - ident, + assoc_tag, + segment.ident, span, None, )? @@ -1318,16 +1372,16 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { ) => self.probe_single_ty_param_bound_for_assoc_item( param_did.expect_local(), hir_self_ty.span, - mode.assoc_tag(), - ident, + assoc_tag, + segment.ident, span, )?, _ => { return Err(self.report_unresolved_type_relative_path( self_ty, hir_self_ty, - mode.assoc_tag(), - ident, + assoc_tag, + segment.ident, qpath_hir_id, span, variant_def_id, @@ -1335,38 +1389,11 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } }; - let trait_def_id = bound.def_id(); let assoc_item = self - .probe_assoc_item(ident, mode.assoc_tag(), qpath_hir_id, span, trait_def_id) + .probe_assoc_item(segment.ident, assoc_tag, qpath_hir_id, span, bound.def_id()) .expect("failed to find associated item"); - let (def_id, args) = self.lower_assoc_item_path(span, assoc_item.def_id, segment, bound)?; - let result = TypeRelativePath::AssocItem(def_id, args); - - if let Some(variant_def_id) = variant_def_id { - tcx.node_span_lint(AMBIGUOUS_ASSOCIATED_ITEMS, qpath_hir_id, span, |lint| { - lint.primary_message("ambiguous associated item"); - let mut could_refer_to = |kind: DefKind, def_id, also| { - let note_msg = format!( - "`{}` could{} refer to the {} defined here", - ident, - also, - tcx.def_kind_descr(kind, def_id) - ); - lint.span_note(tcx.def_span(def_id), note_msg); - }; - could_refer_to(DefKind::Variant, variant_def_id, ""); - could_refer_to(mode.def_kind(), assoc_item.def_id, " also"); - - lint.span_suggestion( - span, - "use fully-qualified syntax", - format!("<{} as {}>::{}", self_ty, tcx.item_name(trait_def_id), ident), - Applicability::MachineApplicable, - ); - }); - } - Ok(result) + Ok((assoc_item.def_id, bound)) } /// Search for inherent associated items for use at the type level. diff --git a/tests/ui/associated-type-bounds/return-type-notation/path-non-param-qself.stderr b/tests/ui/associated-type-bounds/return-type-notation/path-non-param-qself.stderr index 38202bdbf0727..6b7ded3f28552 100644 --- a/tests/ui/associated-type-bounds/return-type-notation/path-non-param-qself.stderr +++ b/tests/ui/associated-type-bounds/return-type-notation/path-non-param-qself.stderr @@ -3,18 +3,36 @@ error[E0223]: ambiguous associated function | LL | <()>::method(..): Send, | ^^^^^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated function `method` implemented for `()`, you could use the fully-qualified path + | +LL - <()>::method(..): Send, +LL + <() as Example>::method: Send, + | error[E0223]: ambiguous associated function --> $DIR/path-non-param-qself.rs:13:5 | LL | i32::method(..): Send, | ^^^^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated function `method` implemented for `i32`, you could use the fully-qualified path + | +LL - i32::method(..): Send, +LL + ::method: Send, + | error[E0223]: ambiguous associated function --> $DIR/path-non-param-qself.rs:15:5 | LL | Adt::method(..): Send, | ^^^^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated function `method` implemented for `Adt`, you could use the fully-qualified path + | +LL - Adt::method(..): Send, +LL + ::method: Send, + | error: aborting due to 3 previous errors From 8c37c8c3e6133ae431bc4a5aec1c7b21a3134969 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Wed, 23 Apr 2025 21:14:25 +0200 Subject: [PATCH 13/23] Preserve generic args in suggestions for ambiguous associated items Most notably, this preserves the `(..)` of ambiguous RTN paths. --- .../src/hir_ty_lowering/errors.rs | 53 ++++++++++--------- ...fers-shadowing-trait-item.uncovered.stderr | 8 ++- ...guous-associated-type-with-generics.stderr | 7 ++- .../associated-item-duplicate-names-3.stderr | 8 ++- .../return-type-notation/path-no-qself.stderr | 2 +- .../path-non-param-qself.stderr | 9 ++-- ...sociated-types-in-ambiguous-context.stderr | 16 +++++- tests/ui/error-codes/E0223.stderr | 8 ++- tests/ui/lint/bare-trait-objects-path.stderr | 8 ++- .../qualified/qualified-path-params-2.stderr | 2 +- tests/ui/self/self-impl.stderr | 16 +++++- .../struct-path-associated-type.stderr | 24 +++++++-- tests/ui/traits/item-privacy.stderr | 16 +++++- .../ice-unexpected-region-123863.stderr | 2 +- tests/ui/typeck/issue-107087.stderr | 8 ++- 15 files changed, 140 insertions(+), 47 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs index 60dc84716c73b..45fee0fa4024e 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs @@ -3,7 +3,8 @@ use rustc_data_structures::sorted_map::SortedMap; use rustc_data_structures::unord::UnordMap; use rustc_errors::codes::*; use rustc_errors::{ - Applicability, Diag, ErrorGuaranteed, MultiSpan, listify, pluralize, struct_span_code_err, + Applicability, Diag, ErrorGuaranteed, MultiSpan, SuggestionStyle, listify, pluralize, + struct_span_code_err, }; use rustc_hir::def::{CtorOf, DefKind, Res}; use rustc_hir::def_id::DefId; @@ -443,7 +444,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { span, &type_names, &[path_str], - item_segment.ident.name, + item_segment.ident, assoc_tag, ) } @@ -552,12 +553,11 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { let traits: Vec<_> = self.probe_traits_that_match_assoc_ty(self_ty, ident); - // Don't print `ty::Error` to the user. self.report_ambiguous_assoc_item_path( span, &[self_ty.to_string()], &traits, - ident.name, + ident, assoc_tag, ) } @@ -568,7 +568,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { span: Span, types: &[String], traits: &[String], - name: Symbol, + ident: Ident, assoc_tag: ty::AssocTag, ) -> ErrorGuaranteed { let kind_str = assoc_tag_str(assoc_tag); @@ -588,6 +588,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { Applicability::MachineApplicable, ); } else { + let sugg_sp = span.until(ident.span); + let mut types = types.to_vec(); types.sort(); let mut traits = traits.to_vec(); @@ -595,76 +597,79 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { match (&types[..], &traits[..]) { ([], []) => { err.span_suggestion_verbose( - span, + sugg_sp, format!( "if there were a type named `Type` that implements a trait named \ - `Trait` with associated {kind_str} `{name}`, you could use the \ + `Trait` with associated {kind_str} `{ident}`, you could use the \ fully-qualified path", ), - format!("::{name}"), + "::", Applicability::HasPlaceholders, ); } ([], [trait_str]) => { err.span_suggestion_verbose( - span, + sugg_sp, format!( "if there were a type named `Example` that implemented `{trait_str}`, \ you could use the fully-qualified path", ), - format!("::{name}"), + format!("::"), Applicability::HasPlaceholders, ); } ([], traits) => { - err.span_suggestions( - span, + err.span_suggestions_with_style( + sugg_sp, format!( "if there were a type named `Example` that implemented one of the \ - traits with associated {kind_str} `{name}`, you could use the \ + traits with associated {kind_str} `{ident}`, you could use the \ fully-qualified path", ), - traits.iter().map(|trait_str| format!("::{name}")), + traits.iter().map(|trait_str| format!("::")), Applicability::HasPlaceholders, + SuggestionStyle::ShowAlways, ); } ([type_str], []) => { err.span_suggestion_verbose( - span, + sugg_sp, format!( - "if there were a trait named `Example` with associated {kind_str} `{name}` \ + "if there were a trait named `Example` with associated {kind_str} `{ident}` \ implemented for `{type_str}`, you could use the fully-qualified path", ), - format!("<{type_str} as Example>::{name}"), + format!("<{type_str} as Example>::"), Applicability::HasPlaceholders, ); } (types, []) => { - err.span_suggestions( - span, + err.span_suggestions_with_style( + sugg_sp, format!( - "if there were a trait named `Example` with associated {kind_str} `{name}` \ + "if there were a trait named `Example` with associated {kind_str} `{ident}` \ implemented for one of the types, you could use the fully-qualified \ path", ), types .into_iter() - .map(|type_str| format!("<{type_str} as Example>::{name}")), + .map(|type_str| format!("<{type_str} as Example>::")), Applicability::HasPlaceholders, + SuggestionStyle::ShowAlways, ); } (types, traits) => { let mut suggestions = vec![]; for type_str in types { for trait_str in traits { - suggestions.push(format!("<{type_str} as {trait_str}>::{name}")); + suggestions.push(format!("<{type_str} as {trait_str}>::")); } } - err.span_suggestions( - span, + err.span_suggestions_with_style( + sugg_sp, "use fully-qualified syntax", suggestions, Applicability::MachineApplicable, + SuggestionStyle::ShowAlways, ); } } diff --git a/tests/ui/associated-inherent-types/not-found-self-type-differs-shadowing-trait-item.uncovered.stderr b/tests/ui/associated-inherent-types/not-found-self-type-differs-shadowing-trait-item.uncovered.stderr index 978305c2ce355..3e914e0538d0b 100644 --- a/tests/ui/associated-inherent-types/not-found-self-type-differs-shadowing-trait-item.uncovered.stderr +++ b/tests/ui/associated-inherent-types/not-found-self-type-differs-shadowing-trait-item.uncovered.stderr @@ -2,7 +2,13 @@ error[E0223]: ambiguous associated type --> $DIR/not-found-self-type-differs-shadowing-trait-item.rs:28:12 | LL | let _: S::::Pr = (); - | ^^^^^^^^^^^^^ help: use fully-qualified syntax: ` as Tr>::Pr` + | ^^^^^^^^^^^^^ + | +help: use fully-qualified syntax + | +LL - let _: S::::Pr = (); +LL + let _: as Tr>::Pr = (); + | error: aborting due to 1 previous error diff --git a/tests/ui/associated-item/ambiguous-associated-type-with-generics.stderr b/tests/ui/associated-item/ambiguous-associated-type-with-generics.stderr index 9e1dd73980792..fcf6b4c3e7361 100644 --- a/tests/ui/associated-item/ambiguous-associated-type-with-generics.stderr +++ b/tests/ui/associated-item/ambiguous-associated-type-with-generics.stderr @@ -2,7 +2,12 @@ error[E0223]: ambiguous associated type --> $DIR/ambiguous-associated-type-with-generics.rs:13:13 | LL | let _x: >::Ty; - | ^^^^^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: ` as Assoc>::Ty` + | ^^^^^^^^^^^^^^^^^^^^ + | +help: use fully-qualified syntax + | +LL | let _x: as Assoc>::Ty; + | ++++++++ error: aborting due to 1 previous error diff --git a/tests/ui/associated-item/associated-item-duplicate-names-3.stderr b/tests/ui/associated-item/associated-item-duplicate-names-3.stderr index a2346e292ac40..84a9da0998832 100644 --- a/tests/ui/associated-item/associated-item-duplicate-names-3.stderr +++ b/tests/ui/associated-item/associated-item-duplicate-names-3.stderr @@ -13,7 +13,13 @@ error[E0223]: ambiguous associated type --> $DIR/associated-item-duplicate-names-3.rs:18:12 | LL | let x: Baz::Bar = 5; - | ^^^^^^^^ help: use fully-qualified syntax: `::Bar` + | ^^^^^^^^ + | +help: use fully-qualified syntax + | +LL - let x: Baz::Bar = 5; +LL + let x: ::Bar = 5; + | error: aborting due to 2 previous errors diff --git a/tests/ui/associated-type-bounds/return-type-notation/path-no-qself.stderr b/tests/ui/associated-type-bounds/return-type-notation/path-no-qself.stderr index aad6dfc496b75..a13fdbda1bf7a 100644 --- a/tests/ui/associated-type-bounds/return-type-notation/path-no-qself.stderr +++ b/tests/ui/associated-type-bounds/return-type-notation/path-no-qself.stderr @@ -7,7 +7,7 @@ LL | Trait::method(..): Send, help: if there were a type named `Example` that implemented `Trait`, you could use the fully-qualified path | LL - Trait::method(..): Send, -LL + ::method: Send, +LL + ::method(..): Send, | error: aborting due to 1 previous error diff --git a/tests/ui/associated-type-bounds/return-type-notation/path-non-param-qself.stderr b/tests/ui/associated-type-bounds/return-type-notation/path-non-param-qself.stderr index 6b7ded3f28552..4c4c2c24079be 100644 --- a/tests/ui/associated-type-bounds/return-type-notation/path-non-param-qself.stderr +++ b/tests/ui/associated-type-bounds/return-type-notation/path-non-param-qself.stderr @@ -6,9 +6,8 @@ LL | <()>::method(..): Send, | help: if there were a trait named `Example` with associated function `method` implemented for `()`, you could use the fully-qualified path | -LL - <()>::method(..): Send, -LL + <() as Example>::method: Send, - | +LL | <() as Example>::method(..): Send, + | ++++++++++ error[E0223]: ambiguous associated function --> $DIR/path-non-param-qself.rs:13:5 @@ -19,7 +18,7 @@ LL | i32::method(..): Send, help: if there were a trait named `Example` with associated function `method` implemented for `i32`, you could use the fully-qualified path | LL - i32::method(..): Send, -LL + ::method: Send, +LL + ::method(..): Send, | error[E0223]: ambiguous associated function @@ -31,7 +30,7 @@ LL | Adt::method(..): Send, help: if there were a trait named `Example` with associated function `method` implemented for `Adt`, you could use the fully-qualified path | LL - Adt::method(..): Send, -LL + ::method: Send, +LL + ::method(..): Send, | error: aborting due to 3 previous errors diff --git a/tests/ui/associated-types/associated-types-in-ambiguous-context.stderr b/tests/ui/associated-types/associated-types-in-ambiguous-context.stderr index 1be8db5ddf44f..a7647cf26aadf 100644 --- a/tests/ui/associated-types/associated-types-in-ambiguous-context.stderr +++ b/tests/ui/associated-types/associated-types-in-ambiguous-context.stderr @@ -14,7 +14,13 @@ error[E0223]: ambiguous associated type --> $DIR/associated-types-in-ambiguous-context.rs:22:17 | LL | trait Foo where Foo::Assoc: Bar { - | ^^^^^^^^^^ help: use fully-qualified syntax: `::Assoc` + | ^^^^^^^^^^ + | +help: use fully-qualified syntax + | +LL - trait Foo where Foo::Assoc: Bar { +LL + trait Foo where ::Assoc: Bar { + | error[E0223]: ambiguous associated type --> $DIR/associated-types-in-ambiguous-context.rs:27:10 @@ -42,7 +48,13 @@ error[E0223]: ambiguous associated type --> $DIR/associated-types-in-ambiguous-context.rs:13:23 | LL | fn grab(&self) -> Grab::Value; - | ^^^^^^^^^^^ help: use fully-qualified syntax: `::Value` + | ^^^^^^^^^^^ + | +help: use fully-qualified syntax + | +LL - fn grab(&self) -> Grab::Value; +LL + fn grab(&self) -> ::Value; + | error[E0223]: ambiguous associated type --> $DIR/associated-types-in-ambiguous-context.rs:16:22 diff --git a/tests/ui/error-codes/E0223.stderr b/tests/ui/error-codes/E0223.stderr index e985a4c9bf0d9..fbfdce5689a70 100644 --- a/tests/ui/error-codes/E0223.stderr +++ b/tests/ui/error-codes/E0223.stderr @@ -2,7 +2,13 @@ error[E0223]: ambiguous associated type --> $DIR/E0223.rs:8:14 | LL | let foo: MyTrait::X; - | ^^^^^^^^^^ help: use fully-qualified syntax: `::X` + | ^^^^^^^^^^ + | +help: use fully-qualified syntax + | +LL - let foo: MyTrait::X; +LL + let foo: ::X; + | error: aborting due to 1 previous error diff --git a/tests/ui/lint/bare-trait-objects-path.stderr b/tests/ui/lint/bare-trait-objects-path.stderr index fbb647c37c5ae..e611abd31f3b4 100644 --- a/tests/ui/lint/bare-trait-objects-path.stderr +++ b/tests/ui/lint/bare-trait-objects-path.stderr @@ -55,7 +55,13 @@ error[E0223]: ambiguous associated type --> $DIR/bare-trait-objects-path.rs:23:12 | LL | let _: Dyn::Ty; - | ^^^^^^^ help: use fully-qualified syntax: `::Ty` + | ^^^^^^^ + | +help: use fully-qualified syntax + | +LL - let _: Dyn::Ty; +LL + let _: ::Ty; + | error: aborting due to 1 previous error; 4 warnings emitted diff --git a/tests/ui/qualified/qualified-path-params-2.stderr b/tests/ui/qualified/qualified-path-params-2.stderr index 6641e81013f05..e70cdbdc3f49e 100644 --- a/tests/ui/qualified/qualified-path-params-2.stderr +++ b/tests/ui/qualified/qualified-path-params-2.stderr @@ -7,7 +7,7 @@ LL | type A = ::A::f; help: if there were a trait named `Example` with associated type `f` implemented for `::A`, you could use the fully-qualified path | LL - type A = ::A::f; -LL + type A = <::A as Example>::f; +LL + type A = <::A as Example>::f; | error: aborting due to 1 previous error diff --git a/tests/ui/self/self-impl.stderr b/tests/ui/self/self-impl.stderr index 18ffd15427f81..1bda307d0eb3a 100644 --- a/tests/ui/self/self-impl.stderr +++ b/tests/ui/self/self-impl.stderr @@ -2,13 +2,25 @@ error[E0223]: ambiguous associated type --> $DIR/self-impl.rs:23:16 | LL | let _: ::Baz = true; - | ^^^^^^^^^^^ help: use fully-qualified syntax: `::Baz` + | ^^^^^^^^^^^ + | +help: use fully-qualified syntax + | +LL - let _: ::Baz = true; +LL + let _: ::Baz = true; + | error[E0223]: ambiguous associated type --> $DIR/self-impl.rs:25:16 | LL | let _: Self::Baz = true; - | ^^^^^^^^^ help: use fully-qualified syntax: `::Baz` + | ^^^^^^^^^ + | +help: use fully-qualified syntax + | +LL - let _: Self::Baz = true; +LL + let _: ::Baz = true; + | error: aborting due to 2 previous errors diff --git a/tests/ui/structs/struct-path-associated-type.stderr b/tests/ui/structs/struct-path-associated-type.stderr index de396e875b0cf..110362293ac32 100644 --- a/tests/ui/structs/struct-path-associated-type.stderr +++ b/tests/ui/structs/struct-path-associated-type.stderr @@ -48,19 +48,37 @@ error[E0223]: ambiguous associated type --> $DIR/struct-path-associated-type.rs:32:13 | LL | let s = S::A {}; - | ^^^^ help: use fully-qualified syntax: `::A` + | ^^^^ + | +help: use fully-qualified syntax + | +LL - let s = S::A {}; +LL + let s = ::A {}; + | error[E0223]: ambiguous associated type --> $DIR/struct-path-associated-type.rs:33:13 | LL | let z = S::A:: {}; - | ^^^^ help: use fully-qualified syntax: `::A` + | ^^^^ + | +help: use fully-qualified syntax + | +LL - let z = S::A:: {}; +LL + let z = ::A:: {}; + | error[E0223]: ambiguous associated type --> $DIR/struct-path-associated-type.rs:35:9 | LL | S::A {} => {} - | ^^^^ help: use fully-qualified syntax: `::A` + | ^^^^ + | +help: use fully-qualified syntax + | +LL - S::A {} => {} +LL + ::A {} => {} + | error: aborting due to 8 previous errors diff --git a/tests/ui/traits/item-privacy.stderr b/tests/ui/traits/item-privacy.stderr index 4fd9ef9119257..1d3d8cb98437c 100644 --- a/tests/ui/traits/item-privacy.stderr +++ b/tests/ui/traits/item-privacy.stderr @@ -230,13 +230,25 @@ error[E0223]: ambiguous associated type --> $DIR/item-privacy.rs:119:12 | LL | let _: S::B; - | ^^^^ help: use fully-qualified syntax: `::B` + | ^^^^ + | +help: use fully-qualified syntax + | +LL - let _: S::B; +LL + let _: ::B; + | error[E0223]: ambiguous associated type --> $DIR/item-privacy.rs:120:12 | LL | let _: S::C; - | ^^^^ help: use fully-qualified syntax: `::C` + | ^^^^ + | +help: use fully-qualified syntax + | +LL - let _: S::C; +LL + let _: ::C; + | error[E0624]: associated type `A` is private --> $DIR/item-privacy.rs:122:12 diff --git a/tests/ui/typeck/ice-unexpected-region-123863.stderr b/tests/ui/typeck/ice-unexpected-region-123863.stderr index 8a4d767c14350..e5050b4d3167a 100644 --- a/tests/ui/typeck/ice-unexpected-region-123863.stderr +++ b/tests/ui/typeck/ice-unexpected-region-123863.stderr @@ -39,7 +39,7 @@ LL | Inner::concat_strs::<"a">::A help: if there were a trait named `Example` with associated type `concat_strs` implemented for `Inner<_>`, you could use the fully-qualified path | LL - Inner::concat_strs::<"a">::A -LL + as Example>::concat_strs::A +LL + as Example>::concat_strs::<"a">::A | error: aborting due to 3 previous errors diff --git a/tests/ui/typeck/issue-107087.stderr b/tests/ui/typeck/issue-107087.stderr index 289c8d161ae86..157ba5a167219 100644 --- a/tests/ui/typeck/issue-107087.stderr +++ b/tests/ui/typeck/issue-107087.stderr @@ -2,7 +2,13 @@ error[E0223]: ambiguous associated type --> $DIR/issue-107087.rs:16:5 | LL | A::B::<>::C - | ^^^^^^^^ help: use fully-qualified syntax: ` as Foo>::B` + | ^^^^^^^^ + | +help: use fully-qualified syntax + | +LL - A::B::<>::C +LL + as Foo>::B::<>::C + | error: aborting due to 1 previous error From c9b6ccc11ce82c753702716f57f786acf322e64f Mon Sep 17 00:00:00 2001 From: mejrs <59372212+mejrs@users.noreply.github.com> Date: Sat, 17 May 2025 12:50:37 +0200 Subject: [PATCH 14/23] Switch library rustc_unimplemented to use `Self` and `This` --- library/core/src/convert/mod.rs | 2 +- library/core/src/fmt/mod.rs | 10 ++--- library/core/src/iter/traits/collect.rs | 33 +++++++--------- library/core/src/iter/traits/iterator.rs | 4 +- library/core/src/marker.rs | 50 ++++++++++++------------ library/core/src/ops/arith.rs | 4 +- library/core/src/ops/function.rs | 6 +-- library/core/src/ops/index.rs | 6 +-- library/core/src/ops/try_trait.rs | 18 ++++----- library/core/src/slice/index.rs | 2 +- library/std/src/process.rs | 2 +- 11 files changed, 67 insertions(+), 70 deletions(-) diff --git a/library/core/src/convert/mod.rs b/library/core/src/convert/mod.rs index e1b10e1074d27..aeafc1be3598f 100644 --- a/library/core/src/convert/mod.rs +++ b/library/core/src/convert/mod.rs @@ -575,7 +575,7 @@ pub trait Into: Sized { #[rustc_diagnostic_item = "From"] #[stable(feature = "rust1", since = "1.0.0")] #[rustc_on_unimplemented(on( - all(_Self = "&str", T = "alloc::string::String"), + all(Self = "&str", T = "alloc::string::String"), note = "to coerce a `{T}` into a `{Self}`, use `&*` as a prefix", ))] #[doc(search_unbox)] diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 4f7f8a5b84dd5..5978cb660f6b3 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -856,10 +856,10 @@ impl Display for Arguments<'_> { on( crate_local, label = "`{Self}` cannot be formatted using `{{:?}}`", - note = "add `#[derive(Debug)]` to `{Self}` or manually `impl {Debug} for {Self}`" + note = "add `#[derive(Debug)]` to `{Self}` or manually `impl {This} for {Self}`" ), - message = "`{Self}` doesn't implement `{Debug}`", - label = "`{Self}` cannot be formatted using `{{:?}}` because it doesn't implement `{Debug}`" + message = "`{Self}` doesn't implement `{This}`", + label = "`{Self}` cannot be formatted using `{{:?}}` because it doesn't implement `{This}`" )] #[doc(alias = "{:?}")] #[rustc_diagnostic_item = "Debug"] @@ -969,12 +969,12 @@ pub use macros::Debug; /// ``` #[rustc_on_unimplemented( on( - any(_Self = "std::path::Path", _Self = "std::path::PathBuf"), + any(Self = "std::path::Path", Self = "std::path::PathBuf"), label = "`{Self}` cannot be formatted with the default formatter; call `.display()` on it", note = "call `.display()` or `.to_string_lossy()` to safely print paths, \ as they may contain non-Unicode data" ), - message = "`{Self}` doesn't implement `{Display}`", + message = "`{Self}` doesn't implement `{This}`", label = "`{Self}` cannot be formatted with the default formatter", note = "in format strings you may be able to use `{{:?}}` (or {{:#?}} for pretty-print) instead" )] diff --git a/library/core/src/iter/traits/collect.rs b/library/core/src/iter/traits/collect.rs index 97bb21c8a36e8..3bc9cff8072bf 100644 --- a/library/core/src/iter/traits/collect.rs +++ b/library/core/src/iter/traits/collect.rs @@ -97,32 +97,32 @@ use super::TrustedLen; #[stable(feature = "rust1", since = "1.0.0")] #[rustc_on_unimplemented( on( - _Self = "&[{A}]", + Self = "&[{A}]", message = "a slice of type `{Self}` cannot be built since we need to store the elements somewhere", label = "try explicitly collecting into a `Vec<{A}>`", ), on( - all(A = "{integer}", any(_Self = "&[{integral}]",)), + all(A = "{integer}", any(Self = "&[{integral}]",)), message = "a slice of type `{Self}` cannot be built since we need to store the elements somewhere", label = "try explicitly collecting into a `Vec<{A}>`", ), on( - _Self = "[{A}]", + Self = "[{A}]", message = "a slice of type `{Self}` cannot be built since `{Self}` has no definite size", label = "try explicitly collecting into a `Vec<{A}>`", ), on( - all(A = "{integer}", any(_Self = "[{integral}]",)), + all(A = "{integer}", any(Self = "[{integral}]",)), message = "a slice of type `{Self}` cannot be built since `{Self}` has no definite size", label = "try explicitly collecting into a `Vec<{A}>`", ), on( - _Self = "[{A}; _]", + Self = "[{A}; _]", message = "an array of type `{Self}` cannot be built directly from an iterator", label = "try collecting into a `Vec<{A}>`, then using `.try_into()`", ), on( - all(A = "{integer}", any(_Self = "[{integral}; _]",)), + all(A = "{integer}", any(Self = "[{integral}; _]",)), message = "an array of type `{Self}` cannot be built directly from an iterator", label = "try collecting into a `Vec<{A}>`, then using `.try_into()`", ), @@ -239,41 +239,38 @@ pub trait FromIterator: Sized { #[rustc_diagnostic_item = "IntoIterator"] #[rustc_on_unimplemented( on( - _Self = "core::ops::range::RangeTo", + Self = "core::ops::range::RangeTo", label = "if you meant to iterate until a value, add a starting value", note = "`..end` is a `RangeTo`, which cannot be iterated on; you might have meant to have a \ bounded `Range`: `0..end`" ), on( - _Self = "core::ops::range::RangeToInclusive", + Self = "core::ops::range::RangeToInclusive", label = "if you meant to iterate until a value (including it), add a starting value", note = "`..=end` is a `RangeToInclusive`, which cannot be iterated on; you might have meant \ to have a bounded `RangeInclusive`: `0..=end`" ), on( - _Self = "[]", + Self = "[]", label = "`{Self}` is not an iterator; try calling `.into_iter()` or `.iter()`" ), - on(_Self = "&[]", label = "`{Self}` is not an iterator; try calling `.iter()`"), + on(Self = "&[]", label = "`{Self}` is not an iterator; try calling `.iter()`"), on( - _Self = "alloc::vec::Vec", + Self = "alloc::vec::Vec", label = "`{Self}` is not an iterator; try calling `.into_iter()` or `.iter()`" ), + on(Self = "&str", label = "`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`"), on( - _Self = "&str", + Self = "alloc::string::String", label = "`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`" ), on( - _Self = "alloc::string::String", - label = "`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`" - ), - on( - _Self = "{integral}", + Self = "{integral}", note = "if you want to iterate between `start` until a value `end`, use the exclusive range \ syntax `start..end` or the inclusive range syntax `start..=end`" ), on( - _Self = "{float}", + Self = "{float}", note = "if you want to iterate between `start` until a value `end`, use the exclusive range \ syntax `start..end` or the inclusive range syntax `start..=end`" ), diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index d1e71f0e60f2a..cf85bdb1352ba 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -22,11 +22,11 @@ fn _assert_is_dyn_compatible(_: &dyn Iterator) {} #[stable(feature = "rust1", since = "1.0.0")] #[rustc_on_unimplemented( on( - _Self = "core::ops::range::RangeTo", + Self = "core::ops::range::RangeTo", note = "you might have meant to use a bounded `Range`" ), on( - _Self = "core::ops::range::RangeToInclusive", + Self = "core::ops::range::RangeToInclusive", note = "you might have meant to use a bounded `RangeInclusive`" ), label = "`{Self}` is not an iterator", diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index f33b8d188d86c..700fb0f386eed 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -550,72 +550,72 @@ pub trait BikeshedGuaranteedNoDrop {} #[lang = "sync"] #[rustc_on_unimplemented( on( - _Self = "core::cell::once::OnceCell", + Self = "core::cell::once::OnceCell", note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::OnceLock` instead" ), on( - _Self = "core::cell::Cell", + Self = "core::cell::Cell", note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU8` instead", ), on( - _Self = "core::cell::Cell", + Self = "core::cell::Cell", note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU16` instead", ), on( - _Self = "core::cell::Cell", + Self = "core::cell::Cell", note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU32` instead", ), on( - _Self = "core::cell::Cell", + Self = "core::cell::Cell", note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU64` instead", ), on( - _Self = "core::cell::Cell", + Self = "core::cell::Cell", note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicUsize` instead", ), on( - _Self = "core::cell::Cell", + Self = "core::cell::Cell", note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI8` instead", ), on( - _Self = "core::cell::Cell", + Self = "core::cell::Cell", note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI16` instead", ), on( - _Self = "core::cell::Cell", + Self = "core::cell::Cell", note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI32` instead", ), on( - _Self = "core::cell::Cell", + Self = "core::cell::Cell", note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI64` instead", ), on( - _Self = "core::cell::Cell", + Self = "core::cell::Cell", note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicIsize` instead", ), on( - _Self = "core::cell::Cell", + Self = "core::cell::Cell", note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicBool` instead", ), on( all( - _Self = "core::cell::Cell", - not(_Self = "core::cell::Cell"), - not(_Self = "core::cell::Cell"), - not(_Self = "core::cell::Cell"), - not(_Self = "core::cell::Cell"), - not(_Self = "core::cell::Cell"), - not(_Self = "core::cell::Cell"), - not(_Self = "core::cell::Cell"), - not(_Self = "core::cell::Cell"), - not(_Self = "core::cell::Cell"), - not(_Self = "core::cell::Cell"), - not(_Self = "core::cell::Cell") + Self = "core::cell::Cell", + not(Self = "core::cell::Cell"), + not(Self = "core::cell::Cell"), + not(Self = "core::cell::Cell"), + not(Self = "core::cell::Cell"), + not(Self = "core::cell::Cell"), + not(Self = "core::cell::Cell"), + not(Self = "core::cell::Cell"), + not(Self = "core::cell::Cell"), + not(Self = "core::cell::Cell"), + not(Self = "core::cell::Cell"), + not(Self = "core::cell::Cell") ), note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock`", ), on( - _Self = "core::cell::RefCell", + Self = "core::cell::RefCell", note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead", ), message = "`{Self}` cannot be shared between threads safely", diff --git a/library/core/src/ops/arith.rs b/library/core/src/ops/arith.rs index 54d79beca95ab..098ce4531f0c0 100644 --- a/library/core/src/ops/arith.rs +++ b/library/core/src/ops/arith.rs @@ -67,8 +67,8 @@ #[stable(feature = "rust1", since = "1.0.0")] #[rustc_const_unstable(feature = "const_ops", issue = "90080")] #[rustc_on_unimplemented( - on(all(_Self = "{integer}", Rhs = "{float}"), message = "cannot add a float to an integer",), - on(all(_Self = "{float}", Rhs = "{integer}"), message = "cannot add an integer to a float",), + on(all(Self = "{integer}", Rhs = "{float}"), message = "cannot add a float to an integer",), + on(all(Self = "{float}", Rhs = "{integer}"), message = "cannot add an integer to a float",), message = "cannot add `{Rhs}` to `{Self}`", label = "no implementation for `{Self} + {Rhs}`", append_const_msg diff --git a/library/core/src/ops/function.rs b/library/core/src/ops/function.rs index e9014458b48ea..df48c104410ca 100644 --- a/library/core/src/ops/function.rs +++ b/library/core/src/ops/function.rs @@ -62,7 +62,7 @@ use crate::marker::Tuple; note = "wrap the `{Self}` in a closure with no arguments: `|| {{ /* code */ }}`" ), on( - _Self = "unsafe fn", + Self = "unsafe fn", note = "unsafe function cannot be called generically without an unsafe block", // SAFETY: tidy is not smart enough to tell that the below unsafe block is a string label = "call the function in a closure: `|| unsafe {{ /* code */ }}`" @@ -149,7 +149,7 @@ pub trait Fn: FnMut { note = "wrap the `{Self}` in a closure with no arguments: `|| {{ /* code */ }}`" ), on( - _Self = "unsafe fn", + Self = "unsafe fn", note = "unsafe function cannot be called generically without an unsafe block", // SAFETY: tidy is not smart enough to tell that the below unsafe block is a string label = "call the function in a closure: `|| unsafe {{ /* code */ }}`" @@ -228,7 +228,7 @@ pub trait FnMut: FnOnce { note = "wrap the `{Self}` in a closure with no arguments: `|| {{ /* code */ }}`" ), on( - _Self = "unsafe fn", + Self = "unsafe fn", note = "unsafe function cannot be called generically without an unsafe block", // SAFETY: tidy is not smart enough to tell that the below unsafe block is a string label = "call the function in a closure: `|| unsafe {{ /* code */ }}`" diff --git a/library/core/src/ops/index.rs b/library/core/src/ops/index.rs index 46e19bed43aba..8092fa9eb2fc5 100644 --- a/library/core/src/ops/index.rs +++ b/library/core/src/ops/index.rs @@ -144,17 +144,17 @@ pub trait Index { #[lang = "index_mut"] #[rustc_on_unimplemented( on( - _Self = "&str", + Self = "&str", note = "you can use `.chars().nth()` or `.bytes().nth()` see chapter in The Book " ), on( - _Self = "str", + Self = "str", note = "you can use `.chars().nth()` or `.bytes().nth()` see chapter in The Book " ), on( - _Self = "alloc::string::String", + Self = "alloc::string::String", note = "you can use `.chars().nth()` or `.bytes().nth()` see chapter in The Book " ), diff --git a/library/core/src/ops/try_trait.rs b/library/core/src/ops/try_trait.rs index 3ba2957526f9c..bac8ffb074ba8 100644 --- a/library/core/src/ops/try_trait.rs +++ b/library/core/src/ops/try_trait.rs @@ -117,12 +117,12 @@ use crate::ops::ControlFlow; on( all(from_desugaring = "TryBlock"), message = "a `try` block must return `Result` or `Option` \ - (or another type that implements `{Try}`)", + (or another type that implements `{This}`)", label = "could not wrap the final value of the block as `{Self}` doesn't implement `Try`", ), on( all(from_desugaring = "QuestionMark"), - message = "the `?` operator can only be applied to values that implement `{Try}`", + message = "the `?` operator can only be applied to values that implement `{This}`", label = "the `?` operator cannot be applied to type `{Self}`" ) )] @@ -226,7 +226,7 @@ pub trait Try: FromResidual { on( all( from_desugaring = "QuestionMark", - _Self = "core::result::Result", + Self = "core::result::Result", R = "core::option::Option", ), message = "the `?` operator can only be used on `Result`s, not `Option`s, \ @@ -237,7 +237,7 @@ pub trait Try: FromResidual { on( all( from_desugaring = "QuestionMark", - _Self = "core::result::Result", + Self = "core::result::Result", ), // There's a special error message in the trait selection code for // `From` in `?`, so this is not shown for result-in-result errors, @@ -250,7 +250,7 @@ pub trait Try: FromResidual { on( all( from_desugaring = "QuestionMark", - _Self = "core::option::Option", + Self = "core::option::Option", R = "core::result::Result", ), message = "the `?` operator can only be used on `Option`s, not `Result`s, \ @@ -261,7 +261,7 @@ pub trait Try: FromResidual { on( all( from_desugaring = "QuestionMark", - _Self = "core::option::Option", + Self = "core::option::Option", ), // `Option`-in-`Option` always works, as there's only one possible // residual, so this can also be phrased strongly. @@ -273,7 +273,7 @@ pub trait Try: FromResidual { on( all( from_desugaring = "QuestionMark", - _Self = "core::ops::control_flow::ControlFlow", + Self = "core::ops::control_flow::ControlFlow", R = "core::ops::control_flow::ControlFlow", ), message = "the `?` operator in {ItemContext} that returns `ControlFlow` \ @@ -285,7 +285,7 @@ pub trait Try: FromResidual { on( all( from_desugaring = "QuestionMark", - _Self = "core::ops::control_flow::ControlFlow", + Self = "core::ops::control_flow::ControlFlow", // `R` is not a `ControlFlow`, as that case was matched previously ), message = "the `?` operator can only be used on `ControlFlow`s \ @@ -297,7 +297,7 @@ pub trait Try: FromResidual { all(from_desugaring = "QuestionMark"), message = "the `?` operator can only be used in {ItemContext} \ that returns `Result` or `Option` \ - (or another type that implements `{FromResidual}`)", + (or another type that implements `{This}`)", label = "cannot use the `?` operator in {ItemContext} that returns `{Self}`", parent_label = "this function should return `Result` or `Option` to accept `?`" ), diff --git a/library/core/src/slice/index.rs b/library/core/src/slice/index.rs index aafa19c0dd3d3..409bad9f06155 100644 --- a/library/core/src/slice/index.rs +++ b/library/core/src/slice/index.rs @@ -161,7 +161,7 @@ mod private_slice_index { #[rustc_on_unimplemented( on(T = "str", label = "string indices are ranges of `usize`",), on( - all(any(T = "str", T = "&str", T = "alloc::string::String"), _Self = "{integer}"), + all(any(T = "str", T = "&str", T = "alloc::string::String"), Self = "{integer}"), note = "you can use `.chars().nth()` or `.bytes().nth()`\n\ for more information, see chapter 8 in The Book: \ " diff --git a/library/std/src/process.rs b/library/std/src/process.rs index df6b9a6e563ce..359e208207252 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -2532,7 +2532,7 @@ pub fn id() -> u32 { #[rustc_on_unimplemented(on( cause = "MainFunctionType", message = "`main` has invalid return type `{Self}`", - label = "`main` can only return types that implement `{Termination}`" + label = "`main` can only return types that implement `{This}`" ))] pub trait Termination { /// Is called to get the representation of the value as status code. From 9ffd0bf75a30b4fce1ffa35732666a37a7e9a736 Mon Sep 17 00:00:00 2001 From: mejrs <59372212+mejrs@users.noreply.github.com> Date: Sat, 17 May 2025 15:15:53 +0200 Subject: [PATCH 15/23] do away with `_Self` and `TraitName` and check generic params for rustc_on_unimplemented --- compiler/rustc_span/src/symbol.rs | 1 - compiler/rustc_trait_selection/messages.ftl | 2 + .../traits/on_unimplemented.rs | 14 +++- .../traits/on_unimplemented_condition.rs | 37 +++++---- .../traits/on_unimplemented_format.rs | 76 ++----------------- compiler/rustc_trait_selection/src/errors.rs | 7 ++ .../clippy/tests/ui/duplicated_attributes.rs | 2 +- ...options_of_the_internal_rustc_attribute.rs | 6 +- ...ons_of_the_internal_rustc_attribute.stderr | 63 +++++++++------ ...o_not_fail_parsing_on_invalid_options_1.rs | 6 +- ...t_fail_parsing_on_invalid_options_1.stderr | 48 +++++++----- tests/ui/on-unimplemented/bad-annotation.rs | 12 ++- .../ui/on-unimplemented/bad-annotation.stderr | 14 +++- tests/ui/on-unimplemented/on-trait.rs | 2 +- 14 files changed, 152 insertions(+), 138 deletions(-) diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index efae6250b0720..bd0988b09d162 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -398,7 +398,6 @@ symbols! { Wrapping, Yield, _DECLS, - _Self, __D, __H, __S, diff --git a/compiler/rustc_trait_selection/messages.ftl b/compiler/rustc_trait_selection/messages.ftl index 00922c6038eee..9b949a0a79545 100644 --- a/compiler/rustc_trait_selection/messages.ftl +++ b/compiler/rustc_trait_selection/messages.ftl @@ -337,6 +337,8 @@ trait_selection_rustc_on_unimplemented_expected_one_predicate_in_not = expected .label = unexpected quantity of predicates here trait_selection_rustc_on_unimplemented_invalid_flag = invalid flag in `on`-clause .label = expected one of the `crate_local`, `direct` or `from_desugaring` flags, not `{$invalid_flag}` +trait_selection_rustc_on_unimplemented_invalid_name = invalid name in `on`-clause + .label = expected one of `cause`, `from_desugaring`, `Self` or any generic parameter of the trait, not `{$invalid_name}` trait_selection_rustc_on_unimplemented_invalid_predicate = this predicate is invalid .label = expected one of `any`, `all` or `not` here, not `{$invalid_pred}` trait_selection_rustc_on_unimplemented_missing_value = this attribute must have a value diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs index d5ee6e2123a19..37968386e9aeb 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs @@ -429,7 +429,19 @@ impl<'tcx> OnUnimplementedDirective { .next() .ok_or_else(|| tcx.dcx().emit_err(InvalidOnClause::Empty { span }))?; - match OnUnimplementedCondition::parse(cond) { + let generics: Vec = tcx + .generics_of(item_def_id) + .own_params + .iter() + .filter_map(|param| { + if matches!(param.kind, GenericParamDefKind::Lifetime) { + None + } else { + Some(param.name) + } + }) + .collect(); + match OnUnimplementedCondition::parse(cond, &generics) { Ok(condition) => Some(condition), Err(e) => return Err(tcx.dcx().emit_err(e)), } diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs index 13753761f0923..e8ea9f2d23ebd 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs @@ -26,9 +26,12 @@ impl OnUnimplementedCondition { }) } - pub(crate) fn parse(input: &MetaItemInner) -> Result { + pub(crate) fn parse( + input: &MetaItemInner, + generics: &[Symbol], + ) -> Result { let span = input.span(); - let pred = Predicate::parse(input)?; + let pred = Predicate::parse(input, generics)?; Ok(OnUnimplementedCondition { span, pred }) } } @@ -52,7 +55,7 @@ enum Predicate { } impl Predicate { - fn parse(input: &MetaItemInner) -> Result { + fn parse(input: &MetaItemInner, generics: &[Symbol]) -> Result { let meta_item = match input { MetaItemInner::MetaItem(meta_item) => meta_item, MetaItemInner::Lit(lit) => { @@ -69,10 +72,10 @@ impl Predicate { match meta_item.kind { MetaItemKind::List(ref mis) => match predicate.name { - sym::any => Ok(Predicate::Any(Predicate::parse_sequence(mis)?)), - sym::all => Ok(Predicate::All(Predicate::parse_sequence(mis)?)), + sym::any => Ok(Predicate::Any(Predicate::parse_sequence(mis, generics)?)), + sym::all => Ok(Predicate::All(Predicate::parse_sequence(mis, generics)?)), sym::not => match &**mis { - [one] => Ok(Predicate::Not(Box::new(Predicate::parse(one)?))), + [one] => Ok(Predicate::Not(Box::new(Predicate::parse(one, generics)?))), [first, .., last] => Err(InvalidOnClause::ExpectedOnePredInNot { span: first.span().to(last.span()), }), @@ -83,7 +86,7 @@ impl Predicate { } }, MetaItemKind::NameValue(MetaItemLit { symbol, .. }) => { - let name = Name::parse(predicate); + let name = Name::parse(predicate, generics)?; let value = FilterFormatString::parse(symbol); let kv = NameValue { name, value }; Ok(Predicate::Match(kv)) @@ -95,8 +98,11 @@ impl Predicate { } } - fn parse_sequence(sequence: &[MetaItemInner]) -> Result, InvalidOnClause> { - sequence.iter().map(Predicate::parse).collect() + fn parse_sequence( + sequence: &[MetaItemInner], + generics: &[Symbol], + ) -> Result, InvalidOnClause> { + sequence.iter().map(|item| Predicate::parse(item, generics)).collect() } fn eval(&self, eval: &mut impl FnMut(FlagOrNv<'_>) -> bool) -> bool { @@ -156,14 +162,13 @@ enum Name { } impl Name { - fn parse(Ident { name, .. }: Ident) -> Self { + fn parse(Ident { name, span }: Ident, generics: &[Symbol]) -> Result { match name { - sym::_Self | kw::SelfUpper => Name::SelfUpper, - sym::from_desugaring => Name::FromDesugaring, - sym::cause => Name::Cause, - // FIXME(mejrs) Perhaps we should start checking that - // this actually is a valid generic parameter? - generic => Name::GenericArg(generic), + kw::SelfUpper => Ok(Name::SelfUpper), + sym::from_desugaring => Ok(Name::FromDesugaring), + sym::cause => Ok(Name::Cause), + generic if generics.contains(&generic) => Ok(Name::GenericArg(generic)), + invalid_name => Err(InvalidOnClause::InvalidName { invalid_name, span }), } } } diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs index ce170f820e12f..7c1dfc1728f04 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs @@ -2,8 +2,8 @@ use std::fmt; use std::ops::Range; use errors::*; -use rustc_middle::ty::TyCtxt; use rustc_middle::ty::print::TraitRefPrintSugared; +use rustc_middle::ty::{GenericParamDefKind, TyCtxt}; use rustc_parse_format::{ Alignment, Argument, Count, FormatSpec, ParseError, ParseMode, Parser, Piece as RpfPiece, Position, @@ -232,48 +232,16 @@ fn parse_arg<'tcx>( ) -> FormatArg { let (Ctx::RustcOnUnimplemented { tcx, trait_def_id } | Ctx::DiagnosticOnUnimplemented { tcx, trait_def_id }) = ctx; - let trait_name = tcx.item_ident(*trait_def_id); - let generics = tcx.generics_of(trait_def_id); + let span = slice_span(input_span, arg.position_span.clone()); match arg.position { // Something like "hello {name}" Position::ArgumentNamed(name) => match (ctx, Symbol::intern(name)) { - // accepted, but deprecated - (Ctx::RustcOnUnimplemented { .. }, sym::_Self) => { - warnings - .push(FormatWarning::FutureIncompat { span, help: String::from("use {Self}") }); - FormatArg::SelfUpper - } - ( - Ctx::RustcOnUnimplemented { .. }, - sym::from_desugaring - | sym::crate_local - | sym::direct - | sym::cause - | sym::float - | sym::integer_ - | sym::integral, - ) => { - warnings.push(FormatWarning::FutureIncompat { - span, - help: String::from("don't use this in a format string"), - }); - FormatArg::AsIs(String::new()) - } - // Only `#[rustc_on_unimplemented]` can use these (Ctx::RustcOnUnimplemented { .. }, sym::ItemContext) => FormatArg::ItemContext, (Ctx::RustcOnUnimplemented { .. }, sym::This) => FormatArg::This, (Ctx::RustcOnUnimplemented { .. }, sym::Trait) => FormatArg::Trait, - // `{ThisTraitsName}`. Some attrs in std use this, but I'd like to change it to the more general `{This}` - // because that'll be simpler to parse and extend in the future - (Ctx::RustcOnUnimplemented { .. }, name) if name == trait_name.name => { - warnings - .push(FormatWarning::FutureIncompat { span, help: String::from("use {This}") }); - FormatArg::This - } - // Any attribute can use these ( Ctx::RustcOnUnimplemented { .. } | Ctx::DiagnosticOnUnimplemented { .. }, @@ -282,7 +250,10 @@ fn parse_arg<'tcx>( ( Ctx::RustcOnUnimplemented { .. } | Ctx::DiagnosticOnUnimplemented { .. }, generic_param, - ) if generics.own_params.iter().any(|param| param.name == generic_param) => { + ) if tcx.generics_of(trait_def_id).own_params.iter().any(|param| { + !matches!(param.kind, GenericParamDefKind::Lifetime) && param.name == generic_param + }) => + { FormatArg::GenericParam { generic_param } } @@ -375,39 +346,4 @@ pub mod errors { #[diag(trait_selection_missing_options_for_on_unimplemented_attr)] #[help] pub struct MissingOptionsForOnUnimplementedAttr; - - #[derive(LintDiagnostic)] - #[diag(trait_selection_ignored_diagnostic_option)] - pub struct IgnoredDiagnosticOption { - pub option_name: &'static str, - #[label] - pub span: Span, - #[label(trait_selection_other_label)] - pub prev_span: Span, - } - - impl IgnoredDiagnosticOption { - pub fn maybe_emit_warning<'tcx>( - tcx: TyCtxt<'tcx>, - item_def_id: DefId, - new: Option, - old: Option, - option_name: &'static str, - ) { - if let (Some(new_item), Some(old_item)) = (new, old) { - if let Some(item_def_id) = item_def_id.as_local() { - tcx.emit_node_span_lint( - UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, - tcx.local_def_id_to_hir_id(item_def_id), - new_item, - IgnoredDiagnosticOption { - span: new_item, - prev_span: old_item, - option_name, - }, - ); - } - } - } - } } diff --git a/compiler/rustc_trait_selection/src/errors.rs b/compiler/rustc_trait_selection/src/errors.rs index 8ab4d795c4597..779c861637a4e 100644 --- a/compiler/rustc_trait_selection/src/errors.rs +++ b/compiler/rustc_trait_selection/src/errors.rs @@ -72,6 +72,13 @@ pub enum InvalidOnClause { span: Span, invalid_flag: Symbol, }, + #[diag(trait_selection_rustc_on_unimplemented_invalid_name, code = E0232)] + InvalidName { + #[primary_span] + #[label] + span: Span, + invalid_name: Symbol, + }, } #[derive(Diagnostic)] diff --git a/src/tools/clippy/tests/ui/duplicated_attributes.rs b/src/tools/clippy/tests/ui/duplicated_attributes.rs index 874f5d22075c1..3ca91d6f1829c 100644 --- a/src/tools/clippy/tests/ui/duplicated_attributes.rs +++ b/src/tools/clippy/tests/ui/duplicated_attributes.rs @@ -21,7 +21,7 @@ fn foo() {} fn bar() {} // No warning: -#[rustc_on_unimplemented(on(_Self = "&str", label = "`a"), on(_Self = "alloc::string::String", label = "a"))] +#[rustc_on_unimplemented(on(Self = "&str", label = "`a"), on(Self = "alloc::string::String", label = "a"))] trait Abc {} #[proc_macro_attr::duplicated_attr()] // Should not warn! diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.rs b/tests/ui/diagnostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.rs index b76b550fcb246..a0e497fa045b3 100644 --- a/tests/ui/diagnostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.rs +++ b/tests/ui/diagnostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.rs @@ -3,7 +3,7 @@ //@ reference: attributes.diagnostic.on_unimplemented.syntax //@ reference: attributes.diagnostic.on_unimplemented.invalid-formats #[diagnostic::on_unimplemented( - on(_Self = "&str"), + on(Self = "&str"), //~^WARN malformed `on_unimplemented` attribute //~|WARN malformed `on_unimplemented` attribute message = "trait has `{Self}` and `{T}` as params", @@ -41,7 +41,7 @@ impl Bar for i32 {} //~|WARN there is no parameter `integral` on trait `Baz` //~|WARN there is no parameter `integer` on trait `Baz` //~|WARN there is no parameter `integer` on trait `Baz` - label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" + label = "{float}{_Self}{crate_local}{Trait}{ItemContext}{This}" //~^WARN there is no parameter `float` on trait `Baz` //~|WARN there is no parameter `float` on trait `Baz` //~|WARN there is no parameter `_Self` on trait `Baz` @@ -52,6 +52,8 @@ impl Bar for i32 {} //~|WARN there is no parameter `Trait` on trait `Baz` //~|WARN there is no parameter `ItemContext` on trait `Baz` //~|WARN there is no parameter `ItemContext` on trait `Baz` + //~|WARN there is no parameter `This` on trait `Baz` + //~|WARN there is no parameter `This` on trait `Baz` )] trait Baz {} diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.stderr b/tests/ui/diagnostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.stderr index 88816a98dcf00..8dace7d905225 100644 --- a/tests/ui/diagnostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.stderr +++ b/tests/ui/diagnostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.stderr @@ -9,8 +9,8 @@ LL | #[diagnostic::on_unimplemented(message = "Not allowed to apply it on a impl warning: malformed `on_unimplemented` attribute --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:6:5 | -LL | on(_Self = "&str"), - | ^^^^^^^^^^^^^^^^^^ invalid option found here +LL | on(Self = "&str"), + | ^^^^^^^^^^^^^^^^^ invalid option found here | = help: only `message`, `note` and `label` are allowed as options @@ -81,7 +81,7 @@ LL | message = "{from_desugaring}{direct}{cause}{integral}{integer}", warning: there is no parameter `float` on trait `Baz` --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:15 | -LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" +LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}{This}" | ^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument @@ -89,7 +89,7 @@ LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" warning: there is no parameter `_Self` on trait `Baz` --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:22 | -LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" +LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}{This}" | ^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument @@ -97,7 +97,7 @@ LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" warning: there is no parameter `crate_local` on trait `Baz` --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:29 | -LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" +LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}{This}" | ^^^^^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument @@ -105,7 +105,7 @@ LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" warning: there is no parameter `Trait` on trait `Baz` --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:42 | -LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" +LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}{This}" | ^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument @@ -113,16 +113,24 @@ LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" warning: there is no parameter `ItemContext` on trait `Baz` --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:49 | -LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" +LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}{This}" | ^^^^^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument +warning: there is no parameter `This` on trait `Baz` + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:62 + | +LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}{This}" + | ^^^^ + | + = help: expect either a generic argument name or `{Self}` as format argument + warning: malformed `on_unimplemented` attribute --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:6:5 | -LL | on(_Self = "&str"), - | ^^^^^^^^^^^^^^^^^^ invalid option found here +LL | on(Self = "&str"), + | ^^^^^^^^^^^^^^^^^ invalid option found here | = help: only `message`, `note` and `label` are allowed as options = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` @@ -146,7 +154,7 @@ LL | append_const_msg = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error[E0277]: trait has `()` and `i32` as params - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:63:15 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:65:15 | LL | takes_foo(()); | --------- ^^ trait has `()` and `i32` as params @@ -161,7 +169,7 @@ help: this trait has no implementations, consider adding one LL | trait Foo {} | ^^^^^^^^^^^^ note: required by a bound in `takes_foo` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:58:22 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:60:22 | LL | fn takes_foo(_: impl Foo) {} | ^^^^^^^^ required by this bound in `takes_foo` @@ -176,7 +184,7 @@ LL | #[diagnostic::on_unimplemented = "Message"] = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error[E0277]: the trait bound `(): Bar` is not satisfied - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:65:15 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:67:15 | LL | takes_bar(()); | --------- ^^ the trait `Bar` is not implemented for `()` @@ -185,7 +193,7 @@ LL | takes_bar(()); | = help: the trait `Bar` is implemented for `i32` note: required by a bound in `takes_bar` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:59:22 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:61:22 | LL | fn takes_bar(_: impl Bar) {} | ^^^ required by this bound in `takes_bar` @@ -238,7 +246,7 @@ LL | message = "{from_desugaring}{direct}{cause}{integral}{integer}", warning: there is no parameter `float` on trait `Baz` --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:15 | -LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" +LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}{This}" | ^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument @@ -247,7 +255,7 @@ LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" warning: there is no parameter `_Self` on trait `Baz` --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:22 | -LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" +LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}{This}" | ^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument @@ -256,7 +264,7 @@ LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" warning: there is no parameter `crate_local` on trait `Baz` --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:29 | -LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" +LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}{This}" | ^^^^^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument @@ -265,7 +273,7 @@ LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" warning: there is no parameter `Trait` on trait `Baz` --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:42 | -LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" +LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}{This}" | ^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument @@ -274,32 +282,41 @@ LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" warning: there is no parameter `ItemContext` on trait `Baz` --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:49 | -LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}" +LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}{This}" | ^^^^^^^^^^^ | = help: expect either a generic argument name or `{Self}` as format argument = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +warning: there is no parameter `This` on trait `Baz` + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:44:62 + | +LL | label = "{float}{_Self}{crate_local}{Trait}{ItemContext}{This}" + | ^^^^ + | + = help: expect either a generic argument name or `{Self}` as format argument + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + error[E0277]: {from_desugaring}{direct}{cause}{integral}{integer} - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:67:15 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:69:15 | LL | takes_baz(()); - | --------- ^^ {float}{_Self}{crate_local}{Trait}{ItemContext} + | --------- ^^ {float}{_Self}{crate_local}{Trait}{ItemContext}{This} | | | required by a bound introduced by this call | = help: the trait `Baz` is not implemented for `()` help: this trait has no implementations, consider adding one - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:56:1 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:58:1 | LL | trait Baz {} | ^^^^^^^^^ note: required by a bound in `takes_baz` - --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:60:22 + --> $DIR/do_not_accept_options_of_the_internal_rustc_attribute.rs:62:22 | LL | fn takes_baz(_: impl Baz) {} | ^^^ required by this bound in `takes_baz` -error: aborting due to 3 previous errors; 29 warnings emitted +error: aborting due to 3 previous errors; 31 warnings emitted For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.rs b/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.rs index 8328c10d2a077..08eb5707e909e 100644 --- a/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.rs +++ b/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.rs @@ -14,11 +14,15 @@ struct Bar {} //~|WARN malformed `on_unimplemented` attribute trait Baz {} -#[diagnostic::on_unimplemented(message = "Boom", on(_Self = "i32", message = "whatever"))] +#[diagnostic::on_unimplemented(message = "Boom", on(Self = "i32", message = "whatever"))] //~^WARN malformed `on_unimplemented` attribute //~|WARN malformed `on_unimplemented` attribute trait Boom {} +#[diagnostic::on_unimplemented(message = "Boom", on(_Self = "i32", message = "whatever"))] +//~^WARN malformed `on_unimplemented` attribute +trait _Self {} + #[diagnostic::on_unimplemented = "boom"] //~^WARN malformed `on_unimplemented` attribute trait Doom {} diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.stderr b/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.stderr index 4dd8c1afca020..80790dc3f792c 100644 --- a/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.stderr +++ b/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.stderr @@ -25,13 +25,21 @@ LL | #[diagnostic::on_unimplemented(message = "Boom", unsupported = "Bar")] warning: malformed `on_unimplemented` attribute --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:17:50 | +LL | #[diagnostic::on_unimplemented(message = "Boom", on(Self = "i32", message = "whatever"))] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid option found here + | + = help: only `message`, `note` and `label` are allowed as options + +warning: malformed `on_unimplemented` attribute + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:22:50 + | LL | #[diagnostic::on_unimplemented(message = "Boom", on(_Self = "i32", message = "whatever"))] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid option found here | = help: only `message`, `note` and `label` are allowed as options warning: malformed `on_unimplemented` attribute - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:22:32 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:26:32 | LL | #[diagnostic::on_unimplemented = "boom"] | ^^^^^^^^ invalid option found here @@ -39,7 +47,7 @@ LL | #[diagnostic::on_unimplemented = "boom"] = help: only `message`, `note` and `label` are allowed as options warning: missing options for `on_unimplemented` attribute - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:26:1 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:30:1 | LL | #[diagnostic::on_unimplemented] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -47,7 +55,7 @@ LL | #[diagnostic::on_unimplemented] = help: at least one of the `message`, `note` and `label` options are expected warning: there is no parameter `DoesNotExist` on trait `Test` - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:31:44 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:35:44 | LL | #[diagnostic::on_unimplemented(message = "{DoesNotExist}")] | ^^^^^^^^^^^^ @@ -64,7 +72,7 @@ LL | #[diagnostic::on_unimplemented(unsupported = "foo")] = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error[E0277]: the trait bound `i32: Foo` is not satisfied - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:43:14 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:47:14 | LL | take_foo(1_i32); | -------- ^^^^^ the trait `Foo` is not implemented for `i32` @@ -77,7 +85,7 @@ help: this trait has no implementations, consider adding one LL | trait Foo {} | ^^^^^^^^^ note: required by a bound in `take_foo` - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:36:21 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:40:21 | LL | fn take_foo(_: impl Foo) {} | ^^^ required by this bound in `take_foo` @@ -92,7 +100,7 @@ LL | #[diagnostic::on_unimplemented(message = "Boom", unsupported = "Bar")] = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error[E0277]: Boom - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:45:14 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:49:14 | LL | take_baz(1_i32); | -------- ^^^^^ the trait `Baz` is not implemented for `i32` @@ -105,7 +113,7 @@ help: this trait has no implementations, consider adding one LL | trait Baz {} | ^^^^^^^^^ note: required by a bound in `take_baz` - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:37:21 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:41:21 | LL | fn take_baz(_: impl Baz) {} | ^^^ required by this bound in `take_baz` @@ -113,14 +121,14 @@ LL | fn take_baz(_: impl Baz) {} warning: malformed `on_unimplemented` attribute --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:17:50 | -LL | #[diagnostic::on_unimplemented(message = "Boom", on(_Self = "i32", message = "whatever"))] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid option found here +LL | #[diagnostic::on_unimplemented(message = "Boom", on(Self = "i32", message = "whatever"))] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid option found here | = help: only `message`, `note` and `label` are allowed as options = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error[E0277]: Boom - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:47:15 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:51:15 | LL | take_boom(1_i32); | --------- ^^^^^ the trait `Boom` is not implemented for `i32` @@ -133,13 +141,13 @@ help: this trait has no implementations, consider adding one LL | trait Boom {} | ^^^^^^^^^^ note: required by a bound in `take_boom` - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:38:22 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:42:22 | LL | fn take_boom(_: impl Boom) {} | ^^^^ required by this bound in `take_boom` warning: missing options for `on_unimplemented` attribute - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:26:1 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:30:1 | LL | #[diagnostic::on_unimplemented] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -148,7 +156,7 @@ LL | #[diagnostic::on_unimplemented] = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error[E0277]: the trait bound `i32: Whatever` is not satisfied - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:49:19 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:53:19 | LL | take_whatever(1_i32); | ------------- ^^^^^ the trait `Whatever` is not implemented for `i32` @@ -156,18 +164,18 @@ LL | take_whatever(1_i32); | required by a bound introduced by this call | help: this trait has no implementations, consider adding one - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:29:1 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:33:1 | LL | trait Whatever {} | ^^^^^^^^^^^^^^ note: required by a bound in `take_whatever` - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:39:26 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:43:26 | LL | fn take_whatever(_: impl Whatever) {} | ^^^^^^^^ required by this bound in `take_whatever` warning: there is no parameter `DoesNotExist` on trait `Test` - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:31:44 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:35:44 | LL | #[diagnostic::on_unimplemented(message = "{DoesNotExist}")] | ^^^^^^^^^^^^ @@ -176,7 +184,7 @@ LL | #[diagnostic::on_unimplemented(message = "{DoesNotExist}")] = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error[E0277]: {DoesNotExist} - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:51:15 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:55:15 | LL | take_test(()); | --------- ^^ the trait `Test` is not implemented for `()` @@ -184,16 +192,16 @@ LL | take_test(()); | required by a bound introduced by this call | help: this trait has no implementations, consider adding one - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:34:1 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:38:1 | LL | trait Test {} | ^^^^^^^^^^ note: required by a bound in `take_test` - --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:40:22 + --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:44:22 | LL | fn take_test(_: impl Test) {} | ^^^^ required by this bound in `take_test` -error: aborting due to 5 previous errors; 12 warnings emitted +error: aborting due to 5 previous errors; 13 warnings emitted For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/on-unimplemented/bad-annotation.rs b/tests/ui/on-unimplemented/bad-annotation.rs index 25de597811027..937e5b82da50c 100644 --- a/tests/ui/on-unimplemented/bad-annotation.rs +++ b/tests/ui/on-unimplemented/bad-annotation.rs @@ -59,7 +59,7 @@ trait EmptyOn {} //~^^^ NOTE expected value here trait ExpectedPredicateInOn {} -#[rustc_on_unimplemented(on(x = "y"), message = "y")] +#[rustc_on_unimplemented(on(Self = "y"), message = "y")] trait OnWithoutDirectives {} #[rustc_on_unimplemented(on(from_desugaring, on(from_desugaring, message = "x")), message = "y")] @@ -107,3 +107,13 @@ trait InvalidPredicate {} //~^ ERROR invalid flag in `on`-clause //~^^ NOTE expected one of the `crate_local`, `direct` or `from_desugaring` flags, not `something` trait InvalidFlag {} + +#[rustc_on_unimplemented(on(_Self = "y", message = "y"))] +//~^ ERROR invalid name in `on`-clause +//~^^ NOTE expected one of `cause`, `from_desugaring`, `Self` or any generic parameter of the trait, not `_Self` +trait InvalidName {} + +#[rustc_on_unimplemented(on(abc = "y", message = "y"))] +//~^ ERROR invalid name in `on`-clause +//~^^ NOTE expected one of `cause`, `from_desugaring`, `Self` or any generic parameter of the trait, not `abc` +trait InvalidName2 {} diff --git a/tests/ui/on-unimplemented/bad-annotation.stderr b/tests/ui/on-unimplemented/bad-annotation.stderr index 35b919c7b7853..3fc5453277404 100644 --- a/tests/ui/on-unimplemented/bad-annotation.stderr +++ b/tests/ui/on-unimplemented/bad-annotation.stderr @@ -125,7 +125,19 @@ error[E0232]: invalid flag in `on`-clause LL | #[rustc_on_unimplemented(on(something, message = "y"))] | ^^^^^^^^^ expected one of the `crate_local`, `direct` or `from_desugaring` flags, not `something` -error: aborting due to 18 previous errors +error[E0232]: invalid name in `on`-clause + --> $DIR/bad-annotation.rs:111:29 + | +LL | #[rustc_on_unimplemented(on(_Self = "y", message = "y"))] + | ^^^^^ expected one of `cause`, `from_desugaring`, `Self` or any generic parameter of the trait, not `_Self` + +error[E0232]: invalid name in `on`-clause + --> $DIR/bad-annotation.rs:116:29 + | +LL | #[rustc_on_unimplemented(on(abc = "y", message = "y"))] + | ^^^ expected one of `cause`, `from_desugaring`, `Self` or any generic parameter of the trait, not `abc` + +error: aborting due to 20 previous errors Some errors have detailed explanations: E0230, E0231, E0232. For more information about an error, try `rustc --explain E0230`. diff --git a/tests/ui/on-unimplemented/on-trait.rs b/tests/ui/on-unimplemented/on-trait.rs index 556813cd4795f..91630af17e92a 100644 --- a/tests/ui/on-unimplemented/on-trait.rs +++ b/tests/ui/on-unimplemented/on-trait.rs @@ -3,7 +3,7 @@ #![feature(rustc_attrs)] pub mod Bar { - #[rustc_on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}` in `{Foo}`"] + #[rustc_on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}` in `{This}`"] pub trait Foo {} } From 6555ef7f09a5b23a8eea05cba499a5b9fd16909a Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 20 May 2025 10:28:31 +0000 Subject: [PATCH 16/23] Querify coroutine_hidden_types --- compiler/rustc_middle/src/query/erase.rs | 7 +++- compiler/rustc_middle/src/query/mod.rs | 6 +++ compiler/rustc_middle/src/ty/context.rs | 2 +- compiler/rustc_middle/src/ty/util.rs | 36 +----------------- .../src/solve/assembly/structural_traits.rs | 4 +- .../src/traits/select/mod.rs | 2 +- .../rustc_traits/src/coroutine_witnesses.rs | 37 +++++++++++++++++++ compiler/rustc_traits/src/lib.rs | 2 + compiler/rustc_type_ir/src/interner.rs | 2 +- compiler/rustc_type_ir/src/ty_kind.rs | 10 +++++ 10 files changed, 67 insertions(+), 41 deletions(-) create mode 100644 compiler/rustc_traits/src/coroutine_witnesses.rs diff --git a/compiler/rustc_middle/src/query/erase.rs b/compiler/rustc_middle/src/query/erase.rs index 5bd111fa2f22d..2d248e028fe8b 100644 --- a/compiler/rustc_middle/src/query/erase.rs +++ b/compiler/rustc_middle/src/query/erase.rs @@ -6,7 +6,7 @@ use rustc_span::ErrorGuaranteed; use crate::query::CyclePlaceholder; use crate::ty::adjustment::CoerceUnsizedInfo; -use crate::ty::{self, Ty}; +use crate::ty::{self, Ty, TyCtxt}; use crate::{mir, traits}; #[derive(Copy, Clone)] @@ -207,6 +207,11 @@ impl EraseType for ty::Binder<'_, ty::FnSig<'_>> { type Result = [u8; size_of::>>()]; } +impl EraseType for ty::Binder<'_, ty::CoroutineWitnessTypes>> { + type Result = + [u8; size_of::>>>()]; +} + impl EraseType for ty::Binder<'_, &'_ ty::List>> { type Result = [u8; size_of::>>>()]; } diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index b2133fea08cc3..b6218fff0c417 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -922,6 +922,12 @@ rustc_queries! { separate_provide_extern } + query coroutine_hidden_types( + def_id: DefId + ) -> ty::EarlyBinder<'tcx, ty::Binder<'tcx, ty::CoroutineWitnessTypes>>> { + desc { "looking up the hidden types stored across await points in a coroutine" } + } + /// Gets a map with the variances of every item in the local crate. /// ///
diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 0759fa3da428a..c205d53b93f1f 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -340,7 +340,7 @@ impl<'tcx> Interner for TyCtxt<'tcx> { fn coroutine_hidden_types( self, def_id: DefId, - ) -> ty::EarlyBinder<'tcx, ty::Binder<'tcx, &'tcx ty::List>>> { + ) -> ty::EarlyBinder<'tcx, ty::Binder<'tcx, ty::CoroutineWitnessTypes>>> { self.coroutine_hidden_types(def_id) } diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index 9676aa4044823..ecf83926df7d2 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -26,7 +26,7 @@ use crate::query::Providers; use crate::ty::layout::{FloatExt, IntegerExt}; use crate::ty::{ self, Asyncness, FallibleTypeFolder, GenericArgKind, GenericArgsRef, Ty, TyCtxt, TypeFoldable, - TypeFolder, TypeSuperFoldable, TypeVisitableExt, Upcast, fold_regions, + TypeFolder, TypeSuperFoldable, TypeVisitableExt, Upcast, }; #[derive(Copy, Clone, Debug)] @@ -737,40 +737,6 @@ impl<'tcx> TyCtxt<'tcx> { } } - /// Return the set of types that should be taken into account when checking - /// trait bounds on a coroutine's internal state. This properly replaces - /// `ReErased` with new existential bound lifetimes. - pub fn coroutine_hidden_types( - self, - def_id: DefId, - ) -> ty::EarlyBinder<'tcx, ty::Binder<'tcx, &'tcx ty::List>>> { - let coroutine_layout = self.mir_coroutine_witnesses(def_id); - let mut vars = vec![]; - let bound_tys = self.mk_type_list_from_iter( - coroutine_layout - .as_ref() - .map_or_else(|| [].iter(), |l| l.field_tys.iter()) - .filter(|decl| !decl.ignore_for_traits) - .map(|decl| { - let ty = fold_regions(self, decl.ty, |re, debruijn| { - assert_eq!(re, self.lifetimes.re_erased); - let var = ty::BoundVar::from_usize(vars.len()); - vars.push(ty::BoundVariableKind::Region(ty::BoundRegionKind::Anon)); - ty::Region::new_bound( - self, - debruijn, - ty::BoundRegion { var, kind: ty::BoundRegionKind::Anon }, - ) - }); - ty - }), - ); - ty::EarlyBinder::bind(ty::Binder::bind_with_vars( - bound_tys, - self.mk_bound_variable_kinds(&vars), - )) - } - /// Expands the given impl trait type, stopping if the type is recursive. #[instrument(skip(self), level = "debug", ret)] pub fn try_expand_impl_trait_type( diff --git a/compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs b/compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs index 2a2b462a36cbd..9b89b9fad8626 100644 --- a/compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs +++ b/compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs @@ -83,7 +83,7 @@ where .cx() .coroutine_hidden_types(def_id) .instantiate(cx, args) - .map_bound(|tys| tys.to_vec())), + .map_bound(|bound| bound.types.to_vec())), ty::UnsafeBinder(bound_ty) => Ok(bound_ty.map_bound(|ty| vec![ty])), @@ -249,7 +249,7 @@ where .cx() .coroutine_hidden_types(def_id) .instantiate(ecx.cx(), args) - .map_bound(|tys| tys.to_vec())), + .map_bound(|bound| bound.types.to_vec())), } } diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 44a76f6e08327..549aed908f381 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -2866,7 +2866,7 @@ fn rebind_coroutine_witness_types<'tcx>( let shifted_coroutine_types = tcx.shift_bound_var_indices(bound_vars.len(), bound_coroutine_types.skip_binder()); ty::Binder::bind_with_vars( - ty::EarlyBinder::bind(shifted_coroutine_types.to_vec()).instantiate(tcx, args), + ty::EarlyBinder::bind(shifted_coroutine_types.types.to_vec()).instantiate(tcx, args), tcx.mk_bound_variable_kinds_from_iter( bound_vars.iter().chain(bound_coroutine_types.bound_vars()), ), diff --git a/compiler/rustc_traits/src/coroutine_witnesses.rs b/compiler/rustc_traits/src/coroutine_witnesses.rs new file mode 100644 index 0000000000000..447e13126ccdc --- /dev/null +++ b/compiler/rustc_traits/src/coroutine_witnesses.rs @@ -0,0 +1,37 @@ +use rustc_hir::def_id::DefId; +use rustc_middle::ty::{self, TyCtxt, fold_regions}; + +/// Return the set of types that should be taken into account when checking +/// trait bounds on a coroutine's internal state. This properly replaces +/// `ReErased` with new existential bound lifetimes. +pub(crate) fn coroutine_hidden_types<'tcx>( + tcx: TyCtxt<'tcx>, + def_id: DefId, +) -> ty::EarlyBinder<'tcx, ty::Binder<'tcx, ty::CoroutineWitnessTypes>>> { + let coroutine_layout = tcx.mir_coroutine_witnesses(def_id); + let mut vars = vec![]; + let bound_tys = tcx.mk_type_list_from_iter( + coroutine_layout + .as_ref() + .map_or_else(|| [].iter(), |l| l.field_tys.iter()) + .filter(|decl| !decl.ignore_for_traits) + .map(|decl| { + let ty = fold_regions(tcx, decl.ty, |re, debruijn| { + assert_eq!(re, tcx.lifetimes.re_erased); + let var = ty::BoundVar::from_usize(vars.len()); + vars.push(ty::BoundVariableKind::Region(ty::BoundRegionKind::Anon)); + ty::Region::new_bound( + tcx, + debruijn, + ty::BoundRegion { var, kind: ty::BoundRegionKind::Anon }, + ) + }); + ty + }), + ); + + ty::EarlyBinder::bind(ty::Binder::bind_with_vars( + ty::CoroutineWitnessTypes { types: bound_tys }, + tcx.mk_bound_variable_kinds(&vars), + )) +} diff --git a/compiler/rustc_traits/src/lib.rs b/compiler/rustc_traits/src/lib.rs index 697c839180312..32d8c3f58e08a 100644 --- a/compiler/rustc_traits/src/lib.rs +++ b/compiler/rustc_traits/src/lib.rs @@ -5,6 +5,7 @@ // tidy-alphabetical-end mod codegen; +mod coroutine_witnesses; mod dropck_outlives; mod evaluate_obligation; mod implied_outlives_bounds; @@ -24,4 +25,5 @@ pub fn provide(p: &mut Providers) { normalize_erasing_regions::provide(p); type_op::provide(p); p.codegen_select_candidate = codegen::codegen_select_candidate; + p.coroutine_hidden_types = coroutine_witnesses::coroutine_hidden_types; } diff --git a/compiler/rustc_type_ir/src/interner.rs b/compiler/rustc_type_ir/src/interner.rs index 0fd2d9f3ad38b..c10241cfcf0f6 100644 --- a/compiler/rustc_type_ir/src/interner.rs +++ b/compiler/rustc_type_ir/src/interner.rs @@ -210,7 +210,7 @@ pub trait Interner: fn coroutine_hidden_types( self, def_id: Self::DefId, - ) -> ty::EarlyBinder>; + ) -> ty::EarlyBinder>>; fn fn_sig( self, diff --git a/compiler/rustc_type_ir/src/ty_kind.rs b/compiler/rustc_type_ir/src/ty_kind.rs index cf2e4284d10da..0cd98b5aa53b4 100644 --- a/compiler/rustc_type_ir/src/ty_kind.rs +++ b/compiler/rustc_type_ir/src/ty_kind.rs @@ -1163,3 +1163,13 @@ pub struct FnHeader { pub safety: I::Safety, pub abi: I::Abi, } + +#[derive_where(Clone, Copy, Debug, PartialEq, Eq, Hash; I: Interner)] +#[cfg_attr( + feature = "nightly", + derive(Encodable_NoContext, Decodable_NoContext, HashStable_NoContext) +)] +#[derive(TypeVisitable_Generic, TypeFoldable_Generic, Lift_Generic)] +pub struct CoroutineWitnessTypes { + pub types: I::Tys, +} From f60bab475e47e84bb188e463b6a17894a57092c4 Mon Sep 17 00:00:00 2001 From: Boxy Date: Tue, 8 Apr 2025 17:01:29 +0100 Subject: [PATCH 17/23] Don't allow repeat expr count inference side effects to propagate --- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 63 ++++++++++++++----- .../copy-check-deferred-before-fallback.rs | 3 +- ...copy-check-deferred-before-fallback.stderr | 14 +++++ .../copy-check-inference-side-effects.rs | 34 ++++++++++ .../copy-check-inference-side-effects.stderr | 28 +++++++++ 5 files changed, 123 insertions(+), 19 deletions(-) create mode 100644 tests/ui/repeat-expr/copy-check-deferred-before-fallback.stderr create mode 100644 tests/ui/repeat-expr/copy-check-inference-side-effects.rs create mode 100644 tests/ui/repeat-expr/copy-check-inference-side-effects.stderr diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index d2cdfe22a3ad0..533b5b2be7981 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -104,24 +104,53 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub(in super::super) fn check_repeat_exprs(&self) { let mut deferred_repeat_expr_checks = self.deferred_repeat_expr_checks.borrow_mut(); debug!("FnCtxt::check_repeat_exprs: {} deferred checks", deferred_repeat_expr_checks.len()); - for (element, element_ty, count) in deferred_repeat_expr_checks.drain(..) { - // We want to emit an error if the const is not structurally resolveable as otherwise - // we can find up conservatively proving `Copy` which may infer the repeat expr count - // to something that never required `Copy` in the first place. - let count = - self.structurally_resolve_const(element.span, self.normalize(element.span, count)); - - // Avoid run on "`NotCopy: Copy` is not implemented" errors when the repeat expr count - // is erroneous/unknown. The user might wind up specifying a repeat count of 0/1. - if count.references_error() { - continue; - } - // If the length is 0, we don't create any elements, so we don't copy any. - // If the length is 1, we don't copy that one element, we move it. Only check - // for `Copy` if the length is larger. - if count.try_to_target_usize(self.tcx).is_none_or(|x| x > 1) { - self.enforce_repeat_element_needs_copy_bound(element, element_ty); + let deferred_repeat_expr_checks = deferred_repeat_expr_checks + .drain(..) + .flat_map(|(element, element_ty, count)| { + // We want to emit an error if the const is not structurally resolveable as otherwise + // we can find up conservatively proving `Copy` which may infer the repeat expr count + // to something that never required `Copy` in the first place. + let count = self + .structurally_resolve_const(element.span, self.normalize(element.span, count)); + + // Avoid run on "`NotCopy: Copy` is not implemented" errors when the repeat expr count + // is erroneous/unknown. The user might wind up specifying a repeat count of 0/1. + if count.references_error() { + return None; + } + + Some((element, element_ty, count)) + }) + // We collect to force the side effects of structurally resolving the repeat count to happen in one + // go, to avoid side effects from proving `Copy` affecting whether repeat counts are known or not. + // If we did not do this we would get results that depend on the order that we evaluate each repeat + // expr's `Copy` check. + .collect::>(); + + for (element, element_ty, count) in deferred_repeat_expr_checks { + match count.kind() { + ty::ConstKind::Value(val) + if val.try_to_target_usize(self.tcx).is_none_or(|count| count > 1) => + { + self.enforce_repeat_element_needs_copy_bound(element, element_ty) + } + // If the length is 0 or 1 we don't actually copy the element, we either don't create it + // or we just use the one value. + ty::ConstKind::Value(_) => (), + + // If the length is a generic parameter or some rigid alias then conservatively + // require `element_ty: Copy` as it may wind up being `>1` after monomorphization. + ty::ConstKind::Param(_) + | ty::ConstKind::Expr(_) + | ty::ConstKind::Placeholder(_) + | ty::ConstKind::Unevaluated(_) => { + self.enforce_repeat_element_needs_copy_bound(element, element_ty) + } + + ty::ConstKind::Bound(_, _) | ty::ConstKind::Infer(_) | ty::ConstKind::Error(_) => { + unreachable!() + } } } } diff --git a/tests/ui/repeat-expr/copy-check-deferred-before-fallback.rs b/tests/ui/repeat-expr/copy-check-deferred-before-fallback.rs index 4654d7483a64d..bf519febb302c 100644 --- a/tests/ui/repeat-expr/copy-check-deferred-before-fallback.rs +++ b/tests/ui/repeat-expr/copy-check-deferred-before-fallback.rs @@ -1,5 +1,3 @@ -//@ check-pass - #![feature(generic_arg_infer)] // Test that if we defer repeat expr copy checks to end of typechecking they're @@ -37,6 +35,7 @@ fn main() { let b: [Foo<_>; 2] = [Foo(PhantomData); _]; tie(&a, b); let c = [NotCopy; _]; + //~^ ERROR: type annotations needed for `[NotCopy; _]` // a is of type `?y` // b is of type `[Foo; 2]` diff --git a/tests/ui/repeat-expr/copy-check-deferred-before-fallback.stderr b/tests/ui/repeat-expr/copy-check-deferred-before-fallback.stderr new file mode 100644 index 0000000000000..d6189329c92ad --- /dev/null +++ b/tests/ui/repeat-expr/copy-check-deferred-before-fallback.stderr @@ -0,0 +1,14 @@ +error[E0282]: type annotations needed for `[NotCopy; _]` + --> $DIR/copy-check-deferred-before-fallback.rs:37:9 + | +LL | let c = [NotCopy; _]; + | ^ ------- type must be known at this point + | +help: consider giving `c` an explicit type, where the value of const parameter `N` is specified + | +LL | let c: [_; N] = [NotCopy; _]; + | ++++++++ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0282`. diff --git a/tests/ui/repeat-expr/copy-check-inference-side-effects.rs b/tests/ui/repeat-expr/copy-check-inference-side-effects.rs new file mode 100644 index 0000000000000..416a20169fba1 --- /dev/null +++ b/tests/ui/repeat-expr/copy-check-inference-side-effects.rs @@ -0,0 +1,34 @@ +#![feature(generic_arg_infer)] + +struct Foo; + +impl Clone for Foo<1> { + fn clone(&self) -> Self { + Self + } +} +impl Copy for Foo<1> {} + +fn unify(_: &[Foo; 2], _: &[String; N]) {} + +fn works_if_inference_side_effects() { + // This will only pass if inference side effectrs from proving `Foo: Copy` are + // able to be relied upon by other repeat expressions. + let a /* : [Foo; 2] */ = [Foo::<_>; 2]; + //~^ ERROR: type annotations needed for `[Foo<_>; 2]` + let b /* : [String; ?x] */ = ["string".to_string(); _]; + + unify(&a, &b); +} + +fn works_if_fixed_point() { + // This will only pass if the *second* array repeat expr is checked first + // allowing `Foo: Copy` to infer the array length of the first repeat expr. + let b /* : [String; ?x] */ = ["string".to_string(); _]; + //~^ ERROR: type annotations needed for `[String; _]` + let a /* : [Foo; 2] */ = [Foo::<_>; 2]; + + unify(&a, &b); +} + +fn main() {} diff --git a/tests/ui/repeat-expr/copy-check-inference-side-effects.stderr b/tests/ui/repeat-expr/copy-check-inference-side-effects.stderr new file mode 100644 index 0000000000000..505beff0f6b2e --- /dev/null +++ b/tests/ui/repeat-expr/copy-check-inference-side-effects.stderr @@ -0,0 +1,28 @@ +error[E0282]: type annotations needed for `[Foo<_>; 2]` + --> $DIR/copy-check-inference-side-effects.rs:17:9 + | +LL | let a /* : [Foo; 2] */ = [Foo::<_>; 2]; + | ^ +LL | +LL | let b /* : [String; ?x] */ = ["string".to_string(); _]; + | -------------------- type must be known at this point + | +help: consider giving `a` an explicit type, where the value of const parameter `N` is specified + | +LL | let a: [Foo; 2] /* : [Foo; 2] */ = [Foo::<_>; 2]; + | +++++++++++++ + +error[E0282]: type annotations needed for `[String; _]` + --> $DIR/copy-check-inference-side-effects.rs:27:9 + | +LL | let b /* : [String; ?x] */ = ["string".to_string(); _]; + | ^ -------------------- type must be known at this point + | +help: consider giving `b` an explicit type, where the value of const parameter `N` is specified + | +LL | let b: [_; N] /* : [String; ?x] */ = ["string".to_string(); _]; + | ++++++++ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0282`. From 52f2c45dee5211418e762789ad63e3ae56d9f3bf Mon Sep 17 00:00:00 2001 From: Boxy Date: Thu, 10 Apr 2025 13:02:07 +0100 Subject: [PATCH 18/23] Properly test whether repeat expr checks are pre/post integer fallback --- .../copy-check-deferred-after-fallback.rs | 64 +++++++++++------- .../copy-check-deferred-after-fallback.stderr | 12 ++-- .../copy-check-deferred-before-fallback.rs | 66 ++++++++----------- ...copy-check-deferred-before-fallback.stderr | 14 ---- 4 files changed, 74 insertions(+), 82 deletions(-) delete mode 100644 tests/ui/repeat-expr/copy-check-deferred-before-fallback.stderr diff --git a/tests/ui/repeat-expr/copy-check-deferred-after-fallback.rs b/tests/ui/repeat-expr/copy-check-deferred-after-fallback.rs index d9ad93541ecff..3f310f07de0fe 100644 --- a/tests/ui/repeat-expr/copy-check-deferred-after-fallback.rs +++ b/tests/ui/repeat-expr/copy-check-deferred-after-fallback.rs @@ -1,37 +1,53 @@ #![feature(generic_arg_infer)] -// Test that would start passing if we defer repeat expr copy checks to end of -// typechecking and they're checked after integer fallback occurs. We accomplish -// this by contriving a situation where integer fallback allows progress to be -// made on a trait goal that infers the length of a repeat expr. +// Test when deferring repeat expr copy checks to end of typechecking whether they're +// checked before integer fallback occurs or not. We accomplish this by having a repeat +// count that can only be inferred after integer fallback has occured. This test will +// pass if we were to check repeat exprs after integer fallback. use std::marker::PhantomData; - -struct NotCopy; +struct Foo(PhantomData); + +// We impl Copy/Clone for multiple (but not all) substitutions +// to ensure that `Foo: Copy` can't be proven on the basis +// of there only being one applying impl. +impl Clone for Foo { + fn clone(&self) -> Self { + Foo(PhantomData) + } +} +impl Clone for Foo { + fn clone(&self) -> Self { + Foo(PhantomData) + } +} +impl Copy for Foo {} +impl Copy for Foo {} trait Trait {} -impl Trait<2> for u32 {} +// We impl `Trait` for both `i32` and `u32` to avoid being able +// to prove `?int: Trait` from there only being one impl. impl Trait<1> for i32 {} +impl Trait<2> for u32 {} -fn make_goal, const N: usize>(_: &T, _: [NotCopy; N]) {} +fn tie_and_make_goal>(_: &T, _: &[Foo; N]) {} fn main() { let a = 1; - let b = [NotCopy; _]; - //~^ ERROR: type annotations needed - - // a is of type `?y` - // b is of type `[NotCopy; ?x]` - // there is a goal ?y: Trait` with two candidates: - // - `i32: Trait<1>`, ?y=i32 ?x=1 which doesnt require `NotCopy: Copy` - // - `u32: Trait<2>` ?y=u32 ?x=2 which requires `NotCopy: Copy` - make_goal(&a, b); - - // final repeat expr checks: - // - // `NotCopy; ?x` - // - succeeds if fallback happens before repeat exprs as `i32: Trait` infers `?x=1` - // - fails if repeat expr checks happen first as `?x` is unconstrained so cannot be - // structurally resolved + // Deferred repeat expr `Foo; ?n` + let b = [Foo(PhantomData); _]; + //~^ ERROR: type annotations needed for `[Foo<{integer}>; _]` + + // Introduces a `?int: Trait` goal + tie_and_make_goal(&a, &b); + + // If fallback doesn't occur: + // - `Foo; ?n`is ambig as repeat count is unknown -> error + + // If fallback occurs: + // - `?int` inferred to `i32` + // - `?int: Trait` becomes `i32: Trait` wihhc infers `?n=1` + // - Repeat expr check `Foo; ?n` is now `Foo; 1` + // - `Foo; 1` doesn't require `Foo: Copy` } diff --git a/tests/ui/repeat-expr/copy-check-deferred-after-fallback.stderr b/tests/ui/repeat-expr/copy-check-deferred-after-fallback.stderr index 2a0cb3fb7a394..103b074dda7c8 100644 --- a/tests/ui/repeat-expr/copy-check-deferred-after-fallback.stderr +++ b/tests/ui/repeat-expr/copy-check-deferred-after-fallback.stderr @@ -1,13 +1,13 @@ -error[E0282]: type annotations needed for `[NotCopy; _]` - --> $DIR/copy-check-deferred-after-fallback.rs:21:9 +error[E0282]: type annotations needed for `[Foo<{integer}>; _]` + --> $DIR/copy-check-deferred-after-fallback.rs:39:9 | -LL | let b = [NotCopy; _]; - | ^ ------- type must be known at this point +LL | let b = [Foo(PhantomData); _]; + | ^ ---------------- type must be known at this point | help: consider giving `b` an explicit type, where the value of const parameter `N` is specified | -LL | let b: [_; N] = [NotCopy; _]; - | ++++++++ +LL | let b: [Foo<{integer}>; N] = [Foo(PhantomData); _]; + | +++++++++++++++++++++ error: aborting due to 1 previous error diff --git a/tests/ui/repeat-expr/copy-check-deferred-before-fallback.rs b/tests/ui/repeat-expr/copy-check-deferred-before-fallback.rs index bf519febb302c..b81997a3c9fa3 100644 --- a/tests/ui/repeat-expr/copy-check-deferred-before-fallback.rs +++ b/tests/ui/repeat-expr/copy-check-deferred-before-fallback.rs @@ -1,16 +1,13 @@ +//@ check-pass #![feature(generic_arg_infer)] -// Test that if we defer repeat expr copy checks to end of typechecking they're -// checked before integer fallback occurs. We accomplish this by contriving a -// situation where we have a goal that can be proven either via another repeat expr -// check or by integer fallback. In the integer fallback case an array length would -// be inferred to `2` requiring `NotCopy: Copy`, and in the repeat expr case it would -// be inferred to `1`. +// Test when deferring repeat expr checks to end of typechecking whether they're +// checked before integer fallback occurs. We accomplish this by having the repeat +// expr check allow inference progress on an ambiguous goal, where the ambiguous goal +// would fail if the inference variable was fallen back to `i32`. This test will +// pass if wecheck repeat exprs before integer fallback. use std::marker::PhantomData; - -struct NotCopy; - struct Foo(PhantomData); impl Clone for Foo { @@ -18,41 +15,34 @@ impl Clone for Foo { Foo(PhantomData) } } - impl Copy for Foo {} -fn tie(_: &T, _: [Foo; 2]) {} - -trait Trait {} +trait Trait {} -impl Trait<2> for i32 {} -impl Trait<1> for u32 {} +// Two impls just to ensure that `?int: Trait` wont itself succeed by unifying with +// a self type on an impl here. It also ensures that integer fallback would actually +// be valid for all of the stalled goals incase that's ever something we take into account. +impl Trait for i32 {} +impl Trait for u32 {} -fn make_goal, const N: usize>(_: &T, _: [NotCopy; N]) {} +fn make_goal(_: &T) {} +fn tie(_: &T, _: &[Foo; 2]) {} fn main() { let a = 1; + // `?int: Trait` + make_goal(&a); + + // Deferred `Foo: Copy` requirement let b: [Foo<_>; 2] = [Foo(PhantomData); _]; - tie(&a, b); - let c = [NotCopy; _]; - //~^ ERROR: type annotations needed for `[NotCopy; _]` - - // a is of type `?y` - // b is of type `[Foo; 2]` - // c is of type `[NotCopy; ?x]` - // there is a goal ?y: Trait` with two candidates: - // - `i32: Trait<2>`, ?y=i32 ?x=2 which requires `NotCopy: Copy` when expr checks happen - // - `u32: Trait<1>` ?y=u32 ?x=1 which doesnt require `NotCopy: Copy` - make_goal(&a, c); - - // final repeat expr checks: - // - // `Foo; 2` - // - Foo: Copy - // - requires ?y=u32 - // - // `NotCopy; ?x` - // - fails if fallback happens before repeat exprs as `i32: Trait` infers `?x=2` - // - succeeds if repeat expr checks happen first as `?y=u32` means `u32: Trait` - // infers `?x=1` + tie(&a, &b); + + // If fallback doesn't occur: + // - `Foo; 2`is > 1, needs copy + // - `Foo: Copy` infers `?int=u32` + // - stalled goal `?int: Trait` can now make progress and succeed + + // If fallback occurs: + // - `Foo; 2` is > 1, needs copy + // - `Foo: Copy` doesn't hold -> error } diff --git a/tests/ui/repeat-expr/copy-check-deferred-before-fallback.stderr b/tests/ui/repeat-expr/copy-check-deferred-before-fallback.stderr deleted file mode 100644 index d6189329c92ad..0000000000000 --- a/tests/ui/repeat-expr/copy-check-deferred-before-fallback.stderr +++ /dev/null @@ -1,14 +0,0 @@ -error[E0282]: type annotations needed for `[NotCopy; _]` - --> $DIR/copy-check-deferred-before-fallback.rs:37:9 - | -LL | let c = [NotCopy; _]; - | ^ ------- type must be known at this point - | -help: consider giving `c` an explicit type, where the value of const parameter `N` is specified - | -LL | let c: [_; N] = [NotCopy; _]; - | ++++++++ - -error: aborting due to 1 previous error - -For more information about this error, try `rustc --explain E0282`. From 77a2fc60a11838d93df08a7a31ba47bcc828f750 Mon Sep 17 00:00:00 2001 From: Boxy Date: Thu, 10 Apr 2025 15:11:55 +0100 Subject: [PATCH 19/23] GAI logic on stable too --- compiler/rustc_hir_typeck/src/expr.rs | 51 +-------------- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 48 +++++++++++++- .../lang-item-generic-requirements.rs | 2 + .../lang-item-generic-requirements.stderr | 20 +++++- ...py-check-const-element-uninferred-count.rs | 65 +++++++++++++++++++ ...heck-const-element-uninferred-count.stderr | 36 ++++++++++ .../copy-inference-side-effects-are-lazy.rs | 9 +-- ...opy-inference-side-effects-are-lazy.stderr | 17 +++++ 8 files changed, 186 insertions(+), 62 deletions(-) create mode 100644 tests/ui/repeat-expr/copy-check-const-element-uninferred-count.rs create mode 100644 tests/ui/repeat-expr/copy-check-const-element-uninferred-count.stderr create mode 100644 tests/ui/repeat-expr/copy-inference-side-effects-are-lazy.stderr diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index a1a33885b944c..de769c1166608 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -1900,62 +1900,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // We defer checking whether the element type is `Copy` as it is possible to have // an inference variable as a repeat count and it seems unlikely that `Copy` would // have inference side effects required for type checking to succeed. - if tcx.features().generic_arg_infer() { - self.deferred_repeat_expr_checks.borrow_mut().push((element, element_ty, count)); - // If the length is 0, we don't create any elements, so we don't copy any. - // If the length is 1, we don't copy that one element, we move it. Only check - // for `Copy` if the length is larger, or unevaluated. - } else if count.try_to_target_usize(self.tcx).is_none_or(|x| x > 1) { - self.enforce_repeat_element_needs_copy_bound(element, element_ty); - } + self.deferred_repeat_expr_checks.borrow_mut().push((element, element_ty, count)); let ty = Ty::new_array_with_const_len(tcx, t, count); self.register_wf_obligation(ty.into(), expr.span, ObligationCauseCode::WellFormed(None)); ty } - /// Requires that `element_ty` is `Copy` (unless it's a const expression itself). - pub(super) fn enforce_repeat_element_needs_copy_bound( - &self, - element: &hir::Expr<'_>, - element_ty: Ty<'tcx>, - ) { - let tcx = self.tcx; - // Actual constants as the repeat element get inserted repeatedly instead of getting copied via Copy. - match &element.kind { - hir::ExprKind::ConstBlock(..) => return, - hir::ExprKind::Path(qpath) => { - let res = self.typeck_results.borrow().qpath_res(qpath, element.hir_id); - if let Res::Def(DefKind::Const | DefKind::AssocConst | DefKind::AnonConst, _) = res - { - return; - } - } - _ => {} - } - // If someone calls a const fn or constructs a const value, they can extract that - // out into a separate constant (or a const block in the future), so we check that - // to tell them that in the diagnostic. Does not affect typeck. - let is_constable = match element.kind { - hir::ExprKind::Call(func, _args) => match *self.node_ty(func.hir_id).kind() { - ty::FnDef(def_id, _) if tcx.is_stable_const_fn(def_id) => traits::IsConstable::Fn, - _ => traits::IsConstable::No, - }, - hir::ExprKind::Path(qpath) => { - match self.typeck_results.borrow().qpath_res(&qpath, element.hir_id) { - Res::Def(DefKind::Ctor(_, CtorKind::Const), _) => traits::IsConstable::Ctor, - _ => traits::IsConstable::No, - } - } - _ => traits::IsConstable::No, - }; - - let lang_item = self.tcx.require_lang_item(LangItem::Copy, None); - let code = - traits::ObligationCauseCode::RepeatElementCopy { is_constable, elt_span: element.span }; - self.require_type_meets(element_ty, element.span, code, lang_item); - } - fn check_expr_tuple( &self, elts: &'tcx [hir::Expr<'tcx>], diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 533b5b2be7981..7a0b4c22a19ae 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -4,10 +4,10 @@ use itertools::Itertools; use rustc_data_structures::fx::FxIndexSet; use rustc_errors::codes::*; use rustc_errors::{Applicability, Diag, ErrorGuaranteed, MultiSpan, a_or_an, listify, pluralize}; -use rustc_hir::def::{CtorOf, DefKind, Res}; +use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::Visitor; -use rustc_hir::{ExprKind, HirId, Node, QPath}; +use rustc_hir::{ExprKind, HirId, LangItem, Node, QPath}; use rustc_hir_analysis::check::potentially_plural_count; use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer; use rustc_index::IndexVec; @@ -155,6 +155,50 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + /// Requires that `element_ty` is `Copy` (unless it's a const expression itself). + pub(super) fn enforce_repeat_element_needs_copy_bound( + &self, + element: &hir::Expr<'_>, + element_ty: Ty<'tcx>, + ) { + // Actual constants as the repeat element get inserted repeatedly instead of getting copied via Copy. + match &element.kind { + hir::ExprKind::ConstBlock(..) => return, + hir::ExprKind::Path(qpath) => { + let res = self.typeck_results.borrow().qpath_res(qpath, element.hir_id); + if let Res::Def(DefKind::Const | DefKind::AssocConst | DefKind::AnonConst, _) = res + { + return; + } + } + _ => {} + } + + // If someone calls a const fn or constructs a const value, they can extract that + // out into a separate constant (or a const block in the future), so we check that + // to tell them that in the diagnostic. Does not affect typeck. + let is_constable = match element.kind { + hir::ExprKind::Call(func, _args) => match *self.node_ty(func.hir_id).kind() { + ty::FnDef(def_id, _) if self.tcx.is_stable_const_fn(def_id) => { + traits::IsConstable::Fn + } + _ => traits::IsConstable::No, + }, + hir::ExprKind::Path(qpath) => { + match self.typeck_results.borrow().qpath_res(&qpath, element.hir_id) { + Res::Def(DefKind::Ctor(_, CtorKind::Const), _) => traits::IsConstable::Ctor, + _ => traits::IsConstable::No, + } + } + _ => traits::IsConstable::No, + }; + + let lang_item = self.tcx.require_lang_item(LangItem::Copy, None); + let code = + traits::ObligationCauseCode::RepeatElementCopy { is_constable, elt_span: element.span }; + self.require_type_meets(element_ty, element.span, code, lang_item); + } + /// Generic function that factors out common logic from function calls, /// method calls and overloaded operators. pub(in super::super) fn check_argument_types( diff --git a/tests/ui/lang-items/lang-item-generic-requirements.rs b/tests/ui/lang-items/lang-item-generic-requirements.rs index 90ed5f3f0efd0..7676b5557d22b 100644 --- a/tests/ui/lang-items/lang-item-generic-requirements.rs +++ b/tests/ui/lang-items/lang-item-generic-requirements.rs @@ -49,12 +49,14 @@ fn ice() { // Use index let arr = [0; 5]; let _ = arr[2]; + //~^ ERROR cannot index into a value of type `[{integer}; 5]` // Use phantomdata let _ = MyPhantomData::<(), i32>; // Use Foo let _: () = Foo; + //~^ ERROR mismatched types } // use `start` diff --git a/tests/ui/lang-items/lang-item-generic-requirements.stderr b/tests/ui/lang-items/lang-item-generic-requirements.stderr index 3de67d6594035..409fa05d6371a 100644 --- a/tests/ui/lang-items/lang-item-generic-requirements.stderr +++ b/tests/ui/lang-items/lang-item-generic-requirements.stderr @@ -76,9 +76,23 @@ LL | r + a; | | | {integer} +error[E0608]: cannot index into a value of type `[{integer}; 5]` + --> $DIR/lang-item-generic-requirements.rs:51:16 + | +LL | let _ = arr[2]; + | ^^^ + +error[E0308]: mismatched types + --> $DIR/lang-item-generic-requirements.rs:58:17 + | +LL | let _: () = Foo; + | -- ^^^ expected `()`, found `Foo` + | | + | expected due to this + error: requires `copy` lang_item -error: aborting due to 10 previous errors +error: aborting due to 12 previous errors -Some errors have detailed explanations: E0369, E0392, E0718. -For more information about an error, try `rustc --explain E0369`. +Some errors have detailed explanations: E0308, E0369, E0392, E0608, E0718. +For more information about an error, try `rustc --explain E0308`. diff --git a/tests/ui/repeat-expr/copy-check-const-element-uninferred-count.rs b/tests/ui/repeat-expr/copy-check-const-element-uninferred-count.rs new file mode 100644 index 0000000000000..8ef0c2690ba0e --- /dev/null +++ b/tests/ui/repeat-expr/copy-check-const-element-uninferred-count.rs @@ -0,0 +1,65 @@ +#![feature(generic_arg_infer)] + +// Test when deferring repeat expr copy checks to end of typechecking whether elements +// that are const items allow for repeat counts to go uninferred without an error being +// emitted if they would later wind up inferred by integer fallback. +// +// This test should be updated if we wind up deferring repeat expr checks until *after* +// integer fallback as the point of the test is not *specifically* about integer fallback +// but rather about the behaviour of `const` element exprs. + +trait Trait {} + +// We impl `Trait` for both `i32` and `u32` to avoid being able +// to prove `?int: Trait` from there only being one impl. +impl Trait<2> for i32 {} +impl Trait<2> for u32 {} + +fn tie_and_make_goal>(_: &T, _: &[String; N]) {} + +fn const_block() { + // Deferred repeat expr `String; ?n` + let a = [const { String::new() }; _]; + //~^ ERROR: type annotations needed for `[String; _]` + + // `?int: Trait` goal + tie_and_make_goal(&1, &a); + + // If repeat expr checks structurally resolve the `?n`s before checking if the + // element is a `const` then we would error here. Otherwise we avoid doing so, + // integer fallback occurs, allowing `?int: Trait` goals to make progress, + // inferring the repeat counts (to `2` but that doesn't matter as the element is `const`). +} + +fn const_item() { + const MY_CONST: String = String::new(); + + // Deferred repeat expr `String; ?n` + let a = [MY_CONST; _]; + //~^ ERROR: type annotations needed for `[String; _]` + + // `?int: Trait` goal + tie_and_make_goal(&1, &a); + + // ... same as `const_block` +} + +fn assoc_const() { + trait Dummy { + const ASSOC: String; + } + impl Dummy for () { + const ASSOC: String = String::new(); + } + + // Deferred repeat expr `String; ?n` + let a = [<() as Dummy>::ASSOC; _]; + //~^ ERROR: type annotations needed for `[String; _]` + + // `?int: Trait` goal + tie_and_make_goal(&1, &a); + + // ... same as `const_block` +} + +fn main() {} diff --git a/tests/ui/repeat-expr/copy-check-const-element-uninferred-count.stderr b/tests/ui/repeat-expr/copy-check-const-element-uninferred-count.stderr new file mode 100644 index 0000000000000..8229b0b2b3794 --- /dev/null +++ b/tests/ui/repeat-expr/copy-check-const-element-uninferred-count.stderr @@ -0,0 +1,36 @@ +error[E0282]: type annotations needed for `[String; _]` + --> $DIR/copy-check-const-element-uninferred-count.rs:22:9 + | +LL | let a = [const { String::new() }; _]; + | ^ ----------------------- type must be known at this point + | +help: consider giving `a` an explicit type, where the value of const parameter `N` is specified + | +LL | let a: [_; N] = [const { String::new() }; _]; + | ++++++++ + +error[E0282]: type annotations needed for `[String; _]` + --> $DIR/copy-check-const-element-uninferred-count.rs:38:9 + | +LL | let a = [MY_CONST; _]; + | ^ -------- type must be known at this point + | +help: consider giving `a` an explicit type, where the value of const parameter `N` is specified + | +LL | let a: [_; N] = [MY_CONST; _]; + | ++++++++ + +error[E0282]: type annotations needed for `[String; _]` + --> $DIR/copy-check-const-element-uninferred-count.rs:56:9 + | +LL | let a = [<() as Dummy>::ASSOC; _]; + | ^ -------------------- type must be known at this point + | +help: consider giving `a` an explicit type, where the value of const parameter `N` is specified + | +LL | let a: [_; N] = [<() as Dummy>::ASSOC; _]; + | ++++++++ + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0282`. diff --git a/tests/ui/repeat-expr/copy-inference-side-effects-are-lazy.rs b/tests/ui/repeat-expr/copy-inference-side-effects-are-lazy.rs index 0b0672d9c2b56..d50466ac4bbd8 100644 --- a/tests/ui/repeat-expr/copy-inference-side-effects-are-lazy.rs +++ b/tests/ui/repeat-expr/copy-inference-side-effects-are-lazy.rs @@ -1,8 +1,3 @@ -//@revisions: current gai -//@[current] check-pass - -#![cfg_attr(gai, feature(generic_arg_infer))] - use std::marker::PhantomData; struct Foo(PhantomData); @@ -20,6 +15,6 @@ fn extract(_: [Foo; N]) -> T { fn main() { let x = [Foo(PhantomData); 2]; - //[gai]~^ ERROR: type annotations needed - _ = extract(x).max(2); + //~^ ERROR: type annotations needed + extract(x).max(2); } diff --git a/tests/ui/repeat-expr/copy-inference-side-effects-are-lazy.stderr b/tests/ui/repeat-expr/copy-inference-side-effects-are-lazy.stderr new file mode 100644 index 0000000000000..ba44beb76dbb7 --- /dev/null +++ b/tests/ui/repeat-expr/copy-inference-side-effects-are-lazy.stderr @@ -0,0 +1,17 @@ +error[E0282]: type annotations needed for `[Foo<_>; 2]` + --> $DIR/copy-inference-side-effects-are-lazy.rs:17:9 + | +LL | let x = [Foo(PhantomData); 2]; + | ^ +LL | +LL | extract(x).max(2); + | ---------- type must be known at this point + | +help: consider giving `x` an explicit type, where the type for type parameter `T` is specified + | +LL | let x: [Foo; 2] = [Foo(PhantomData); 2]; + | +++++++++++++ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0282`. From 508a9f085393a1e348725ca7f169780c65dbc212 Mon Sep 17 00:00:00 2001 From: Boxy Date: Thu, 10 Apr 2025 15:30:28 +0100 Subject: [PATCH 20/23] Check for element being `const` before resolving repeat count --- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 95 +++++++++---------- ...py-check-const-element-uninferred-count.rs | 13 ++- ...heck-const-element-uninferred-count.stderr | 37 ++------ 3 files changed, 65 insertions(+), 80 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 7a0b4c22a19ae..dc6a9dc93d07a 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -108,6 +108,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let deferred_repeat_expr_checks = deferred_repeat_expr_checks .drain(..) .flat_map(|(element, element_ty, count)| { + // Actual constants as the repeat element get inserted repeatedly instead of getting copied via Copy + // so we don't need to attempt to structurally resolve the repeat count which may unnecessarily error. + match &element.kind { + hir::ExprKind::ConstBlock(..) => return None, + hir::ExprKind::Path(qpath) => { + let res = self.typeck_results.borrow().qpath_res(qpath, element.hir_id); + if let Res::Def( + DefKind::Const | DefKind::AssocConst | DefKind::AnonConst, + _, + ) = res + { + return None; + } + } + _ => {} + } + // We want to emit an error if the const is not structurally resolveable as otherwise // we can find up conservatively proving `Copy` which may infer the repeat expr count // to something that never required `Copy` in the first place. @@ -128,12 +145,40 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // expr's `Copy` check. .collect::>(); + let enforce_copy_bound = |element: &hir::Expr<'_>, element_ty| { + // If someone calls a const fn or constructs a const value, they can extract that + // out into a separate constant (or a const block in the future), so we check that + // to tell them that in the diagnostic. Does not affect typeck. + let is_constable = match element.kind { + hir::ExprKind::Call(func, _args) => match *self.node_ty(func.hir_id).kind() { + ty::FnDef(def_id, _) if self.tcx.is_stable_const_fn(def_id) => { + traits::IsConstable::Fn + } + _ => traits::IsConstable::No, + }, + hir::ExprKind::Path(qpath) => { + match self.typeck_results.borrow().qpath_res(&qpath, element.hir_id) { + Res::Def(DefKind::Ctor(_, CtorKind::Const), _) => traits::IsConstable::Ctor, + _ => traits::IsConstable::No, + } + } + _ => traits::IsConstable::No, + }; + + let lang_item = self.tcx.require_lang_item(LangItem::Copy, None); + let code = traits::ObligationCauseCode::RepeatElementCopy { + is_constable, + elt_span: element.span, + }; + self.require_type_meets(element_ty, element.span, code, lang_item); + }; + for (element, element_ty, count) in deferred_repeat_expr_checks { match count.kind() { ty::ConstKind::Value(val) if val.try_to_target_usize(self.tcx).is_none_or(|count| count > 1) => { - self.enforce_repeat_element_needs_copy_bound(element, element_ty) + enforce_copy_bound(element, element_ty) } // If the length is 0 or 1 we don't actually copy the element, we either don't create it // or we just use the one value. @@ -144,9 +189,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ty::ConstKind::Param(_) | ty::ConstKind::Expr(_) | ty::ConstKind::Placeholder(_) - | ty::ConstKind::Unevaluated(_) => { - self.enforce_repeat_element_needs_copy_bound(element, element_ty) - } + | ty::ConstKind::Unevaluated(_) => enforce_copy_bound(element, element_ty), ty::ConstKind::Bound(_, _) | ty::ConstKind::Infer(_) | ty::ConstKind::Error(_) => { unreachable!() @@ -155,50 +198,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - /// Requires that `element_ty` is `Copy` (unless it's a const expression itself). - pub(super) fn enforce_repeat_element_needs_copy_bound( - &self, - element: &hir::Expr<'_>, - element_ty: Ty<'tcx>, - ) { - // Actual constants as the repeat element get inserted repeatedly instead of getting copied via Copy. - match &element.kind { - hir::ExprKind::ConstBlock(..) => return, - hir::ExprKind::Path(qpath) => { - let res = self.typeck_results.borrow().qpath_res(qpath, element.hir_id); - if let Res::Def(DefKind::Const | DefKind::AssocConst | DefKind::AnonConst, _) = res - { - return; - } - } - _ => {} - } - - // If someone calls a const fn or constructs a const value, they can extract that - // out into a separate constant (or a const block in the future), so we check that - // to tell them that in the diagnostic. Does not affect typeck. - let is_constable = match element.kind { - hir::ExprKind::Call(func, _args) => match *self.node_ty(func.hir_id).kind() { - ty::FnDef(def_id, _) if self.tcx.is_stable_const_fn(def_id) => { - traits::IsConstable::Fn - } - _ => traits::IsConstable::No, - }, - hir::ExprKind::Path(qpath) => { - match self.typeck_results.borrow().qpath_res(&qpath, element.hir_id) { - Res::Def(DefKind::Ctor(_, CtorKind::Const), _) => traits::IsConstable::Ctor, - _ => traits::IsConstable::No, - } - } - _ => traits::IsConstable::No, - }; - - let lang_item = self.tcx.require_lang_item(LangItem::Copy, None); - let code = - traits::ObligationCauseCode::RepeatElementCopy { is_constable, elt_span: element.span }; - self.require_type_meets(element_ty, element.span, code, lang_item); - } - /// Generic function that factors out common logic from function calls, /// method calls and overloaded operators. pub(in super::super) fn check_argument_types( diff --git a/tests/ui/repeat-expr/copy-check-const-element-uninferred-count.rs b/tests/ui/repeat-expr/copy-check-const-element-uninferred-count.rs index 8ef0c2690ba0e..6115146539c19 100644 --- a/tests/ui/repeat-expr/copy-check-const-element-uninferred-count.rs +++ b/tests/ui/repeat-expr/copy-check-const-element-uninferred-count.rs @@ -20,7 +20,6 @@ fn tie_and_make_goal>(_: &T, _: &[String; N]) {} fn const_block() { // Deferred repeat expr `String; ?n` let a = [const { String::new() }; _]; - //~^ ERROR: type annotations needed for `[String; _]` // `?int: Trait` goal tie_and_make_goal(&1, &a); @@ -36,7 +35,6 @@ fn const_item() { // Deferred repeat expr `String; ?n` let a = [MY_CONST; _]; - //~^ ERROR: type annotations needed for `[String; _]` // `?int: Trait` goal tie_and_make_goal(&1, &a); @@ -54,7 +52,6 @@ fn assoc_const() { // Deferred repeat expr `String; ?n` let a = [<() as Dummy>::ASSOC; _]; - //~^ ERROR: type annotations needed for `[String; _]` // `?int: Trait` goal tie_and_make_goal(&1, &a); @@ -62,4 +59,14 @@ fn assoc_const() { // ... same as `const_block` } +fn const_block_but_uninferred() { + // Deferred repeat expr `String; ?n` + let a = [const { String::new() }; _]; + //~^ ERROR: type annotations needed for `[String; _]` + + // Even if we don't structurally resolve the repeat count as part of repeat expr + // checks, we still error on the repeat count being uninferred as we require all + // types/consts to be inferred by the end of type checking. +} + fn main() {} diff --git a/tests/ui/repeat-expr/copy-check-const-element-uninferred-count.stderr b/tests/ui/repeat-expr/copy-check-const-element-uninferred-count.stderr index 8229b0b2b3794..2f52537fa9407 100644 --- a/tests/ui/repeat-expr/copy-check-const-element-uninferred-count.stderr +++ b/tests/ui/repeat-expr/copy-check-const-element-uninferred-count.stderr @@ -1,36 +1,15 @@ -error[E0282]: type annotations needed for `[String; _]` - --> $DIR/copy-check-const-element-uninferred-count.rs:22:9 +error[E0284]: type annotations needed for `[String; _]` + --> $DIR/copy-check-const-element-uninferred-count.rs:64:9 | LL | let a = [const { String::new() }; _]; - | ^ ----------------------- type must be known at this point + | ^ ---------------------------- type must be known at this point | -help: consider giving `a` an explicit type, where the value of const parameter `N` is specified + = note: the length of array `[String; _]` must be type `usize` +help: consider giving `a` an explicit type, where the placeholders `_` are specified | -LL | let a: [_; N] = [const { String::new() }; _]; +LL | let a: [_; _] = [const { String::new() }; _]; | ++++++++ -error[E0282]: type annotations needed for `[String; _]` - --> $DIR/copy-check-const-element-uninferred-count.rs:38:9 - | -LL | let a = [MY_CONST; _]; - | ^ -------- type must be known at this point - | -help: consider giving `a` an explicit type, where the value of const parameter `N` is specified - | -LL | let a: [_; N] = [MY_CONST; _]; - | ++++++++ - -error[E0282]: type annotations needed for `[String; _]` - --> $DIR/copy-check-const-element-uninferred-count.rs:56:9 - | -LL | let a = [<() as Dummy>::ASSOC; _]; - | ^ -------------------- type must be known at this point - | -help: consider giving `a` an explicit type, where the value of const parameter `N` is specified - | -LL | let a: [_; N] = [<() as Dummy>::ASSOC; _]; - | ++++++++ - -error: aborting due to 3 previous errors +error: aborting due to 1 previous error -For more information about this error, try `rustc --explain E0282`. +For more information about this error, try `rustc --explain E0284`. From 43162597292f904ff5ff47e251ba40fb9ae41ded Mon Sep 17 00:00:00 2001 From: Boxy Date: Mon, 14 Apr 2025 13:38:37 +0100 Subject: [PATCH 21/23] Anon consts cant appear as repeat expr elements --- compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index dc6a9dc93d07a..124b6e80fd44d 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -114,11 +114,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { hir::ExprKind::ConstBlock(..) => return None, hir::ExprKind::Path(qpath) => { let res = self.typeck_results.borrow().qpath_res(qpath, element.hir_id); - if let Res::Def( - DefKind::Const | DefKind::AssocConst | DefKind::AnonConst, - _, - ) = res - { + if let Res::Def(DefKind::Const | DefKind::AssocConst, _) = res { return None; } } From 8373bb17d6f9ccb00171f788bbc1379bffc61f68 Mon Sep 17 00:00:00 2001 From: bendn Date: Tue, 29 Apr 2025 13:51:45 +0700 Subject: [PATCH 22/23] use uX::from instead of _ as uX in non - const contexts --- .../src/check_unnecessary_transmutes.rs | 19 ++- .../transmute/unnecessary-transmutation.fixed | 24 +++- .../ui/transmute/unnecessary-transmutation.rs | 20 ++++ .../unnecessary-transmutation.stderr | 113 +++++++++++------- 4 files changed, 130 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_mir_transform/src/check_unnecessary_transmutes.rs b/compiler/rustc_mir_transform/src/check_unnecessary_transmutes.rs index 0514de3ea13fb..1a3715465ad4e 100644 --- a/compiler/rustc_mir_transform/src/check_unnecessary_transmutes.rs +++ b/compiler/rustc_mir_transform/src/check_unnecessary_transmutes.rs @@ -30,6 +30,7 @@ impl<'a, 'tcx> UnnecessaryTransmuteChecker<'a, 'tcx> { function: &Operand<'tcx>, arg: String, span: Span, + is_in_const: bool, ) -> Option { let fn_sig = function.ty(self.body, self.tcx).fn_sig(self.tcx).skip_binder(); let [input] = fn_sig.inputs() else { return None }; @@ -97,8 +98,14 @@ impl<'a, 'tcx> UnnecessaryTransmuteChecker<'a, 'tcx> { )), // uNN → fNN (Uint(_), Float(ty)) => err(format!("{}::from_bits({arg})", ty.name_str())), - // bool → { x8 } - (Bool, Int(..) | Uint(..)) => err(format!("({arg}) as {}", fn_sig.output())), + // bool → { x8 } in const context since `From::from` is not const yet + // FIXME: is it possible to know when the parentheses arent necessary? + // FIXME(const_traits): Remove this when From::from is constified? + (Bool, Int(..) | Uint(..)) if is_in_const => { + err(format!("({arg}) as {}", fn_sig.output())) + } + // " using `x8::from` + (Bool, Int(..) | Uint(..)) => err(format!("{}::from({arg})", fn_sig.output())), _ => return None, }) } @@ -114,7 +121,13 @@ impl<'tcx> Visitor<'tcx> for UnnecessaryTransmuteChecker<'_, 'tcx> { && self.tcx.is_intrinsic(func_def_id, sym::transmute) && let span = self.body.source_info(location).span && let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(arg) - && let Some(lint) = self.is_unnecessary_transmute(func, snippet, span) + && let def_id = self.body.source.def_id() + && let Some(lint) = self.is_unnecessary_transmute( + func, + snippet, + span, + self.tcx.hir_body_const_context(def_id.expect_local()).is_some(), + ) && let Some(hir_id) = terminator.source_info.scope.lint_root(&self.body.source_scopes) { self.tcx.emit_node_span_lint(UNNECESSARY_TRANSMUTES, hir_id, span, lint); diff --git a/tests/ui/transmute/unnecessary-transmutation.fixed b/tests/ui/transmute/unnecessary-transmutation.fixed index f6478c5aa5c2f..08010ec8b84e4 100644 --- a/tests/ui/transmute/unnecessary-transmutation.fixed +++ b/tests/ui/transmute/unnecessary-transmutation.fixed @@ -8,7 +8,27 @@ pub fn bytes_at_home(x: u32) -> [u8; 4] { //~^ ERROR } +pub const fn intinator_const(from: bool) -> u8 { + unsafe { (from) as u8 } + //~^ ERROR +} + +pub static X: u8 = unsafe { (true) as u8 }; +//~^ ERROR +pub const Y: u8 = unsafe { (true) as u8 }; +//~^ ERROR + +pub struct Z {} +impl Z { + pub const fn intinator_assoc(x: bool) -> u8 { + unsafe { (x) as u8 } + //~^ ERROR + } +} + fn main() { + const { unsafe { (true) as u8 } }; + //~^ ERROR unsafe { let x: u16 = u16::from_ne_bytes(*b"01"); //~^ ERROR @@ -83,12 +103,12 @@ fn main() { let z: bool = transmute(1u8); // clippy - let z: u8 = (z) as u8; + let z: u8 = u8::from(z); //~^ ERROR let z: bool = transmute(1i8); // clippy - let z: i8 = (z) as i8; + let z: i8 = i8::from(z); //~^ ERROR } } diff --git a/tests/ui/transmute/unnecessary-transmutation.rs b/tests/ui/transmute/unnecessary-transmutation.rs index ab0af03acc2ec..43eefb97dc28f 100644 --- a/tests/ui/transmute/unnecessary-transmutation.rs +++ b/tests/ui/transmute/unnecessary-transmutation.rs @@ -8,7 +8,27 @@ pub fn bytes_at_home(x: u32) -> [u8; 4] { //~^ ERROR } +pub const fn intinator_const(from: bool) -> u8 { + unsafe { transmute(from) } + //~^ ERROR +} + +pub static X: u8 = unsafe { transmute(true) }; +//~^ ERROR +pub const Y: u8 = unsafe { transmute(true) }; +//~^ ERROR + +pub struct Z {} +impl Z { + pub const fn intinator_assoc(x: bool) -> u8 { + unsafe { transmute(x) } + //~^ ERROR + } +} + fn main() { + const { unsafe { transmute::<_, u8>(true) } }; + //~^ ERROR unsafe { let x: u16 = transmute(*b"01"); //~^ ERROR diff --git a/tests/ui/transmute/unnecessary-transmutation.stderr b/tests/ui/transmute/unnecessary-transmutation.stderr index 59e933bbc81b9..602e964f5b2bd 100644 --- a/tests/ui/transmute/unnecessary-transmutation.stderr +++ b/tests/ui/transmute/unnecessary-transmutation.stderr @@ -1,10 +1,9 @@ error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:7:14 + --> $DIR/unnecessary-transmutation.rs:16:29 | -LL | unsafe { transmute(x) } - | ^^^^^^^^^^^^ help: replace this with: `u32::to_ne_bytes(x)` +LL | pub static X: u8 = unsafe { transmute(true) }; + | ^^^^^^^^^^^^^^^ help: replace this with: `(true) as u8` | - = help: there's also `to_le_bytes` and `to_be_bytes` if you expect a particular byte order note: the lint level is defined here --> $DIR/unnecessary-transmutation.rs:2:9 | @@ -12,7 +11,33 @@ LL | #![deny(unnecessary_transmutes)] | ^^^^^^^^^^^^^^^^^^^^^^ error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:13:22 + --> $DIR/unnecessary-transmutation.rs:18:28 + | +LL | pub const Y: u8 = unsafe { transmute(true) }; + | ^^^^^^^^^^^^^^^ help: replace this with: `(true) as u8` + +error: unnecessary transmute + --> $DIR/unnecessary-transmutation.rs:7:14 + | +LL | unsafe { transmute(x) } + | ^^^^^^^^^^^^ help: replace this with: `u32::to_ne_bytes(x)` + | + = help: there's also `to_le_bytes` and `to_be_bytes` if you expect a particular byte order + +error: unnecessary transmute + --> $DIR/unnecessary-transmutation.rs:12:14 + | +LL | unsafe { transmute(from) } + | ^^^^^^^^^^^^^^^ help: replace this with: `(from) as u8` + +error: unnecessary transmute + --> $DIR/unnecessary-transmutation.rs:24:18 + | +LL | unsafe { transmute(x) } + | ^^^^^^^^^^^^ help: replace this with: `(x) as u8` + +error: unnecessary transmute + --> $DIR/unnecessary-transmutation.rs:33:22 | LL | let x: u16 = transmute(*b"01"); | ^^^^^^^^^^^^^^^^^ help: replace this with: `u16::from_ne_bytes(*b"01")` @@ -20,7 +45,7 @@ LL | let x: u16 = transmute(*b"01"); = help: there's also `from_le_bytes` and `from_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:15:26 + --> $DIR/unnecessary-transmutation.rs:35:26 | LL | let x: [u8; 2] = transmute(x); | ^^^^^^^^^^^^ help: replace this with: `u16::to_ne_bytes(x)` @@ -28,7 +53,7 @@ LL | let x: [u8; 2] = transmute(x); = help: there's also `to_le_bytes` and `to_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:17:22 + --> $DIR/unnecessary-transmutation.rs:37:22 | LL | let x: u32 = transmute(*b"0123"); | ^^^^^^^^^^^^^^^^^^^ help: replace this with: `u32::from_ne_bytes(*b"0123")` @@ -36,7 +61,7 @@ LL | let x: u32 = transmute(*b"0123"); = help: there's also `from_le_bytes` and `from_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:19:26 + --> $DIR/unnecessary-transmutation.rs:39:26 | LL | let x: [u8; 4] = transmute(x); | ^^^^^^^^^^^^ help: replace this with: `u32::to_ne_bytes(x)` @@ -44,7 +69,7 @@ LL | let x: [u8; 4] = transmute(x); = help: there's also `to_le_bytes` and `to_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:21:22 + --> $DIR/unnecessary-transmutation.rs:41:22 | LL | let x: u64 = transmute(*b"feriscat"); | ^^^^^^^^^^^^^^^^^^^^^^^ help: replace this with: `u64::from_ne_bytes(*b"feriscat")` @@ -52,7 +77,7 @@ LL | let x: u64 = transmute(*b"feriscat"); = help: there's also `from_le_bytes` and `from_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:23:26 + --> $DIR/unnecessary-transmutation.rs:43:26 | LL | let x: [u8; 8] = transmute(x); | ^^^^^^^^^^^^ help: replace this with: `u64::to_ne_bytes(x)` @@ -60,7 +85,7 @@ LL | let x: [u8; 8] = transmute(x); = help: there's also `to_le_bytes` and `to_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:26:22 + --> $DIR/unnecessary-transmutation.rs:46:22 | LL | let y: i16 = transmute(*b"01"); | ^^^^^^^^^^^^^^^^^ help: replace this with: `i16::from_ne_bytes(*b"01")` @@ -68,7 +93,7 @@ LL | let y: i16 = transmute(*b"01"); = help: there's also `from_le_bytes` and `from_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:28:26 + --> $DIR/unnecessary-transmutation.rs:48:26 | LL | let y: [u8; 2] = transmute(y); | ^^^^^^^^^^^^ help: replace this with: `i16::to_ne_bytes(y)` @@ -76,7 +101,7 @@ LL | let y: [u8; 2] = transmute(y); = help: there's also `to_le_bytes` and `to_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:30:22 + --> $DIR/unnecessary-transmutation.rs:50:22 | LL | let y: i32 = transmute(*b"0123"); | ^^^^^^^^^^^^^^^^^^^ help: replace this with: `i32::from_ne_bytes(*b"0123")` @@ -84,7 +109,7 @@ LL | let y: i32 = transmute(*b"0123"); = help: there's also `from_le_bytes` and `from_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:32:26 + --> $DIR/unnecessary-transmutation.rs:52:26 | LL | let y: [u8; 4] = transmute(y); | ^^^^^^^^^^^^ help: replace this with: `i32::to_ne_bytes(y)` @@ -92,7 +117,7 @@ LL | let y: [u8; 4] = transmute(y); = help: there's also `to_le_bytes` and `to_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:34:22 + --> $DIR/unnecessary-transmutation.rs:54:22 | LL | let y: i64 = transmute(*b"feriscat"); | ^^^^^^^^^^^^^^^^^^^^^^^ help: replace this with: `i64::from_ne_bytes(*b"feriscat")` @@ -100,7 +125,7 @@ LL | let y: i64 = transmute(*b"feriscat"); = help: there's also `from_le_bytes` and `from_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:36:26 + --> $DIR/unnecessary-transmutation.rs:56:26 | LL | let y: [u8; 8] = transmute(y); | ^^^^^^^^^^^^ help: replace this with: `i64::to_ne_bytes(y)` @@ -108,7 +133,7 @@ LL | let y: [u8; 8] = transmute(y); = help: there's also `to_le_bytes` and `to_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:39:22 + --> $DIR/unnecessary-transmutation.rs:59:22 | LL | let z: f32 = transmute(*b"0123"); | ^^^^^^^^^^^^^^^^^^^ help: replace this with: `f32::from_ne_bytes(*b"0123")` @@ -116,7 +141,7 @@ LL | let z: f32 = transmute(*b"0123"); = help: there's also `from_le_bytes` and `from_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:41:26 + --> $DIR/unnecessary-transmutation.rs:61:26 | LL | let z: [u8; 4] = transmute(z); | ^^^^^^^^^^^^ help: replace this with: `f32::to_ne_bytes(z)` @@ -124,7 +149,7 @@ LL | let z: [u8; 4] = transmute(z); = help: there's also `to_le_bytes` and `to_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:43:22 + --> $DIR/unnecessary-transmutation.rs:63:22 | LL | let z: f64 = transmute(*b"feriscat"); | ^^^^^^^^^^^^^^^^^^^^^^^ help: replace this with: `f64::from_ne_bytes(*b"feriscat")` @@ -132,7 +157,7 @@ LL | let z: f64 = transmute(*b"feriscat"); = help: there's also `from_le_bytes` and `from_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:45:26 + --> $DIR/unnecessary-transmutation.rs:65:26 | LL | let z: [u8; 8] = transmute(z); | ^^^^^^^^^^^^ help: replace this with: `f64::to_ne_bytes(z)` @@ -140,13 +165,13 @@ LL | let z: [u8; 8] = transmute(z); = help: there's also `to_le_bytes` and `to_be_bytes` if you expect a particular byte order error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:48:22 + --> $DIR/unnecessary-transmutation.rs:68:22 | LL | let y: u32 = transmute('🦀'); | ^^^^^^^^^^^^^^^ help: replace this with: `u32::from('🦀')` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:50:23 + --> $DIR/unnecessary-transmutation.rs:70:23 | LL | let y: char = transmute(y); | ^^^^^^^^^^^^ help: replace this with: `char::from_u32_unchecked(y)` @@ -154,13 +179,13 @@ LL | let y: char = transmute(y); = help: consider `char::from_u32(…).unwrap()` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:52:22 + --> $DIR/unnecessary-transmutation.rs:72:22 | LL | let y: i32 = transmute('🐱'); | ^^^^^^^^^^^^^^^ help: replace this with: `u32::from('🐱').cast_signed()` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:54:23 + --> $DIR/unnecessary-transmutation.rs:74:23 | LL | let y: char = transmute(y); | ^^^^^^^^^^^^ help: replace this with: `char::from_u32_unchecked(i32::cast_unsigned(y))` @@ -168,88 +193,94 @@ LL | let y: char = transmute(y); = help: consider `char::from_u32(i32::cast_unsigned(…)).unwrap()` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:57:22 + --> $DIR/unnecessary-transmutation.rs:77:22 | LL | let x: u16 = transmute(8i16); | ^^^^^^^^^^^^^^^ help: replace this with: `i16::cast_unsigned(8i16)` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:59:22 + --> $DIR/unnecessary-transmutation.rs:79:22 | LL | let x: i16 = transmute(x); | ^^^^^^^^^^^^ help: replace this with: `u16::cast_signed(x)` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:61:22 + --> $DIR/unnecessary-transmutation.rs:81:22 | LL | let x: u32 = transmute(4i32); | ^^^^^^^^^^^^^^^ help: replace this with: `i32::cast_unsigned(4i32)` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:63:22 + --> $DIR/unnecessary-transmutation.rs:83:22 | LL | let x: i32 = transmute(x); | ^^^^^^^^^^^^ help: replace this with: `u32::cast_signed(x)` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:65:22 + --> $DIR/unnecessary-transmutation.rs:85:22 | LL | let x: u64 = transmute(7i64); | ^^^^^^^^^^^^^^^ help: replace this with: `i64::cast_unsigned(7i64)` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:67:22 + --> $DIR/unnecessary-transmutation.rs:87:22 | LL | let x: i64 = transmute(x); | ^^^^^^^^^^^^ help: replace this with: `u64::cast_signed(x)` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:70:22 + --> $DIR/unnecessary-transmutation.rs:90:22 | LL | let y: f32 = transmute(1u32); | ^^^^^^^^^^^^^^^ help: replace this with: `f32::from_bits(1u32)` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:72:22 + --> $DIR/unnecessary-transmutation.rs:92:22 | LL | let y: u32 = transmute(y); | ^^^^^^^^^^^^ help: replace this with: `f32::to_bits(y)` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:74:22 + --> $DIR/unnecessary-transmutation.rs:94:22 | LL | let y: f64 = transmute(3u64); | ^^^^^^^^^^^^^^^ help: replace this with: `f64::from_bits(3u64)` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:76:22 + --> $DIR/unnecessary-transmutation.rs:96:22 | LL | let y: u64 = transmute(2.0); | ^^^^^^^^^^^^^^ help: replace this with: `f64::to_bits(2.0)` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:79:22 + --> $DIR/unnecessary-transmutation.rs:99:22 | LL | let y: f64 = transmute(1i64); | ^^^^^^^^^^^^^^^ help: replace this with: `f64::from_bits(i64::cast_unsigned(1i64))` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:81:22 + --> $DIR/unnecessary-transmutation.rs:101:22 | LL | let y: i64 = transmute(1f64); | ^^^^^^^^^^^^^^^ help: replace this with: `f64::to_bits(1f64).cast_signed()` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:86:21 + --> $DIR/unnecessary-transmutation.rs:106:21 | LL | let z: u8 = transmute(z); - | ^^^^^^^^^^^^ help: replace this with: `(z) as u8` + | ^^^^^^^^^^^^ help: replace this with: `u8::from(z)` error: unnecessary transmute - --> $DIR/unnecessary-transmutation.rs:91:21 + --> $DIR/unnecessary-transmutation.rs:111:21 | LL | let z: i8 = transmute(z); - | ^^^^^^^^^^^^ help: replace this with: `(z) as i8` + | ^^^^^^^^^^^^ help: replace this with: `i8::from(z)` + +error: unnecessary transmute + --> $DIR/unnecessary-transmutation.rs:30:22 + | +LL | const { unsafe { transmute::<_, u8>(true) } }; + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace this with: `(true) as u8` -error: aborting due to 35 previous errors +error: aborting due to 40 previous errors From cc10370fc8b2b412600aa846e3046b8a09dda7ca Mon Sep 17 00:00:00 2001 From: Boxy Date: Mon, 14 Apr 2025 13:41:18 +0100 Subject: [PATCH 23/23] Reviews --- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 38 ++++++++++--------- compiler/rustc_hir_typeck/src/lib.rs | 15 +++++--- .../copy-check-deferred-before-fallback.rs | 2 +- .../copy-check-inference-side-effects.rs | 2 +- 4 files changed, 32 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 124b6e80fd44d..4da014dd23626 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -108,8 +108,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let deferred_repeat_expr_checks = deferred_repeat_expr_checks .drain(..) .flat_map(|(element, element_ty, count)| { - // Actual constants as the repeat element get inserted repeatedly instead of getting copied via Copy - // so we don't need to attempt to structurally resolve the repeat count which may unnecessarily error. + // Actual constants as the repeat element are inserted repeatedly instead + // of being copied via `Copy`, so we don't need to attempt to structurally + // resolve the repeat count which may unnecessarily error. match &element.kind { hir::ExprKind::ConstBlock(..) => return None, hir::ExprKind::Path(qpath) => { @@ -121,23 +122,26 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { _ => {} } - // We want to emit an error if the const is not structurally resolveable as otherwise - // we can find up conservatively proving `Copy` which may infer the repeat expr count - // to something that never required `Copy` in the first place. + // We want to emit an error if the const is not structurally resolveable + // as otherwise we can wind up conservatively proving `Copy` which may + // infer the repeat expr count to something that never required `Copy` in + // the first place. let count = self .structurally_resolve_const(element.span, self.normalize(element.span, count)); - // Avoid run on "`NotCopy: Copy` is not implemented" errors when the repeat expr count - // is erroneous/unknown. The user might wind up specifying a repeat count of 0/1. + // Avoid run on "`NotCopy: Copy` is not implemented" errors when the + // repeat expr count is erroneous/unknown. The user might wind up + // specifying a repeat count of 0/1. if count.references_error() { return None; } Some((element, element_ty, count)) }) - // We collect to force the side effects of structurally resolving the repeat count to happen in one - // go, to avoid side effects from proving `Copy` affecting whether repeat counts are known or not. - // If we did not do this we would get results that depend on the order that we evaluate each repeat + // We collect to force the side effects of structurally resolving the repeat + // count to happen in one go, to avoid side effects from proving `Copy` + // affecting whether repeat counts are known or not. If we did not do this we + // would get results that depend on the order that we evaluate each repeat // expr's `Copy` check. .collect::>(); @@ -171,14 +175,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { for (element, element_ty, count) in deferred_repeat_expr_checks { match count.kind() { - ty::ConstKind::Value(val) - if val.try_to_target_usize(self.tcx).is_none_or(|count| count > 1) => - { - enforce_copy_bound(element, element_ty) + ty::ConstKind::Value(val) => { + if val.try_to_target_usize(self.tcx).is_none_or(|count| count > 1) { + enforce_copy_bound(element, element_ty) + } else { + // If the length is 0 or 1 we don't actually copy the element, we either don't create it + // or we just use the one value. + } } - // If the length is 0 or 1 we don't actually copy the element, we either don't create it - // or we just use the one value. - ty::ConstKind::Value(_) => (), // If the length is a generic parameter or some rigid alias then conservatively // require `element_ty: Copy` as it may wind up being `>1` after monomorphization. diff --git a/compiler/rustc_hir_typeck/src/lib.rs b/compiler/rustc_hir_typeck/src/lib.rs index 161f5e981d430..ba83db7eebd00 100644 --- a/compiler/rustc_hir_typeck/src/lib.rs +++ b/compiler/rustc_hir_typeck/src/lib.rs @@ -195,13 +195,16 @@ fn typeck_with_inspect<'tcx>( fcx.write_ty(id, expected_type); }; - // Whether to check repeat exprs before/after inference fallback is somewhat arbitrary of a decision - // as neither option is strictly more permissive than the other. However, we opt to check repeat exprs - // first as errors from not having inferred array lengths yet seem less confusing than errors from inference - // fallback arbitrarily inferring something incompatible with `Copy` inference side effects. + // Whether to check repeat exprs before/after inference fallback is somewhat + // arbitrary of a decision as neither option is strictly more permissive than + // the other. However, we opt to check repeat exprs first as errors from not + // having inferred array lengths yet seem less confusing than errors from inference + // fallback arbitrarily inferring something incompatible with `Copy` inference + // side effects. // - // This should also be forwards compatible with moving repeat expr checks to a custom goal kind or using - // marker traits in the future. + // FIXME(#140855): This should also be forwards compatible with moving + // repeat expr checks to a custom goal kind or using marker traits in + // the future. fcx.check_repeat_exprs(); fcx.type_inference_fallback(); diff --git a/tests/ui/repeat-expr/copy-check-deferred-before-fallback.rs b/tests/ui/repeat-expr/copy-check-deferred-before-fallback.rs index b81997a3c9fa3..4fbb8f0a00caf 100644 --- a/tests/ui/repeat-expr/copy-check-deferred-before-fallback.rs +++ b/tests/ui/repeat-expr/copy-check-deferred-before-fallback.rs @@ -5,7 +5,7 @@ // checked before integer fallback occurs. We accomplish this by having the repeat // expr check allow inference progress on an ambiguous goal, where the ambiguous goal // would fail if the inference variable was fallen back to `i32`. This test will -// pass if wecheck repeat exprs before integer fallback. +// pass if we check repeat exprs before integer fallback. use std::marker::PhantomData; struct Foo(PhantomData); diff --git a/tests/ui/repeat-expr/copy-check-inference-side-effects.rs b/tests/ui/repeat-expr/copy-check-inference-side-effects.rs index 416a20169fba1..4e3bfdead26b4 100644 --- a/tests/ui/repeat-expr/copy-check-inference-side-effects.rs +++ b/tests/ui/repeat-expr/copy-check-inference-side-effects.rs @@ -12,7 +12,7 @@ impl Copy for Foo<1> {} fn unify(_: &[Foo; 2], _: &[String; N]) {} fn works_if_inference_side_effects() { - // This will only pass if inference side effectrs from proving `Foo: Copy` are + // This will only pass if inference side effects from proving `Foo: Copy` are // able to be relied upon by other repeat expressions. let a /* : [Foo; 2] */ = [Foo::<_>; 2]; //~^ ERROR: type annotations needed for `[Foo<_>; 2]`