From 7fc9e95673de47b7c9096e7a3d77102fba4ae405 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 5 Jun 2022 13:06:37 -0700 Subject: [PATCH 1/5] Make Ty::is_suggestable use a visitor --- compiler/rustc_middle/src/ty/diagnostics.rs | 138 ++++++++++---------- 1 file changed, 69 insertions(+), 69 deletions(-) diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs index 9bb64d4023bca..e4e575542c2e5 100644 --- a/compiler/rustc_middle/src/ty/diagnostics.rs +++ b/compiler/rustc_middle/src/ty/diagnostics.rs @@ -1,9 +1,10 @@ //! Diagnostics related methods for `Ty`. -use crate::ty::subst::{GenericArg, GenericArgKind}; +use std::ops::ControlFlow; + use crate::ty::{ - ConstKind, DefIdTree, ExistentialPredicate, ExistentialProjection, ExistentialTraitRef, - InferTy, ProjectionTy, Term, Ty, TyCtxt, TypeAndMut, + fold::TypeFoldable, Const, ConstKind, DefIdTree, ExistentialPredicate, InferTy, Ty, TyCtxt, + TypeVisitor, }; use rustc_data_structures::fx::FxHashMap; @@ -75,72 +76,7 @@ impl<'tcx> Ty<'tcx> { /// Whether the type can be safely suggested during error recovery. pub fn is_suggestable(self, tcx: TyCtxt<'tcx>) -> bool { - fn generic_arg_is_suggestible<'tcx>(arg: GenericArg<'tcx>, tcx: TyCtxt<'tcx>) -> bool { - match arg.unpack() { - GenericArgKind::Type(ty) => ty.is_suggestable(tcx), - GenericArgKind::Const(c) => const_is_suggestable(c.val()), - _ => true, - } - } - - fn const_is_suggestable(kind: ConstKind<'_>) -> bool { - match kind { - ConstKind::Infer(..) - | ConstKind::Bound(..) - | ConstKind::Placeholder(..) - | ConstKind::Error(..) => false, - _ => true, - } - } - - // FIXME(compiler-errors): Some types are still not good to suggest, - // specifically references with lifetimes within the function. Not - //sure we have enough information to resolve whether a region is - // temporary, so I'll leave this as a fixme. - - match self.kind() { - FnDef(..) - | Closure(..) - | Infer(..) - | Generator(..) - | GeneratorWitness(..) - | Bound(_, _) - | Placeholder(_) - | Error(_) => false, - Opaque(did, substs) => { - let parent = tcx.parent(*did); - if let hir::def::DefKind::TyAlias | hir::def::DefKind::AssocTy = tcx.def_kind(parent) - && let Opaque(parent_did, _) = tcx.type_of(parent).kind() - && parent_did == did - { - substs.iter().all(|a| generic_arg_is_suggestible(a, tcx)) - } else { - false - } - } - Dynamic(dty, _) => dty.iter().all(|pred| match pred.skip_binder() { - ExistentialPredicate::Trait(ExistentialTraitRef { substs, .. }) => { - substs.iter().all(|a| generic_arg_is_suggestible(a, tcx)) - } - ExistentialPredicate::Projection(ExistentialProjection { - substs, term, .. - }) => { - let term_is_suggestable = match term { - Term::Ty(ty) => ty.is_suggestable(tcx), - Term::Const(c) => const_is_suggestable(c.val()), - }; - term_is_suggestable && substs.iter().all(|a| generic_arg_is_suggestible(a, tcx)) - } - _ => true, - }), - Projection(ProjectionTy { substs: args, .. }) | Adt(_, args) => { - args.iter().all(|a| generic_arg_is_suggestible(a, tcx)) - } - Tuple(args) => args.iter().all(|ty| ty.is_suggestable(tcx)), - Slice(ty) | RawPtr(TypeAndMut { ty, .. }) | Ref(_, ty, _) => ty.is_suggestable(tcx), - Array(ty, c) => ty.is_suggestable(tcx) && const_is_suggestable(c.val()), - _ => true, - } + self.visit_with(&mut IsSuggestableVisitor { tcx }).is_continue() } } @@ -463,3 +399,67 @@ impl<'v> hir::intravisit::Visitor<'v> for StaticLifetimeVisitor<'v> { } } } + +pub struct IsSuggestableVisitor<'tcx> { + tcx: TyCtxt<'tcx>, +} + +impl<'tcx> TypeVisitor<'tcx> for IsSuggestableVisitor<'tcx> { + type BreakTy = (); + + fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow { + match t.kind() { + FnDef(..) + | Closure(..) + | Infer(..) + | Generator(..) + | GeneratorWitness(..) + | Bound(_, _) + | Placeholder(_) + | Error(_) => { + return ControlFlow::Break(()); + } + + Opaque(did, _) => { + let parent = self.tcx.parent(*did); + if let hir::def::DefKind::TyAlias | hir::def::DefKind::AssocTy = self.tcx.def_kind(parent) + && let Opaque(parent_did, _) = self.tcx.type_of(parent).kind() + && parent_did == did + { + // Okay + } else { + return ControlFlow::Break(()); + } + } + + Dynamic(dty, _) => { + for pred in *dty { + match pred.skip_binder() { + ExistentialPredicate::Trait(_) | ExistentialPredicate::Projection(_) => { + // Okay + } + _ => return ControlFlow::Break(()), + } + } + } + + _ => {} + } + + t.super_visit_with(self) + } + + fn visit_const(&mut self, c: Const<'tcx>) -> ControlFlow { + match c.val() { + ConstKind::Infer(..) + | ConstKind::Bound(..) + | ConstKind::Placeholder(..) + | ConstKind::Error(..) => { + return ControlFlow::Break(()); + } + _ => {} + } + + c.super_visit_with(self) + } +} From e848eca2b6de6c7ea277c82194e085a3feb661a8 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 5 Jun 2022 17:37:45 -0700 Subject: [PATCH 2/5] Handle empty where-clause better --- compiler/rustc_ast_lowering/src/item.rs | 6 +++-- compiler/rustc_ast_lowering/src/lib.rs | 6 +++-- compiler/rustc_hir/src/hir.rs | 26 ++++++++++++------- .../src/infer/error_reporting/mod.rs | 2 +- .../src/infer/error_reporting/note.rs | 11 +++----- compiler/rustc_lint/src/builtin.rs | 3 ++- compiler/rustc_middle/src/ty/diagnostics.rs | 13 +++------- .../src/traits/error_reporting/suggestions.rs | 11 ++++---- .../rustc_typeck/src/check/method/suggest.rs | 12 ++++----- compiler/rustc_typeck/src/check/wfcheck.rs | 2 +- src/test/ui/issues/issue-35668.stderr | 2 +- src/test/ui/partialeq_help.rs | 5 ++++ src/test/ui/partialeq_help.stderr | 16 ++++++++++-- .../assoc-type.stderr | 2 +- ...-method-body-is-const-body-checking.stderr | 2 +- .../default-associated-type-bound-2.stderr | 2 +- .../derive-macro-missing-bounds.stderr | 2 +- src/test/ui/suggestions/invalid-bin-op.stderr | 2 +- src/test/ui/suggestions/suggest-change-mut.rs | 2 +- .../ui/suggestions/suggest-change-mut.stderr | 2 +- .../traits/resolution-in-overloaded-op.stderr | 2 +- .../ui/traits/suggest-where-clause.stderr | 4 +-- .../multiple-def-uses-in-one-fn.stderr | 2 +- 23 files changed, 78 insertions(+), 59 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index dab4d76857a50..84eab512f7fe1 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -1376,7 +1376,8 @@ impl<'hir> LoweringContext<'_, 'hir> { let mut params: SmallVec<[hir::GenericParam<'hir>; 4]> = self.lower_generic_params_mut(&generics.params).collect(); - let has_where_clause = !generics.where_clause.predicates.is_empty(); + let has_where_clause_predicates = !generics.where_clause.predicates.is_empty(); + let has_where_clause_token = generics.where_clause.has_where_token; let where_clause_span = self.lower_span(generics.where_clause.span); let span = self.lower_span(generics.span); let res = f(self); @@ -1394,7 +1395,8 @@ impl<'hir> LoweringContext<'_, 'hir> { let lowered_generics = self.arena.alloc(hir::Generics { params: self.arena.alloc_from_iter(params), predicates: self.arena.alloc_from_iter(predicates), - has_where_clause, + has_where_clause_predicates, + has_where_clause_token, where_clause_span, span, }); diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 51e5c3384a791..c4ba9960a9b5b 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -1332,7 +1332,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { generics: self.arena.alloc(hir::Generics { params: lifetime_defs, predicates: &[], - has_where_clause: false, + has_where_clause_predicates: false, + has_where_clause_token: false, where_clause_span: lctx.lower_span(span), span: lctx.lower_span(span), }), @@ -1654,7 +1655,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { generics: this.arena.alloc(hir::Generics { params: generic_params, predicates: &[], - has_where_clause: false, + has_where_clause_predicates: false, + has_where_clause_token: false, where_clause_span: this.lower_span(span), span: this.lower_span(span), }), diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index cb2e66090e7c3..06f7b0af09488 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -535,7 +535,8 @@ pub struct GenericParamCount { pub struct Generics<'hir> { pub params: &'hir [GenericParam<'hir>], pub predicates: &'hir [WherePredicate<'hir>], - pub has_where_clause: bool, + pub has_where_clause_predicates: bool, + pub has_where_clause_token: bool, pub where_clause_span: Span, pub span: Span, } @@ -545,7 +546,8 @@ impl<'hir> Generics<'hir> { const NOPE: Generics<'_> = Generics { params: &[], predicates: &[], - has_where_clause: false, + has_where_clause_predicates: false, + has_where_clause_token: false, where_clause_span: DUMMY_SP, span: DUMMY_SP, }; @@ -585,17 +587,11 @@ impl<'hir> Generics<'hir> { if self.predicates.is_empty() { None } else { Some(self.where_clause_span) } } - /// The `where_span` under normal circumstances points at either the predicates or the empty - /// space where the `where` clause should be. Only of use for diagnostic suggestions. - pub fn span_for_predicates_or_empty_place(&self) -> Span { - self.where_clause_span - } - /// `Span` where further predicates would be suggested, accounting for trailing commas, like /// in `fn foo(t: T) where T: Foo,` so we don't suggest two trailing commas. pub fn tail_span_for_predicate_suggestion(&self) -> Span { - let end = self.span_for_predicates_or_empty_place().shrink_to_hi(); - if self.has_where_clause { + let end = self.where_clause_span.shrink_to_hi(); + if self.has_where_clause_predicates { self.predicates .iter() .filter(|p| p.in_where_clause()) @@ -608,6 +604,16 @@ impl<'hir> Generics<'hir> { } } + pub fn add_where_or_trailing_comma(&self) -> &'static str { + if self.has_where_clause_predicates { + "," + } else if self.has_where_clause_token { + "" + } else { + " where" + } + } + pub fn bounds_for_param( &self, param_def_id: LocalDefId, diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 579d7efb56803..e461efc1cda79 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -2547,7 +2547,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let pred = format!("{}: {}", bound_kind, sub); let suggestion = format!( "{} {}", - if !generics.predicates.is_empty() { "," } else { " where" }, + generics.add_where_or_trailing_comma(), pred, ); err.span_suggestion( diff --git a/compiler/rustc_infer/src/infer/error_reporting/note.rs b/compiler/rustc_infer/src/infer/error_reporting/note.rs index cbdcf01352271..67bbace39e3ca 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/note.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/note.rs @@ -367,17 +367,12 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { .collect(); if !clauses.is_empty() { - let where_clause_span = self - .tcx - .hir() - .get_generics(impl_item_def_id) - .unwrap() - .where_clause_span - .shrink_to_hi(); + let generics = self.tcx.hir().get_generics(impl_item_def_id).unwrap(); + let where_clause_span = generics.tail_span_for_predicate_suggestion(); let suggestion = format!( "{} {}", - if !impl_predicates.is_empty() { "," } else { " where" }, + generics.add_where_or_trailing_comma(), clauses.join(", "), ); err.span_suggestion( diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 6be78c52f99e3..f8154fe33375f 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -2264,7 +2264,8 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements { // If all predicates are inferable, drop the entire clause // (including the `where`) - if hir_generics.has_where_clause && dropped_predicate_count == num_predicates { + if hir_generics.has_where_clause_predicates && dropped_predicate_count == num_predicates + { let where_span = hir_generics .where_clause_span() .expect("span of (nonempty) where clause should exist"); diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs index e4e575542c2e5..6fbe6b8550ca2 100644 --- a/compiler/rustc_middle/src/ty/diagnostics.rs +++ b/compiler/rustc_middle/src/ty/diagnostics.rs @@ -92,19 +92,14 @@ pub fn suggest_arbitrary_trait_bound( _ => {} } // Suggest a where clause bound for a non-type parameter. - let (action, prefix) = if generics.has_where_clause { - ("extending the", ", ") - } else { - ("introducing a", " where ") - }; err.span_suggestion_verbose( generics.tail_span_for_predicate_suggestion(), &format!( - "consider {} `where` bound, but there might be an alternative better way to express \ + "consider {} `where` clause, but there might be an alternative better way to express \ this requirement", - action, + if generics.has_where_clause_token { "extending the" } else { "introducing a" }, ), - format!("{}{}: {}", prefix, param_name, constraint), + format!("{} {}: {}", generics.add_where_or_trailing_comma(), param_name, constraint), Applicability::MaybeIncorrect, ); true @@ -257,7 +252,7 @@ pub fn suggest_constraining_type_params<'a>( continue; } - if generics.has_where_clause { + if generics.has_where_clause_predicates { // This part is a bit tricky, because using the `where` clause user can // provide zero, one or many bounds for the same type parameter, so we // have following cases to consider: diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index d0f20022bfbad..50a3eb8c20dd7 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -322,7 +322,7 @@ pub trait InferCtxtExt<'tcx> { fn predicate_constraint(generics: &hir::Generics<'_>, pred: String) -> (Span, String) { ( generics.tail_span_for_predicate_suggestion(), - format!("{} {}", if generics.has_where_clause { "," } else { " where" }, pred,), + format!("{} {}", generics.add_where_or_trailing_comma(), pred), ) } @@ -337,15 +337,16 @@ fn suggest_restriction<'tcx>( fn_sig: Option<&hir::FnSig<'_>>, projection: Option<&ty::ProjectionTy<'_>>, trait_pred: ty::PolyTraitPredicate<'tcx>, - super_traits: Option<(&Ident, &hir::GenericBounds<'_>)>, -) { // When we are dealing with a trait, `super_traits` will be `Some`: // Given `trait T: A + B + C {}` // - ^^^^^^^^^ GenericBounds // | // &Ident - let span = generics.span_for_predicates_or_empty_place(); - if span.from_expansion() || span.desugaring_kind().is_some() { + super_traits: Option<(&Ident, &hir::GenericBounds<'_>)>, +) { + if generics.where_clause_span.from_expansion() + || generics.where_clause_span.desugaring_kind().is_some() + { return; } // Given `fn foo(t: impl Trait)` where `Trait` requires assoc type `A`... diff --git a/compiler/rustc_typeck/src/check/method/suggest.rs b/compiler/rustc_typeck/src/check/method/suggest.rs index 0e198907c8d50..0b0320ff4c53c 100644 --- a/compiler/rustc_typeck/src/check/method/suggest.rs +++ b/compiler/rustc_typeck/src/check/method/suggest.rs @@ -542,10 +542,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }; if let Some(hir::Node::Item(hir::Item { kind, .. })) = node { if let Some(g) = kind.generics() { - let key = match g.predicates { - [.., pred] => (pred.span().shrink_to_hi(), false), - [] => (g.span_for_predicates_or_empty_place(), true), - }; + let key = ( + g.tail_span_for_predicate_suggestion(), + g.add_where_or_trailing_comma(), + ); type_params .entry(key) .or_insert_with(FxHashSet::default) @@ -809,7 +809,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .enumerate() .collect::>(); - for ((span, empty_where), obligations) in type_params.into_iter() { + for ((span, add_where_or_comma), obligations) in type_params.into_iter() { restrict_type_params = true; // #74886: Sort here so that the output is always the same. let mut obligations = obligations.into_iter().collect::>(); @@ -823,7 +823,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ), format!( "{} {}", - if empty_where { " where" } else { "," }, + add_where_or_comma, obligations.join(", ") ), Applicability::MaybeIncorrect, diff --git a/compiler/rustc_typeck/src/check/wfcheck.rs b/compiler/rustc_typeck/src/check/wfcheck.rs index 20ef97c085f18..413eb72af1c18 100644 --- a/compiler/rustc_typeck/src/check/wfcheck.rs +++ b/compiler/rustc_typeck/src/check/wfcheck.rs @@ -421,7 +421,7 @@ fn check_gat_where_clauses(tcx: TyCtxt<'_>, associated_items: &[hir::TraitItemRe let suggestion = format!( "{} {}", - if !gat_item_hir.generics.predicates.is_empty() { "," } else { " where" }, + gat_item_hir.generics.add_where_or_trailing_comma(), unsatisfied_bounds.join(", "), ); err.span_suggestion( diff --git a/src/test/ui/issues/issue-35668.stderr b/src/test/ui/issues/issue-35668.stderr index 07409e9834a46..84add5799abae 100644 --- a/src/test/ui/issues/issue-35668.stderr +++ b/src/test/ui/issues/issue-35668.stderr @@ -6,7 +6,7 @@ LL | a.iter().map(|a| a*a) | | | &T | -help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement +help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement | LL | fn func<'a, T>(a: &'a [T]) -> impl Iterator where &T: Mul<&T> { | +++++++++++++++++ diff --git a/src/test/ui/partialeq_help.rs b/src/test/ui/partialeq_help.rs index c3ba805405b33..34b88b8a86685 100644 --- a/src/test/ui/partialeq_help.rs +++ b/src/test/ui/partialeq_help.rs @@ -2,6 +2,11 @@ fn foo(a: &T, b: T) { a == b; //~ ERROR E0277 } +fn foo2(a: &T, b: T) where { + a == b; //~ ERROR E0277 +} + fn main() { foo(&1, 1); + foo2(&1, 1); } diff --git a/src/test/ui/partialeq_help.stderr b/src/test/ui/partialeq_help.stderr index 528306b22dd5a..fdff94f425c8a 100644 --- a/src/test/ui/partialeq_help.stderr +++ b/src/test/ui/partialeq_help.stderr @@ -5,11 +5,23 @@ LL | a == b; | ^^ no implementation for `&T == T` | = help: the trait `PartialEq` is not implemented for `&T` -help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement +help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement | LL | fn foo(a: &T, b: T) where &T: PartialEq { | ++++++++++++++++++++++ -error: aborting due to previous error +error[E0277]: can't compare `&T` with `T` + --> $DIR/partialeq_help.rs:6:7 + | +LL | a == b; + | ^^ no implementation for `&T == T` + | + = help: the trait `PartialEq` is not implemented for `&T` +help: consider extending the `where` clause, but there might be an alternative better way to express this requirement + | +LL | fn foo2(a: &T, b: T) where &T: PartialEq { + | ++++++++++++++++ + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/rfc-2632-const-trait-impl/assoc-type.stderr b/src/test/ui/rfc-2632-const-trait-impl/assoc-type.stderr index 0788b17a1c032..64501c5237419 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/assoc-type.stderr +++ b/src/test/ui/rfc-2632-const-trait-impl/assoc-type.stderr @@ -15,7 +15,7 @@ note: required by a bound in `Foo::Bar` | LL | type Bar: ~const std::ops::Add; | ^^^^^^^^^^^^^^^^^^^^ required by this bound in `Foo::Bar` -help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement +help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement | LL | impl const Foo for NonConstAdd where NonConstAdd: ~const Add { | +++++++++++++++++++++++++++++ diff --git a/src/test/ui/rfc-2632-const-trait-impl/default-method-body-is-const-body-checking.stderr b/src/test/ui/rfc-2632-const-trait-impl/default-method-body-is-const-body-checking.stderr index 668e166c29897..7542b81fe2adb 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/default-method-body-is-const-body-checking.stderr +++ b/src/test/ui/rfc-2632-const-trait-impl/default-method-body-is-const-body-checking.stderr @@ -14,7 +14,7 @@ note: required by a bound in `foo` | LL | const fn foo() where T: ~const Tr {} | ^^^^^^^^^ required by this bound in `foo` -help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement +help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement | LL | pub trait Foo where (): ~const Tr { | +++++++++++++++++++ diff --git a/src/test/ui/specialization/default-associated-type-bound-2.stderr b/src/test/ui/specialization/default-associated-type-bound-2.stderr index 0fd1f65b0a201..91778ed0f4cde 100644 --- a/src/test/ui/specialization/default-associated-type-bound-2.stderr +++ b/src/test/ui/specialization/default-associated-type-bound-2.stderr @@ -20,7 +20,7 @@ note: required by a bound in `X::U` | LL | type U: PartialEq; | ^^^^^^^^^^^^ required by this bound in `X::U` -help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement +help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement | LL | impl X for T where &'static B: PartialEq { | ++++++++++++++++++++++++++++++ diff --git a/src/test/ui/suggestions/derive-macro-missing-bounds.stderr b/src/test/ui/suggestions/derive-macro-missing-bounds.stderr index 75658f58c8a1b..501d083e2bc60 100644 --- a/src/test/ui/suggestions/derive-macro-missing-bounds.stderr +++ b/src/test/ui/suggestions/derive-macro-missing-bounds.stderr @@ -13,7 +13,7 @@ help: consider annotating `a::Inner` with `#[derive(Debug)]` | LL | #[derive(Debug)] | -help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement +help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement | LL | struct Outer(Inner) where a::Inner: Debug; | ++++++++++++++++++++++++ diff --git a/src/test/ui/suggestions/invalid-bin-op.stderr b/src/test/ui/suggestions/invalid-bin-op.stderr index fe5e2b5816fdf..94bd2d4156582 100644 --- a/src/test/ui/suggestions/invalid-bin-op.stderr +++ b/src/test/ui/suggestions/invalid-bin-op.stderr @@ -15,7 +15,7 @@ help: consider annotating `S` with `#[derive(PartialEq)]` | LL | #[derive(PartialEq)] | -help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement +help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement | LL | pub fn foo(s: S, t: S) where S: PartialEq { | +++++++++++++++++++++ diff --git a/src/test/ui/suggestions/suggest-change-mut.rs b/src/test/ui/suggestions/suggest-change-mut.rs index a2bc6fd09b566..47dc7c343daeb 100644 --- a/src/test/ui/suggestions/suggest-change-mut.rs +++ b/src/test/ui/suggestions/suggest-change-mut.rs @@ -2,7 +2,7 @@ use std::io::{BufRead, BufReader, Read, Write}; -fn issue_81421(mut stream: T) { //~ HELP consider introducing a `where` bound +fn issue_81421(mut stream: T) { //~ HELP consider introducing a `where` clause let initial_message = format!("Hello world"); let mut buffer: Vec = Vec::new(); let bytes_written = stream.write_all(initial_message.as_bytes()); diff --git a/src/test/ui/suggestions/suggest-change-mut.stderr b/src/test/ui/suggestions/suggest-change-mut.stderr index 2fa69cd5a2c8d..be549239e3685 100644 --- a/src/test/ui/suggestions/suggest-change-mut.stderr +++ b/src/test/ui/suggestions/suggest-change-mut.stderr @@ -16,7 +16,7 @@ help: consider removing the leading `&`-reference LL - let mut stream_reader = BufReader::new(&stream); LL + let mut stream_reader = BufReader::new(stream); | -help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement +help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement | LL | fn issue_81421(mut stream: T) where &T: std::io::Read { | +++++++++++++++++++++++ diff --git a/src/test/ui/traits/resolution-in-overloaded-op.stderr b/src/test/ui/traits/resolution-in-overloaded-op.stderr index 3ae6bf130cc7e..34fae64e4d20f 100644 --- a/src/test/ui/traits/resolution-in-overloaded-op.stderr +++ b/src/test/ui/traits/resolution-in-overloaded-op.stderr @@ -6,7 +6,7 @@ LL | a * b | | | &T | -help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement +help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement | LL | fn foo>(a: &T, b: f64) -> f64 where &T: Mul { | ++++++++++++++++++ diff --git a/src/test/ui/traits/suggest-where-clause.stderr b/src/test/ui/traits/suggest-where-clause.stderr index e2cdd368888a8..520ee0b5ea733 100644 --- a/src/test/ui/traits/suggest-where-clause.stderr +++ b/src/test/ui/traits/suggest-where-clause.stderr @@ -49,7 +49,7 @@ error[E0277]: the trait bound `u64: From` is not satisfied LL | >::from; | ^^^^^^^^^^^^^^^^^^^^^^ the trait `From` is not implemented for `u64` | -help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement +help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement | LL | fn check() where u64: From { | ++++++++++++++++++ @@ -60,7 +60,7 @@ error[E0277]: the trait bound `u64: From<::Item>` is not satisfie LL | ::Item>>::from; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `From<::Item>` is not implemented for `u64` | -help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement +help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement | LL | fn check() where u64: From<::Item> { | ++++++++++++++++++++++++++++++++++++++ diff --git a/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn.stderr b/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn.stderr index f4d8b4509d43d..198f3e26393d4 100644 --- a/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn.stderr +++ b/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn.stderr @@ -5,7 +5,7 @@ LL | (a, a) | ^ the trait `From<&A>` is not implemented for `&'static B` | = note: required because of the requirements on the impl of `Into<&'static B>` for `&A` -help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement +help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement | LL | fn f(a: &'static A, b: B) -> (X, X) where &'static B: From<&A> { | ++++++++++++++++++++++++++ From 119d314ef481f46f296c0506dc1381a1feb66de1 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 5 Jun 2022 18:45:45 -0700 Subject: [PATCH 3/5] Make is_suggestable work on all TypeFoldable --- .../src/infer/error_reporting/mod.rs | 6 +- compiler/rustc_middle/src/ty/diagnostics.rs | 61 ++++++++++++++----- .../src/traits/error_reporting/suggestions.rs | 40 ++++++------ compiler/rustc_typeck/src/astconv/generics.rs | 2 +- compiler/rustc_typeck/src/astconv/mod.rs | 4 +- .../src/check/fn_ctxt/suggestions.rs | 2 +- .../rustc_typeck/src/check/method/suggest.rs | 6 +- compiler/rustc_typeck/src/collect.rs | 2 +- 8 files changed, 73 insertions(+), 50 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index e461efc1cda79..4e5d477f291f7 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -2545,11 +2545,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { labeled_user_string ); let pred = format!("{}: {}", bound_kind, sub); - let suggestion = format!( - "{} {}", - generics.add_where_or_trailing_comma(), - pred, - ); + let suggestion = format!("{} {}", generics.add_where_or_trailing_comma(), pred,); err.span_suggestion( generics.tail_span_for_predicate_suggestion(), "consider adding a where clause", diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs index 6fbe6b8550ca2..0f4ccdff20d3d 100644 --- a/compiler/rustc_middle/src/ty/diagnostics.rs +++ b/compiler/rustc_middle/src/ty/diagnostics.rs @@ -3,8 +3,8 @@ use std::ops::ControlFlow; use crate::ty::{ - fold::TypeFoldable, Const, ConstKind, DefIdTree, ExistentialPredicate, InferTy, Ty, TyCtxt, - TypeVisitor, + fold::TypeFoldable, Const, ConstKind, DefIdTree, ExistentialPredicate, InferTy, + PolyTraitPredicate, Ty, TyCtxt, TypeVisitor, }; use rustc_data_structures::fx::FxHashMap; @@ -73,31 +73,53 @@ impl<'tcx> Ty<'tcx> { _ => self.is_simple_ty(), } } +} + +pub trait IsSuggestable<'tcx> { + fn is_suggestable(self, tcx: TyCtxt<'tcx>) -> bool; - /// Whether the type can be safely suggested during error recovery. - pub fn is_suggestable(self, tcx: TyCtxt<'tcx>) -> bool { - self.visit_with(&mut IsSuggestableVisitor { tcx }).is_continue() + fn is_suggestable_modulo_impl_trait(self, tcx: TyCtxt<'tcx>, bound_str: &str) -> bool; +} + +impl<'tcx, T> IsSuggestable<'tcx> for T +where + T: TypeFoldable<'tcx>, +{ + fn is_suggestable(self, tcx: TyCtxt<'tcx>) -> bool { + self.visit_with(&mut IsSuggestableVisitor { tcx, bound_str: None }).is_continue() + } + + fn is_suggestable_modulo_impl_trait(self, tcx: TyCtxt<'tcx>, bound_str: &str) -> bool { + self.visit_with(&mut IsSuggestableVisitor { tcx, bound_str: Some(bound_str) }).is_continue() } } -pub fn suggest_arbitrary_trait_bound( +pub fn suggest_arbitrary_trait_bound<'tcx>( + tcx: TyCtxt<'tcx>, generics: &hir::Generics<'_>, err: &mut Diagnostic, - param_name: &str, - constraint: &str, + trait_pred: PolyTraitPredicate<'tcx>, ) -> bool { + if !trait_pred.is_suggestable(tcx) { + return false; + } + + let param_name = trait_pred.skip_binder().self_ty().to_string(); + let constraint = trait_pred.print_modifiers_and_trait_path().to_string(); let param = generics.params.iter().find(|p| p.name.ident().as_str() == param_name); - match (param, param_name) { - (Some(_), "Self") => return false, - _ => {} + + // Skip, there is a param named Self + if param.is_some() && param_name == "Self" { + return false; } + // Suggest a where clause bound for a non-type parameter. err.span_suggestion_verbose( generics.tail_span_for_predicate_suggestion(), &format!( "consider {} `where` clause, but there might be an alternative better way to express \ this requirement", - if generics.has_where_clause_token { "extending the" } else { "introducing a" }, + if generics.has_where_clause_token { "extending the" } else { "introducing a" }, ), format!("{} {}: {}", generics.add_where_or_trailing_comma(), param_name, constraint), Applicability::MaybeIncorrect, @@ -395,11 +417,12 @@ impl<'v> hir::intravisit::Visitor<'v> for StaticLifetimeVisitor<'v> { } } -pub struct IsSuggestableVisitor<'tcx> { +pub struct IsSuggestableVisitor<'tcx, 's> { tcx: TyCtxt<'tcx>, + bound_str: Option<&'s str>, } -impl<'tcx> TypeVisitor<'tcx> for IsSuggestableVisitor<'tcx> { +impl<'tcx> TypeVisitor<'tcx> for IsSuggestableVisitor<'tcx, '_> { type BreakTy = (); fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow { @@ -438,6 +461,16 @@ impl<'tcx> TypeVisitor<'tcx> for IsSuggestableVisitor<'tcx> { } } + Param(param) => { + if let Some(found_bound_str) = + param.name.as_str().strip_prefix("impl ").map(|s| s.trim_start()) + { + if self.bound_str.map_or(true, |bound_str| bound_str != found_bound_str) { + return ControlFlow::Break(()); + } + } + } + _ => {} } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 50a3eb8c20dd7..a0a8946d1c49f 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -21,11 +21,9 @@ use rustc_hir::lang_items::LangItem; use rustc_hir::{AsyncGeneratorKind, GeneratorKind, Node}; use rustc_middle::hir::map; use rustc_middle::ty::{ - self, - subst::{GenericArgKind, SubstsRef}, - suggest_arbitrary_trait_bound, suggest_constraining_type_param, AdtKind, DefIdTree, - GeneratorDiagnosticData, GeneratorInteriorTypeCause, Infer, InferTy, ToPredicate, Ty, TyCtxt, - TypeFoldable, + self, suggest_arbitrary_trait_bound, suggest_constraining_type_param, AdtKind, DefIdTree, + GeneratorDiagnosticData, GeneratorInteriorTypeCause, Infer, InferTy, IsSuggestable, + ToPredicate, Ty, TyCtxt, TypeFoldable, }; use rustc_middle::ty::{TypeAndMut, TypeckResults}; use rustc_session::Limit; @@ -356,11 +354,14 @@ fn suggest_restriction<'tcx>( ty::Param(param) => { // `fn foo(t: impl Trait)` // ^^^^^ get this string - param.name.as_str().strip_prefix("impl").map(|s| (s.trim_start().to_string(), sig)) + param.name.as_str().strip_prefix("impl ").map(|s| (s.trim_start().to_string(), sig)) } _ => None, }) { + if !trait_pred.is_suggestable_modulo_impl_trait(tcx, &bound_str) { + return; + } // We know we have an `impl Trait` that doesn't satisfy a required projection. // Find all of the occurrences of `impl Trait` for `Trait` in the function arguments' @@ -415,6 +416,9 @@ fn suggest_restriction<'tcx>( Applicability::MaybeIncorrect, ); } else { + if !trait_pred.is_suggestable(tcx) { + return; + } // Trivial case: `T` needs an extra bound: `T: Bound`. let (sp, suggestion) = match ( generics @@ -461,16 +465,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { _ => (false, None), }; - let generic_args_have_impl_trait = |args: SubstsRef<'tcx>| -> bool { - args.iter().any(|arg| match arg.unpack() { - GenericArgKind::Type(ty) => match ty.kind() { - ty::Param(param) => param.name.as_str().starts_with("impl"), - _ => false, - }, - _ => false, - }) - }; - // FIXME: Add check for trait bound that is already present, particularly `?Sized` so we // don't suggest `T: Sized + ?Sized`. let mut hir_id = body_id; @@ -572,6 +566,12 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { | hir::Node::ImplItem(hir::ImplItem { generics, .. }) if param_ty => { + if !trait_pred.skip_binder().trait_ref.substs[1..] + .iter() + .all(|g| g.is_suggestable(self.tcx)) + { + return; + } // Missing generic type parameter bound. let param_name = self_ty.to_string(); let constraint = with_no_trimmed_paths!( @@ -601,13 +601,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { | hir::ItemKind::TraitAlias(generics, _) | hir::ItemKind::OpaqueTy(hir::OpaqueTy { generics, .. }), .. - }) if !param_ty - && !generic_args_have_impl_trait(trait_pred.skip_binder().trait_ref.substs) => - { + }) if !param_ty => { // Missing generic type parameter bound. - let param_name = self_ty.to_string(); - let constraint = trait_pred.print_modifiers_and_trait_path().to_string(); - if suggest_arbitrary_trait_bound(generics, &mut err, ¶m_name, &constraint) { + if suggest_arbitrary_trait_bound(self.tcx, generics, &mut err, trait_pred) { return; } } diff --git a/compiler/rustc_typeck/src/astconv/generics.rs b/compiler/rustc_typeck/src/astconv/generics.rs index dc4bc8fb55a17..e18b352969bbd 100644 --- a/compiler/rustc_typeck/src/astconv/generics.rs +++ b/compiler/rustc_typeck/src/astconv/generics.rs @@ -13,7 +13,7 @@ use rustc_hir::def_id::DefId; use rustc_hir::GenericArg; use rustc_infer::infer::TyCtxtInferExt; use rustc_middle::ty::{ - self, subst, subst::SubstsRef, GenericParamDef, GenericParamDefKind, Ty, TyCtxt, + self, subst, subst::SubstsRef, GenericParamDef, GenericParamDefKind, IsSuggestable, Ty, TyCtxt, }; use rustc_session::lint::builtin::LATE_BOUND_LIFETIME_ARGUMENTS; use rustc_span::{symbol::kw, Span}; diff --git a/compiler/rustc_typeck/src/astconv/mod.rs b/compiler/rustc_typeck/src/astconv/mod.rs index bcff2ae512909..41565d2c2fa1d 100644 --- a/compiler/rustc_typeck/src/astconv/mod.rs +++ b/compiler/rustc_typeck/src/astconv/mod.rs @@ -26,7 +26,9 @@ use rustc_hir::lang_items::LangItem; use rustc_hir::{GenericArg, GenericArgs}; use rustc_middle::ty::subst::{self, GenericArgKind, InternalSubsts, Subst, SubstsRef}; use rustc_middle::ty::GenericParamDefKind; -use rustc_middle::ty::{self, Const, DefIdTree, EarlyBinder, Ty, TyCtxt, TypeFoldable}; +use rustc_middle::ty::{ + self, Const, DefIdTree, EarlyBinder, IsSuggestable, Ty, TyCtxt, TypeFoldable, +}; use rustc_session::lint::builtin::{AMBIGUOUS_ASSOCIATED_ITEMS, BARE_TRAIT_OBJECTS}; use rustc_span::edition::Edition; use rustc_span::lev_distance::find_best_match_for_name; diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index 76add2fb9c285..de2fd5b2e3786 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -15,7 +15,7 @@ use rustc_infer::infer::{self, TyCtxtInferExt}; use rustc_infer::traits; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::subst::GenericArgKind; -use rustc_middle::ty::{self, Binder, ToPredicate, Ty}; +use rustc_middle::ty::{self, Binder, IsSuggestable, ToPredicate, Ty}; use rustc_span::symbol::{kw, sym}; use rustc_span::Span; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; diff --git a/compiler/rustc_typeck/src/check/method/suggest.rs b/compiler/rustc_typeck/src/check/method/suggest.rs index 0b0320ff4c53c..4c5577f5f76f1 100644 --- a/compiler/rustc_typeck/src/check/method/suggest.rs +++ b/compiler/rustc_typeck/src/check/method/suggest.rs @@ -821,11 +821,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { trait bound{s}", s = pluralize!(obligations.len()) ), - format!( - "{} {}", - add_where_or_comma, - obligations.join(", ") - ), + format!("{} {}", add_where_or_comma, obligations.join(", ")), Applicability::MaybeIncorrect, ); } diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index 838980e08aa04..1a373faeaa666 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -39,7 +39,7 @@ use rustc_middle::ty::query::Providers; use rustc_middle::ty::subst::InternalSubsts; use rustc_middle::ty::util::Discr; use rustc_middle::ty::util::IntTypeExt; -use rustc_middle::ty::{self, AdtKind, Const, DefIdTree, Ty, TyCtxt}; +use rustc_middle::ty::{self, AdtKind, Const, DefIdTree, IsSuggestable, Ty, TyCtxt}; use rustc_middle::ty::{ReprOptions, ToPredicate}; use rustc_session::lint; use rustc_session::parse::feature_err; From d69aed1c72b80e380f4537aa7e4f5385da5dc060 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 5 Jun 2022 18:56:26 -0700 Subject: [PATCH 4/5] Properly replace `impl Trait` in fn args, turn {integer} to i32 --- .../src/traits/error_reporting/suggestions.rs | 39 +++++++++++++------ src/test/ui/suggestions/issue-97677.rs | 6 +++ src/test/ui/suggestions/issue-97677.stderr | 16 ++++++++ src/test/ui/suggestions/issue-97760.rs | 9 +++++ src/test/ui/suggestions/issue-97760.stderr | 18 +++++++++ 5 files changed, 76 insertions(+), 12 deletions(-) create mode 100644 src/test/ui/suggestions/issue-97677.rs create mode 100644 src/test/ui/suggestions/issue-97677.stderr create mode 100644 src/test/ui/suggestions/issue-97760.rs create mode 100644 src/test/ui/suggestions/issue-97760.stderr diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index a0a8946d1c49f..6f671d86cdf69 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -370,21 +370,34 @@ fn suggest_restriction<'tcx>( // but instead we choose to suggest replacing all instances of `impl Trait` with `T` // where `T: Trait`. let mut ty_spans = vec![]; - let impl_trait_str = format!("impl {}", bound_str); for input in fn_sig.decl.inputs { - if let hir::TyKind::Path(hir::QPath::Resolved( - None, - hir::Path { segments: [segment], .. }, - )) = input.kind - { - if segment.ident.as_str() == impl_trait_str.as_str() { - // `fn foo(t: impl Trait)` - // ^^^^^^^^^^ get this to suggest `T` instead + struct ReplaceImplTraitVisitor<'a> { + ty_spans: &'a mut Vec, + bound_str: &'a str, + } + impl<'a, 'hir> hir::intravisit::Visitor<'hir> for ReplaceImplTraitVisitor<'a> { + fn visit_ty(&mut self, t: &'hir hir::Ty<'hir>) { + if let hir::TyKind::Path(hir::QPath::Resolved( + None, + hir::Path { segments: [segment], .. }, + )) = t.kind + { + if segment.ident.as_str().strip_prefix("impl ").map(|s| s.trim_start()) + == Some(self.bound_str) + { + // `fn foo(t: impl Trait)` + // ^^^^^^^^^^ get this to suggest `T` instead - // There might be more than one `impl Trait`. - ty_spans.push(input.span); + // There might be more than one `impl Trait`. + self.ty_spans.push(t.span); + return; + } + } + hir::intravisit::walk_ty(self, t); } } + ReplaceImplTraitVisitor { ty_spans: &mut ty_spans, bound_str: &bound_str } + .visit_ty(input); } let type_param_name = generics.params.next_type_param_name(Some(&bound_str)); @@ -394,7 +407,7 @@ fn suggest_restriction<'tcx>( // FIXME: modify the `trait_pred` instead of string shenanigans. // Turn `::Bar: Qux` into `::Bar: Qux`. let pred = trait_pred.to_predicate(tcx).to_string(); - let pred = pred.replace(&impl_trait_str, &type_param_name); + let pred = pred.replace(&format!("impl {}", bound_str), &type_param_name); let mut sugg = vec![ if let Some(span) = generics.span_for_param_suggestion() { (span, format!(", {}", type_param)) @@ -458,6 +471,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { trait_pred: ty::PolyTraitPredicate<'tcx>, body_id: hir::HirId, ) { + let trait_pred = self.resolve_numeric_literals_with_default(trait_pred); + let self_ty = trait_pred.skip_binder().self_ty(); let (param_ty, projection) = match self_ty.kind() { ty::Param(_) => (true, None), diff --git a/src/test/ui/suggestions/issue-97677.rs b/src/test/ui/suggestions/issue-97677.rs new file mode 100644 index 0000000000000..a4c3b1350b891 --- /dev/null +++ b/src/test/ui/suggestions/issue-97677.rs @@ -0,0 +1,6 @@ +fn add_ten(n: N) -> N { + n + 10 + //~^ ERROR cannot add `{integer}` to `N` +} + +fn main() {} diff --git a/src/test/ui/suggestions/issue-97677.stderr b/src/test/ui/suggestions/issue-97677.stderr new file mode 100644 index 0000000000000..ea563ea844de5 --- /dev/null +++ b/src/test/ui/suggestions/issue-97677.stderr @@ -0,0 +1,16 @@ +error[E0369]: cannot add `{integer}` to `N` + --> $DIR/issue-97677.rs:2:7 + | +LL | n + 10 + | - ^ -- {integer} + | | + | N + | +help: consider restricting type parameter `N` + | +LL | fn add_ten>(n: N) -> N { + | ++++++++++++++++++++ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0369`. diff --git a/src/test/ui/suggestions/issue-97760.rs b/src/test/ui/suggestions/issue-97760.rs new file mode 100644 index 0000000000000..cf9c3c58dca6f --- /dev/null +++ b/src/test/ui/suggestions/issue-97760.rs @@ -0,0 +1,9 @@ +pub fn print_values(values: &impl IntoIterator) +where { + for x in values.into_iter() { + println!("{x}"); + //~^ ERROR ::Item` doesn't implement `std::fmt::Display + } +} + +fn main() {} diff --git a/src/test/ui/suggestions/issue-97760.stderr b/src/test/ui/suggestions/issue-97760.stderr new file mode 100644 index 0000000000000..459556bddaee5 --- /dev/null +++ b/src/test/ui/suggestions/issue-97760.stderr @@ -0,0 +1,18 @@ +error[E0277]: `::Item` doesn't implement `std::fmt::Display` + --> $DIR/issue-97760.rs:4:20 + | +LL | println!("{x}"); + | ^ `::Item` cannot be formatted with the default formatter + | + = help: the trait `std::fmt::Display` is not implemented for `::Item` + = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead + = note: this error originates in the macro `$crate::format_args_nl` (in Nightly builds, run with -Z macro-backtrace for more info) +help: introduce a type parameter with a trait bound instead of using `impl Trait` + | +LL ~ pub fn print_values(values: &I) +LL ~ where ::Item: std::fmt::Display { + | + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. From 91b99887f5a4329953d0e99f8f79924650d534eb Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 6 Jun 2022 21:01:06 -0700 Subject: [PATCH 5/5] Address comments --- compiler/rustc_ast_lowering/src/item.rs | 2 - compiler/rustc_ast_lowering/src/lib.rs | 2 - compiler/rustc_hir/src/hir.rs | 13 +- compiler/rustc_lint/src/builtin.rs | 4 +- compiler/rustc_middle/src/ty/diagnostics.rs | 34 ++--- compiler/rustc_middle/src/ty/generics.rs | 7 + .../src/traits/error_reporting/suggestions.rs | 143 ++++++++++++------ compiler/rustc_typeck/src/lib.rs | 11 +- 8 files changed, 131 insertions(+), 85 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 84eab512f7fe1..1c3bfa5aa5ae3 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -1377,7 +1377,6 @@ impl<'hir> LoweringContext<'_, 'hir> { let mut params: SmallVec<[hir::GenericParam<'hir>; 4]> = self.lower_generic_params_mut(&generics.params).collect(); let has_where_clause_predicates = !generics.where_clause.predicates.is_empty(); - let has_where_clause_token = generics.where_clause.has_where_token; let where_clause_span = self.lower_span(generics.where_clause.span); let span = self.lower_span(generics.span); let res = f(self); @@ -1396,7 +1395,6 @@ impl<'hir> LoweringContext<'_, 'hir> { params: self.arena.alloc_from_iter(params), predicates: self.arena.alloc_from_iter(predicates), has_where_clause_predicates, - has_where_clause_token, where_clause_span, span, }); diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index c4ba9960a9b5b..cfaa2cf662dd6 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -1333,7 +1333,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { params: lifetime_defs, predicates: &[], has_where_clause_predicates: false, - has_where_clause_token: false, where_clause_span: lctx.lower_span(span), span: lctx.lower_span(span), }), @@ -1656,7 +1655,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { params: generic_params, predicates: &[], has_where_clause_predicates: false, - has_where_clause_token: false, where_clause_span: this.lower_span(span), span: this.lower_span(span), }), diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 06f7b0af09488..2e60aa8107f4c 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -536,7 +536,6 @@ pub struct Generics<'hir> { pub params: &'hir [GenericParam<'hir>], pub predicates: &'hir [WherePredicate<'hir>], pub has_where_clause_predicates: bool, - pub has_where_clause_token: bool, pub where_clause_span: Span, pub span: Span, } @@ -547,7 +546,6 @@ impl<'hir> Generics<'hir> { params: &[], predicates: &[], has_where_clause_predicates: false, - has_where_clause_token: false, where_clause_span: DUMMY_SP, span: DUMMY_SP, }; @@ -583,10 +581,6 @@ impl<'hir> Generics<'hir> { } } - pub fn where_clause_span(&self) -> Option { - if self.predicates.is_empty() { None } else { Some(self.where_clause_span) } - } - /// `Span` where further predicates would be suggested, accounting for trailing commas, like /// in `fn foo(t: T) where T: Foo,` so we don't suggest two trailing commas. pub fn tail_span_for_predicate_suggestion(&self) -> Span { @@ -607,10 +601,11 @@ impl<'hir> Generics<'hir> { pub fn add_where_or_trailing_comma(&self) -> &'static str { if self.has_where_clause_predicates { "," - } else if self.has_where_clause_token { - "" - } else { + } else if self.where_clause_span.is_empty() { " where" + } else { + // No where clause predicates, but we have `where` token + "" } } diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index f8154fe33375f..60c458dcc9bb2 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -2266,9 +2266,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements { // (including the `where`) if hir_generics.has_where_clause_predicates && dropped_predicate_count == num_predicates { - let where_span = hir_generics - .where_clause_span() - .expect("span of (nonempty) where clause should exist"); + let where_span = hir_generics.where_clause_span; // Extend the where clause back to the closing `>` of the // generics, except for tuple struct, which have the `where` // after the fields of the struct. diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs index 0f4ccdff20d3d..d831e2a78d609 100644 --- a/compiler/rustc_middle/src/ty/diagnostics.rs +++ b/compiler/rustc_middle/src/ty/diagnostics.rs @@ -76,9 +76,13 @@ impl<'tcx> Ty<'tcx> { } pub trait IsSuggestable<'tcx> { + /// Whether this makes sense to suggest in a diagnostic. + /// + /// We filter out certain types and constants since they don't provide + /// meaningful rendered suggestions when pretty-printed. We leave some + /// nonsense, such as region vars, since those render as `'_` and are + /// usually okay to reinterpret as elided lifetimes. fn is_suggestable(self, tcx: TyCtxt<'tcx>) -> bool; - - fn is_suggestable_modulo_impl_trait(self, tcx: TyCtxt<'tcx>, bound_str: &str) -> bool; } impl<'tcx, T> IsSuggestable<'tcx> for T @@ -86,11 +90,7 @@ where T: TypeFoldable<'tcx>, { fn is_suggestable(self, tcx: TyCtxt<'tcx>) -> bool { - self.visit_with(&mut IsSuggestableVisitor { tcx, bound_str: None }).is_continue() - } - - fn is_suggestable_modulo_impl_trait(self, tcx: TyCtxt<'tcx>, bound_str: &str) -> bool { - self.visit_with(&mut IsSuggestableVisitor { tcx, bound_str: Some(bound_str) }).is_continue() + self.visit_with(&mut IsSuggestableVisitor { tcx }).is_continue() } } @@ -119,7 +119,7 @@ pub fn suggest_arbitrary_trait_bound<'tcx>( &format!( "consider {} `where` clause, but there might be an alternative better way to express \ this requirement", - if generics.has_where_clause_token { "extending the" } else { "introducing a" }, + if generics.where_clause_span.is_empty() { "introducing a" } else { "extending the" }, ), format!("{} {}: {}", generics.add_where_or_trailing_comma(), param_name, constraint), Applicability::MaybeIncorrect, @@ -417,12 +417,11 @@ impl<'v> hir::intravisit::Visitor<'v> for StaticLifetimeVisitor<'v> { } } -pub struct IsSuggestableVisitor<'tcx, 's> { +pub struct IsSuggestableVisitor<'tcx> { tcx: TyCtxt<'tcx>, - bound_str: Option<&'s str>, } -impl<'tcx> TypeVisitor<'tcx> for IsSuggestableVisitor<'tcx, '_> { +impl<'tcx> TypeVisitor<'tcx> for IsSuggestableVisitor<'tcx> { type BreakTy = (); fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow { @@ -462,12 +461,13 @@ impl<'tcx> TypeVisitor<'tcx> for IsSuggestableVisitor<'tcx, '_> { } Param(param) => { - if let Some(found_bound_str) = - param.name.as_str().strip_prefix("impl ").map(|s| s.trim_start()) - { - if self.bound_str.map_or(true, |bound_str| bound_str != found_bound_str) { - return ControlFlow::Break(()); - } + // FIXME: It would be nice to make this not use string manipulation, + // but it's pretty hard to do this, since `ty::ParamTy` is missing + // sufficient info to determine if it is synthetic, and we don't + // always have a convenient way of getting `ty::Generics` at the call + // sites we invoke `IsSuggestable::is_suggestable`. + if param.name.as_str().starts_with("impl ") { + return ControlFlow::Break(()); } } diff --git a/compiler/rustc_middle/src/ty/generics.rs b/compiler/rustc_middle/src/ty/generics.rs index 1feabb2d6b122..7fea823bffcd7 100644 --- a/compiler/rustc_middle/src/ty/generics.rs +++ b/compiler/rustc_middle/src/ty/generics.rs @@ -39,6 +39,13 @@ impl GenericParamDefKind { GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => true, } } + + pub fn is_synthetic(&self) -> bool { + match self { + GenericParamDefKind::Type { synthetic, .. } => *synthetic, + _ => false, + } + } } #[derive(Clone, Debug, TyEncodable, TyDecodable, HashStable)] diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 6f671d86cdf69..b80b1e64c9bad 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -7,6 +7,7 @@ use crate::autoderef::Autoderef; use crate::infer::InferCtxt; use crate::traits::normalize_projection_type; +use hir::HirId; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_errors::{ @@ -23,7 +24,7 @@ use rustc_middle::hir::map; use rustc_middle::ty::{ self, suggest_arbitrary_trait_bound, suggest_constraining_type_param, AdtKind, DefIdTree, GeneratorDiagnosticData, GeneratorInteriorTypeCause, Infer, InferTy, IsSuggestable, - ToPredicate, Ty, TyCtxt, TypeFoldable, + ToPredicate, Ty, TyCtxt, TypeFoldable, TypeFolder, }; use rustc_middle::ty::{TypeAndMut, TypeckResults}; use rustc_session::Limit; @@ -329,7 +330,8 @@ fn predicate_constraint(generics: &hir::Generics<'_>, pred: String) -> (Span, St /// param for cleaner code. fn suggest_restriction<'tcx>( tcx: TyCtxt<'tcx>, - generics: &hir::Generics<'tcx>, + hir_id: HirId, + hir_generics: &hir::Generics<'tcx>, msg: &str, err: &mut Diagnostic, fn_sig: Option<&hir::FnSig<'_>>, @@ -342,24 +344,37 @@ fn suggest_restriction<'tcx>( // &Ident super_traits: Option<(&Ident, &hir::GenericBounds<'_>)>, ) { - if generics.where_clause_span.from_expansion() - || generics.where_clause_span.desugaring_kind().is_some() + if hir_generics.where_clause_span.from_expansion() + || hir_generics.where_clause_span.desugaring_kind().is_some() { return; } + let Some(item_id) = hir_id.as_owner() else { return; }; + let generics = tcx.generics_of(item_id); // Given `fn foo(t: impl Trait)` where `Trait` requires assoc type `A`... - if let Some((bound_str, fn_sig)) = + if let Some((param, bound_str, fn_sig)) = fn_sig.zip(projection).and_then(|(sig, p)| match p.self_ty().kind() { // Shenanigans to get the `Trait` from the `impl Trait`. ty::Param(param) => { - // `fn foo(t: impl Trait)` - // ^^^^^ get this string - param.name.as_str().strip_prefix("impl ").map(|s| (s.trim_start().to_string(), sig)) + let param_def = generics.type_param(param, tcx); + if param_def.kind.is_synthetic() { + let bound_str = + param_def.name.as_str().strip_prefix("impl ")?.trim_start().to_string(); + return Some((param_def, bound_str, sig)); + } + None } _ => None, }) { - if !trait_pred.is_suggestable_modulo_impl_trait(tcx, &bound_str) { + let type_param_name = hir_generics.params.next_type_param_name(Some(&bound_str)); + let trait_pred = trait_pred.fold_with(&mut ReplaceImplTraitFolder { + tcx, + param, + replace_ty: ty::ParamTy::new(generics.count() as u32, Symbol::intern(&type_param_name)) + .to_ty(tcx), + }); + if !trait_pred.is_suggestable(tcx) { return; } // We know we have an `impl Trait` that doesn't satisfy a required projection. @@ -371,52 +386,21 @@ fn suggest_restriction<'tcx>( // where `T: Trait`. let mut ty_spans = vec![]; for input in fn_sig.decl.inputs { - struct ReplaceImplTraitVisitor<'a> { - ty_spans: &'a mut Vec, - bound_str: &'a str, - } - impl<'a, 'hir> hir::intravisit::Visitor<'hir> for ReplaceImplTraitVisitor<'a> { - fn visit_ty(&mut self, t: &'hir hir::Ty<'hir>) { - if let hir::TyKind::Path(hir::QPath::Resolved( - None, - hir::Path { segments: [segment], .. }, - )) = t.kind - { - if segment.ident.as_str().strip_prefix("impl ").map(|s| s.trim_start()) - == Some(self.bound_str) - { - // `fn foo(t: impl Trait)` - // ^^^^^^^^^^ get this to suggest `T` instead - - // There might be more than one `impl Trait`. - self.ty_spans.push(t.span); - return; - } - } - hir::intravisit::walk_ty(self, t); - } - } - ReplaceImplTraitVisitor { ty_spans: &mut ty_spans, bound_str: &bound_str } + ReplaceImplTraitVisitor { ty_spans: &mut ty_spans, param_did: param.def_id } .visit_ty(input); } - - let type_param_name = generics.params.next_type_param_name(Some(&bound_str)); // The type param `T: Trait` we will suggest to introduce. let type_param = format!("{}: {}", type_param_name, bound_str); - // FIXME: modify the `trait_pred` instead of string shenanigans. - // Turn `::Bar: Qux` into `::Bar: Qux`. - let pred = trait_pred.to_predicate(tcx).to_string(); - let pred = pred.replace(&format!("impl {}", bound_str), &type_param_name); let mut sugg = vec![ - if let Some(span) = generics.span_for_param_suggestion() { + if let Some(span) = hir_generics.span_for_param_suggestion() { (span, format!(", {}", type_param)) } else { - (generics.span, format!("<{}>", type_param)) + (hir_generics.span, format!("<{}>", type_param)) }, // `fn foo(t: impl Trait)` // ^ suggest `where ::A: Bound` - predicate_constraint(generics, pred), + predicate_constraint(hir_generics, trait_pred.to_predicate(tcx).to_string()), ]; sugg.extend(ty_spans.into_iter().map(|s| (s, type_param_name.to_string()))); @@ -434,13 +418,15 @@ fn suggest_restriction<'tcx>( } // Trivial case: `T` needs an extra bound: `T: Bound`. let (sp, suggestion) = match ( - generics + hir_generics .params .iter() .find(|p| !matches!(p.kind, hir::GenericParamKind::Type { synthetic: true, .. })), super_traits, ) { - (_, None) => predicate_constraint(generics, trait_pred.to_predicate(tcx).to_string()), + (_, None) => { + predicate_constraint(hir_generics, trait_pred.to_predicate(tcx).to_string()) + } (None, Some((ident, []))) => ( ident.span.shrink_to_hi(), format!(": {}", trait_pred.print_modifiers_and_trait_path()), @@ -450,7 +436,7 @@ fn suggest_restriction<'tcx>( format!(" + {}", trait_pred.print_modifiers_and_trait_path()), ), (Some(_), Some((_, []))) => ( - generics.span.shrink_to_hi(), + hir_generics.span.shrink_to_hi(), format!(": {}", trait_pred.print_modifiers_and_trait_path()), ), }; @@ -494,6 +480,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { // Restricting `Self` for a single method. suggest_restriction( self.tcx, + hir_id, &generics, "`Self`", err, @@ -513,7 +500,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { assert!(param_ty); // Restricting `Self` for a single method. suggest_restriction( - self.tcx, &generics, "`Self`", err, None, projection, trait_pred, None, + self.tcx, hir_id, &generics, "`Self`", err, None, projection, trait_pred, + None, ); return; } @@ -534,6 +522,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { // Missing restriction on associated type of type parameter (unmet projection). suggest_restriction( self.tcx, + hir_id, &generics, "the associated type", err, @@ -553,6 +542,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { // Missing restriction on associated type of type parameter (unmet projection). suggest_restriction( self.tcx, + hir_id, &generics, "the associated type", err, @@ -581,6 +571,14 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { | hir::Node::ImplItem(hir::ImplItem { generics, .. }) if param_ty => { + // We skip the 0'th subst (self) because we do not want + // to consider the predicate as not suggestible if the + // self type is an arg position `impl Trait` -- instead, + // we handle that by adding ` + Bound` below. + // FIXME(compiler-errors): It would be nice to do the same + // this that we do in `suggest_restriction` and pull the + // `impl Trait` into a new generic if it shows up somewhere + // else in the predicate. if !trait_pred.skip_binder().trait_ref.substs[1..] .iter() .all(|g| g.is_suggestable(self.tcx)) @@ -3004,3 +3002,52 @@ fn suggest_trait_object_return_type_alternatives( ); } } + +/// Collect the spans that we see the generic param `param_did` +struct ReplaceImplTraitVisitor<'a> { + ty_spans: &'a mut Vec, + param_did: DefId, +} + +impl<'a, 'hir> hir::intravisit::Visitor<'hir> for ReplaceImplTraitVisitor<'a> { + fn visit_ty(&mut self, t: &'hir hir::Ty<'hir>) { + if let hir::TyKind::Path(hir::QPath::Resolved( + None, + hir::Path { res: hir::def::Res::Def(_, segment_did), .. }, + )) = t.kind + { + if self.param_did == *segment_did { + // `fn foo(t: impl Trait)` + // ^^^^^^^^^^ get this to suggest `T` instead + + // There might be more than one `impl Trait`. + self.ty_spans.push(t.span); + return; + } + } + + hir::intravisit::walk_ty(self, t); + } +} + +// Replace `param` with `replace_ty` +struct ReplaceImplTraitFolder<'tcx> { + tcx: TyCtxt<'tcx>, + param: &'tcx ty::GenericParamDef, + replace_ty: Ty<'tcx>, +} + +impl<'tcx> TypeFolder<'tcx> for ReplaceImplTraitFolder<'tcx> { + fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> { + if let ty::Param(ty::ParamTy { index, .. }) = t.kind() { + if self.param.index == *index { + return self.replace_ty; + } + } + t.super_fold_with(self) + } + + fn tcx(&self) -> TyCtxt<'tcx> { + self.tcx + } +} diff --git a/compiler/rustc_typeck/src/lib.rs b/compiler/rustc_typeck/src/lib.rs index 454c71d4971eb..ae02eb600be2e 100644 --- a/compiler/rustc_typeck/src/lib.rs +++ b/compiler/rustc_typeck/src/lib.rs @@ -212,7 +212,7 @@ fn check_main_fn_ty(tcx: TyCtxt<'_>, main_def_id: DefId) { let hir_id = tcx.hir().local_def_id_to_hir_id(def_id.expect_local()); match tcx.hir().find(hir_id) { Some(Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, ref generics, _), .. })) => { - generics.where_clause_span() + Some(generics.where_clause_span) } _ => { span_bug!(tcx.def_span(def_id), "main has a non-function type"); @@ -402,14 +402,17 @@ fn check_start_fn_ty(tcx: TyCtxt<'_>, start_def_id: DefId) { .emit(); error = true; } - if let Some(sp) = generics.where_clause_span() { + if generics.has_where_clause_predicates { struct_span_err!( tcx.sess, - sp, + generics.where_clause_span, E0647, "start function is not allowed to have a `where` clause" ) - .span_label(sp, "start function cannot have a `where` clause") + .span_label( + generics.where_clause_span, + "start function cannot have a `where` clause", + ) .emit(); error = true; }