From 00c9d3d2ee99d7350bd9625fc098a4297c9a3523 Mon Sep 17 00:00:00 2001 From: Michael Goulet <michael@errs.io> Date: Sun, 4 Sep 2022 21:11:50 +0000 Subject: [PATCH 1/5] Avoid source-map call in operator error --- compiler/rustc_typeck/src/check/op.rs | 34 ++++++++----------- .../ui/binop/binary-op-on-double-ref.stderr | 2 +- .../ui/typeck/assign-non-lval-derefmut.stderr | 4 +-- .../ui/typeck/assign-non-lval-mut-ref.stderr | 4 +-- 4 files changed, 20 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_typeck/src/check/op.rs b/compiler/rustc_typeck/src/check/op.rs index 0d9dbb5bc11c2..fa0fac93276da 100644 --- a/compiler/rustc_typeck/src/check/op.rs +++ b/compiler/rustc_typeck/src/check/op.rs @@ -313,7 +313,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // error types are considered "builtin" Err(_) if lhs_ty.references_error() || rhs_ty.references_error() => self.tcx.ty_error(), Err(errors) => { - let source_map = self.tcx.sess.source_map(); let (mut err, missing_trait, use_output) = match is_assign { IsAssign::Yes => { let mut err = struct_span_err!( @@ -448,24 +447,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) .is_ok() { - if let Ok(lstring) = source_map.span_to_snippet(lhs_expr.span) { - let msg = &format!( - "`{}{}` can be used on `{}`, you can dereference `{}`", - op.node.as_str(), - match is_assign { - IsAssign::Yes => "=", - IsAssign::No => "", - }, - lhs_deref_ty.peel_refs(), - lstring, - ); - err.span_suggestion_verbose( - lhs_expr.span.shrink_to_lo(), - msg, - "*", - rustc_errors::Applicability::MachineApplicable, - ); - } + let msg = &format!( + "`{}{}` can be used on `{}` if you dereference the left-hand side", + op.node.as_str(), + match is_assign { + IsAssign::Yes => "=", + IsAssign::No => "", + }, + lhs_deref_ty, + ); + err.span_suggestion_verbose( + lhs_expr.span.shrink_to_lo(), + msg, + "*", + rustc_errors::Applicability::MachineApplicable, + ); } }; diff --git a/src/test/ui/binop/binary-op-on-double-ref.stderr b/src/test/ui/binop/binary-op-on-double-ref.stderr index 1651f70d5cde7..34826d2f4bf7a 100644 --- a/src/test/ui/binop/binary-op-on-double-ref.stderr +++ b/src/test/ui/binop/binary-op-on-double-ref.stderr @@ -6,7 +6,7 @@ LL | x % 2 == 0 | | | &&{integer} | -help: `%` can be used on `{integer}`, you can dereference `x` +help: `%` can be used on `&{integer}` if you dereference the left-hand side | LL | *x % 2 == 0 | + diff --git a/src/test/ui/typeck/assign-non-lval-derefmut.stderr b/src/test/ui/typeck/assign-non-lval-derefmut.stderr index a6fcdfe21f481..e394cf8206ede 100644 --- a/src/test/ui/typeck/assign-non-lval-derefmut.stderr +++ b/src/test/ui/typeck/assign-non-lval-derefmut.stderr @@ -19,7 +19,7 @@ LL | x.lock().unwrap() += 1; | | | cannot use `+=` on type `MutexGuard<'_, usize>` | -help: `+=` can be used on `usize`, you can dereference `x.lock().unwrap()` +help: `+=` can be used on `usize` if you dereference the left-hand side | LL | *x.lock().unwrap() += 1; | + @@ -47,7 +47,7 @@ LL | y += 1; | | | cannot use `+=` on type `MutexGuard<'_, usize>` | -help: `+=` can be used on `usize`, you can dereference `y` +help: `+=` can be used on `usize` if you dereference the left-hand side | LL | *y += 1; | + diff --git a/src/test/ui/typeck/assign-non-lval-mut-ref.stderr b/src/test/ui/typeck/assign-non-lval-mut-ref.stderr index be2e9fe95e871..cbdc960baab8e 100644 --- a/src/test/ui/typeck/assign-non-lval-mut-ref.stderr +++ b/src/test/ui/typeck/assign-non-lval-mut-ref.stderr @@ -19,7 +19,7 @@ LL | x.last_mut().unwrap() += 1; | | | cannot use `+=` on type `&mut usize` | -help: `+=` can be used on `usize`, you can dereference `x.last_mut().unwrap()` +help: `+=` can be used on `usize` if you dereference the left-hand side | LL | *x.last_mut().unwrap() += 1; | + @@ -45,7 +45,7 @@ LL | y += 1; | | | cannot use `+=` on type `&mut usize` | -help: `+=` can be used on `usize`, you can dereference `y` +help: `+=` can be used on `usize` if you dereference the left-hand side | LL | *y += 1; | + From d0746e886383af7eb9d3ed68f53ff1c19914ec39 Mon Sep 17 00:00:00 2001 From: Michael Goulet <michael@errs.io> Date: Sun, 4 Sep 2022 21:35:13 +0000 Subject: [PATCH 2/5] Simplify printing operator lang item paths in error message --- compiler/rustc_typeck/src/check/op.rs | 259 +++++++++++--------------- 1 file changed, 109 insertions(+), 150 deletions(-) diff --git a/compiler/rustc_typeck/src/check/op.rs b/compiler/rustc_typeck/src/check/op.rs index fa0fac93276da..c269a9a0c46ce 100644 --- a/compiler/rustc_typeck/src/check/op.rs +++ b/compiler/rustc_typeck/src/check/op.rs @@ -11,6 +11,7 @@ use rustc_infer::traits::ObligationCauseCode; use rustc_middle::ty::adjustment::{ Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, }; +use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::{ self, Ty, TyCtxt, TypeFolder, TypeSuperFoldable, TypeSuperVisitable, TypeVisitable, TypeVisitor, }; @@ -313,7 +314,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // error types are considered "builtin" Err(_) if lhs_ty.references_error() || rhs_ty.references_error() => self.tcx.ty_error(), Err(errors) => { - let (mut err, missing_trait, use_output) = match is_assign { + let (_, item) = lang_item_for_op(self.tcx, Op::Binary(op, is_assign), op.span); + let missing_trait = + item.map(|def_id| with_no_trimmed_paths!(self.tcx.def_path_str(def_id))); + let (mut err, use_output) = match is_assign { IsAssign::Yes => { let mut err = struct_span_err!( self.tcx.sess, @@ -327,112 +331,60 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { lhs_expr.span, format!("cannot use `{}=` on type `{}`", op.node.as_str(), lhs_ty), ); - let missing_trait = match op.node { - hir::BinOpKind::Add => Some("std::ops::AddAssign"), - hir::BinOpKind::Sub => Some("std::ops::SubAssign"), - hir::BinOpKind::Mul => Some("std::ops::MulAssign"), - hir::BinOpKind::Div => Some("std::ops::DivAssign"), - hir::BinOpKind::Rem => Some("std::ops::RemAssign"), - hir::BinOpKind::BitAnd => Some("std::ops::BitAndAssign"), - hir::BinOpKind::BitXor => Some("std::ops::BitXorAssign"), - hir::BinOpKind::BitOr => Some("std::ops::BitOrAssign"), - hir::BinOpKind::Shl => Some("std::ops::ShlAssign"), - hir::BinOpKind::Shr => Some("std::ops::ShrAssign"), - _ => None, - }; self.note_unmet_impls_on_type(&mut err, errors); - (err, missing_trait, false) + (err, false) } IsAssign::No => { - let (message, missing_trait, use_output) = match op.node { - hir::BinOpKind::Add => ( - format!("cannot add `{rhs_ty}` to `{lhs_ty}`"), - Some("std::ops::Add"), - true, - ), - hir::BinOpKind::Sub => ( - format!("cannot subtract `{rhs_ty}` from `{lhs_ty}`"), - Some("std::ops::Sub"), - true, - ), - hir::BinOpKind::Mul => ( - format!("cannot multiply `{lhs_ty}` by `{rhs_ty}`"), - Some("std::ops::Mul"), - true, - ), - hir::BinOpKind::Div => ( - format!("cannot divide `{lhs_ty}` by `{rhs_ty}`"), - Some("std::ops::Div"), - true, - ), - hir::BinOpKind::Rem => ( - format!("cannot mod `{lhs_ty}` by `{rhs_ty}`"), - Some("std::ops::Rem"), - true, - ), - hir::BinOpKind::BitAnd => ( - format!("no implementation for `{lhs_ty} & {rhs_ty}`"), - Some("std::ops::BitAnd"), - true, - ), - hir::BinOpKind::BitXor => ( - format!("no implementation for `{lhs_ty} ^ {rhs_ty}`"), - Some("std::ops::BitXor"), - true, - ), - hir::BinOpKind::BitOr => ( - format!("no implementation for `{lhs_ty} | {rhs_ty}`"), - Some("std::ops::BitOr"), - true, - ), - hir::BinOpKind::Shl => ( - format!("no implementation for `{lhs_ty} << {rhs_ty}`"), - Some("std::ops::Shl"), - true, - ), - hir::BinOpKind::Shr => ( - format!("no implementation for `{lhs_ty} >> {rhs_ty}`"), - Some("std::ops::Shr"), - true, - ), - hir::BinOpKind::Eq | hir::BinOpKind::Ne => ( - format!( - "binary operation `{}` cannot be applied to type `{}`", - op.node.as_str(), - lhs_ty - ), - Some("std::cmp::PartialEq"), - false, - ), - hir::BinOpKind::Lt - | hir::BinOpKind::Le - | hir::BinOpKind::Gt - | hir::BinOpKind::Ge => ( - format!( - "binary operation `{}` cannot be applied to type `{}`", - op.node.as_str(), - lhs_ty - ), - Some("std::cmp::PartialOrd"), - false, - ), - _ => ( - format!( - "binary operation `{}` cannot be applied to type `{}`", - op.node.as_str(), - lhs_ty - ), - None, - false, + let message = match op.node { + hir::BinOpKind::Add => { + format!("cannot add `{rhs_ty}` to `{lhs_ty}`") + } + hir::BinOpKind::Sub => { + format!("cannot subtract `{rhs_ty}` from `{lhs_ty}`") + } + hir::BinOpKind::Mul => { + format!("cannot multiply `{lhs_ty}` by `{rhs_ty}`") + } + hir::BinOpKind::Div => { + format!("cannot divide `{lhs_ty}` by `{rhs_ty}`") + } + hir::BinOpKind::Rem => { + format!("cannot mod `{lhs_ty}` by `{rhs_ty}`") + } + hir::BinOpKind::BitAnd => { + format!("no implementation for `{lhs_ty} & {rhs_ty}`") + } + hir::BinOpKind::BitXor => { + format!("no implementation for `{lhs_ty} ^ {rhs_ty}`") + } + hir::BinOpKind::BitOr => { + format!("no implementation for `{lhs_ty} | {rhs_ty}`") + } + hir::BinOpKind::Shl => { + format!("no implementation for `{lhs_ty} << {rhs_ty}`") + } + hir::BinOpKind::Shr => { + format!("no implementation for `{lhs_ty} >> {rhs_ty}`") + } + _ => format!( + "binary operation `{}` cannot be applied to type `{}`", + op.node.as_str(), + lhs_ty ), }; + let use_output = item.map_or(false, |def_id| { + self.tcx.associated_item_def_ids(def_id).iter().any(|item_def_id| { + self.tcx.opt_associated_item(*item_def_id).unwrap().name + == sym::Output + }) + }); let mut err = struct_span_err!(self.tcx.sess, op.span, E0369, "{message}"); if !lhs_expr.span.eq(&rhs_expr.span) { err.span_label(lhs_expr.span, lhs_ty.to_string()); err.span_label(rhs_expr.span, rhs_ty.to_string()); } self.note_unmet_impls_on_type(&mut err, errors); - (err, missing_trait, use_output) + (err, use_output) } }; @@ -773,64 +725,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { op: Op, expected: Expectation<'tcx>, ) -> Result<MethodCallee<'tcx>, Vec<FulfillmentError<'tcx>>> { - let lang = self.tcx.lang_items(); - let span = match op { Op::Binary(op, _) => op.span, Op::Unary(_, span) => span, }; - let (opname, trait_did) = if let Op::Binary(op, IsAssign::Yes) = op { - match op.node { - hir::BinOpKind::Add => (sym::add_assign, lang.add_assign_trait()), - hir::BinOpKind::Sub => (sym::sub_assign, lang.sub_assign_trait()), - hir::BinOpKind::Mul => (sym::mul_assign, lang.mul_assign_trait()), - hir::BinOpKind::Div => (sym::div_assign, lang.div_assign_trait()), - hir::BinOpKind::Rem => (sym::rem_assign, lang.rem_assign_trait()), - hir::BinOpKind::BitXor => (sym::bitxor_assign, lang.bitxor_assign_trait()), - hir::BinOpKind::BitAnd => (sym::bitand_assign, lang.bitand_assign_trait()), - hir::BinOpKind::BitOr => (sym::bitor_assign, lang.bitor_assign_trait()), - hir::BinOpKind::Shl => (sym::shl_assign, lang.shl_assign_trait()), - hir::BinOpKind::Shr => (sym::shr_assign, lang.shr_assign_trait()), - hir::BinOpKind::Lt - | hir::BinOpKind::Le - | hir::BinOpKind::Ge - | hir::BinOpKind::Gt - | hir::BinOpKind::Eq - | hir::BinOpKind::Ne - | hir::BinOpKind::And - | hir::BinOpKind::Or => { - span_bug!(span, "impossible assignment operation: {}=", op.node.as_str()) - } - } - } else if let Op::Binary(op, IsAssign::No) = op { - match op.node { - hir::BinOpKind::Add => (sym::add, lang.add_trait()), - hir::BinOpKind::Sub => (sym::sub, lang.sub_trait()), - hir::BinOpKind::Mul => (sym::mul, lang.mul_trait()), - hir::BinOpKind::Div => (sym::div, lang.div_trait()), - hir::BinOpKind::Rem => (sym::rem, lang.rem_trait()), - hir::BinOpKind::BitXor => (sym::bitxor, lang.bitxor_trait()), - hir::BinOpKind::BitAnd => (sym::bitand, lang.bitand_trait()), - hir::BinOpKind::BitOr => (sym::bitor, lang.bitor_trait()), - hir::BinOpKind::Shl => (sym::shl, lang.shl_trait()), - hir::BinOpKind::Shr => (sym::shr, lang.shr_trait()), - hir::BinOpKind::Lt => (sym::lt, lang.partial_ord_trait()), - hir::BinOpKind::Le => (sym::le, lang.partial_ord_trait()), - hir::BinOpKind::Ge => (sym::ge, lang.partial_ord_trait()), - hir::BinOpKind::Gt => (sym::gt, lang.partial_ord_trait()), - hir::BinOpKind::Eq => (sym::eq, lang.eq_trait()), - hir::BinOpKind::Ne => (sym::ne, lang.eq_trait()), - hir::BinOpKind::And | hir::BinOpKind::Or => { - span_bug!(span, "&& and || are not overloadable") - } - } - } else if let Op::Unary(hir::UnOp::Not, _) = op { - (sym::not, lang.not_trait()) - } else if let Op::Unary(hir::UnOp::Neg, _) = op { - (sym::neg, lang.neg_trait()) - } else { - bug!("lookup_op_method: op not supported: {:?}", op) - }; + let (opname, trait_did) = lang_item_for_op(self.tcx, op, span); debug!( "lookup_op_method(lhs_ty={:?}, op={:?}, opname={:?}, trait_did={:?})", @@ -891,6 +790,66 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } +fn lang_item_for_op( + tcx: TyCtxt<'_>, + op: Op, + span: Span, +) -> (rustc_span::Symbol, Option<hir::def_id::DefId>) { + let lang = tcx.lang_items(); + if let Op::Binary(op, IsAssign::Yes) = op { + match op.node { + hir::BinOpKind::Add => (sym::add_assign, lang.add_assign_trait()), + hir::BinOpKind::Sub => (sym::sub_assign, lang.sub_assign_trait()), + hir::BinOpKind::Mul => (sym::mul_assign, lang.mul_assign_trait()), + hir::BinOpKind::Div => (sym::div_assign, lang.div_assign_trait()), + hir::BinOpKind::Rem => (sym::rem_assign, lang.rem_assign_trait()), + hir::BinOpKind::BitXor => (sym::bitxor_assign, lang.bitxor_assign_trait()), + hir::BinOpKind::BitAnd => (sym::bitand_assign, lang.bitand_assign_trait()), + hir::BinOpKind::BitOr => (sym::bitor_assign, lang.bitor_assign_trait()), + hir::BinOpKind::Shl => (sym::shl_assign, lang.shl_assign_trait()), + hir::BinOpKind::Shr => (sym::shr_assign, lang.shr_assign_trait()), + hir::BinOpKind::Lt + | hir::BinOpKind::Le + | hir::BinOpKind::Ge + | hir::BinOpKind::Gt + | hir::BinOpKind::Eq + | hir::BinOpKind::Ne + | hir::BinOpKind::And + | hir::BinOpKind::Or => { + span_bug!(span, "impossible assignment operation: {}=", op.node.as_str()) + } + } + } else if let Op::Binary(op, IsAssign::No) = op { + match op.node { + hir::BinOpKind::Add => (sym::add, lang.add_trait()), + hir::BinOpKind::Sub => (sym::sub, lang.sub_trait()), + hir::BinOpKind::Mul => (sym::mul, lang.mul_trait()), + hir::BinOpKind::Div => (sym::div, lang.div_trait()), + hir::BinOpKind::Rem => (sym::rem, lang.rem_trait()), + hir::BinOpKind::BitXor => (sym::bitxor, lang.bitxor_trait()), + hir::BinOpKind::BitAnd => (sym::bitand, lang.bitand_trait()), + hir::BinOpKind::BitOr => (sym::bitor, lang.bitor_trait()), + hir::BinOpKind::Shl => (sym::shl, lang.shl_trait()), + hir::BinOpKind::Shr => (sym::shr, lang.shr_trait()), + hir::BinOpKind::Lt => (sym::lt, lang.partial_ord_trait()), + hir::BinOpKind::Le => (sym::le, lang.partial_ord_trait()), + hir::BinOpKind::Ge => (sym::ge, lang.partial_ord_trait()), + hir::BinOpKind::Gt => (sym::gt, lang.partial_ord_trait()), + hir::BinOpKind::Eq => (sym::eq, lang.eq_trait()), + hir::BinOpKind::Ne => (sym::ne, lang.eq_trait()), + hir::BinOpKind::And | hir::BinOpKind::Or => { + span_bug!(span, "&& and || are not overloadable") + } + } + } else if let Op::Unary(hir::UnOp::Not, _) = op { + (sym::not, lang.not_trait()) + } else if let Op::Unary(hir::UnOp::Neg, _) = op { + (sym::neg, lang.neg_trait()) + } else { + bug!("lookup_op_method: op not supported: {:?}", op) + } +} + // Binary operator categories. These categories summarize the behavior // with respect to the builtin operations supported. enum BinOpCategory { From 0dbbf0f49398d6c74fd3337dd171fac6c7aa3d12 Mon Sep 17 00:00:00 2001 From: Michael Goulet <michael@errs.io> Date: Sun, 4 Sep 2022 21:55:38 +0000 Subject: [PATCH 3/5] Remove TypeParamVisitor --- compiler/rustc_typeck/src/check/op.rs | 36 ++++++--------------------- 1 file changed, 7 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_typeck/src/check/op.rs b/compiler/rustc_typeck/src/check/op.rs index c269a9a0c46ce..a7e080c13c7d5 100644 --- a/compiler/rustc_typeck/src/check/op.rs +++ b/compiler/rustc_typeck/src/check/op.rs @@ -12,9 +12,7 @@ use rustc_middle::ty::adjustment::{ Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, }; use rustc_middle::ty::print::with_no_trimmed_paths; -use rustc_middle::ty::{ - self, Ty, TyCtxt, TypeFolder, TypeSuperFoldable, TypeSuperVisitable, TypeVisitable, TypeVisitor, -}; +use rustc_middle::ty::{self, Ty, TyCtxt, TypeFolder, TypeSuperFoldable, TypeVisitable}; use rustc_span::source_map::Spanned; use rustc_span::symbol::{sym, Ident}; use rustc_span::Span; @@ -23,8 +21,6 @@ use rustc_trait_selection::traits::error_reporting::suggestions::InferCtxtExt as use rustc_trait_selection::traits::{FulfillmentError, TraitEngine, TraitEngineExt}; use rustc_type_ir::sty::TyKind::*; -use std::ops::ControlFlow; - impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// Checks a `a <op>= b` pub fn check_binop_assign( @@ -462,9 +458,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } if let Some(missing_trait) = missing_trait { - let mut visitor = TypeParamVisitor(vec![]); - visitor.visit_ty(lhs_ty); - if op.node == hir::BinOpKind::Add && self.check_str_addition( lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, is_assign, op, @@ -473,7 +466,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // This has nothing here because it means we did string // concatenation (e.g., "Hello " + "World!"). This means // we don't want the note in the else clause to be emitted - } else if let [ty] = &visitor.0[..] { + } else if lhs_ty.has_param_types_or_consts() { // Look for a TraitPredicate in the Fulfillment errors, // and use it to generate a suggestion. // @@ -513,7 +506,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } } - } else if *ty != lhs_ty { + } else { // When we know that a missing bound is responsible, we don't show // this note as it is redundant. err.note(&format!( @@ -650,14 +643,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { format!("cannot apply unary operator `{}`", op.as_str()), ); - let mut visitor = TypeParamVisitor(vec![]); - visitor.visit_ty(operand_ty); - if let [_] = &visitor.0[..] && let ty::Param(_) = *operand_ty.kind() { - let predicates = errors - .iter() - .filter_map(|error| { - error.obligation.predicate.to_opt_poly_trait_pred() - }); + if operand_ty.has_param_types_or_consts() { + let predicates = errors.iter().filter_map(|error| { + error.obligation.predicate.to_opt_poly_trait_pred() + }); for pred in predicates { self.suggest_restricting_param_bound( &mut err, @@ -972,17 +961,6 @@ fn is_builtin_binop<'tcx>(lhs: Ty<'tcx>, rhs: Ty<'tcx>, op: hir::BinOp) -> bool } } -struct TypeParamVisitor<'tcx>(Vec<Ty<'tcx>>); - -impl<'tcx> TypeVisitor<'tcx> for TypeParamVisitor<'tcx> { - fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> { - if let ty::Param(_) = ty.kind() { - self.0.push(ty); - } - ty.super_visit_with(self) - } -} - struct TypeParamEraser<'a, 'tcx>(&'a FnCtxt<'a, 'tcx>, Span); impl<'tcx> TypeFolder<'tcx> for TypeParamEraser<'_, 'tcx> { From 30e3673d437d7ca049d6080eee19e696c3d7429f Mon Sep 17 00:00:00 2001 From: Michael Goulet <michael@errs.io> Date: Sun, 4 Sep 2022 22:21:15 +0000 Subject: [PATCH 4/5] Add associated item binding to non-param-ty where clause suggestions --- compiler/rustc_middle/src/traits/mod.rs | 4 +- compiler/rustc_middle/src/ty/diagnostics.rs | 14 +++++- .../src/traits/error_reporting/suggestions.rs | 24 +++++----- .../rustc_typeck/src/check/fn_ctxt/_impl.rs | 2 +- compiler/rustc_typeck/src/check/method/mod.rs | 26 ++-------- compiler/rustc_typeck/src/check/op.rs | 47 ++++++++++++------- .../traits/resolution-in-overloaded-op.stderr | 4 +- 7 files changed, 65 insertions(+), 56 deletions(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index ab7e5ba3a1067..a56fac7c4dd2c 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -12,7 +12,7 @@ pub mod util; use crate::infer::canonical::Canonical; use crate::ty::abstract_const::NotConstEvaluatable; use crate::ty::subst::SubstsRef; -use crate::ty::{self, AdtKind, Predicate, Ty, TyCtxt}; +use crate::ty::{self, AdtKind, Ty, TyCtxt}; use rustc_data_structures::sync::Lrc; use rustc_errors::{Applicability, Diagnostic}; @@ -416,7 +416,7 @@ pub enum ObligationCauseCode<'tcx> { BinOp { rhs_span: Option<Span>, is_lit: bool, - output_pred: Option<Predicate<'tcx>>, + output_ty: Option<Ty<'tcx>>, }, } diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs index dd2f43210603a..fc6a77d400d47 100644 --- a/compiler/rustc_middle/src/ty/diagnostics.rs +++ b/compiler/rustc_middle/src/ty/diagnostics.rs @@ -102,13 +102,25 @@ pub fn suggest_arbitrary_trait_bound<'tcx>( generics: &hir::Generics<'_>, err: &mut Diagnostic, trait_pred: PolyTraitPredicate<'tcx>, + associated_ty: Option<(&'static str, Ty<'tcx>)>, ) -> bool { if !trait_pred.is_suggestable(tcx, false) { 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 mut constraint = trait_pred.print_modifiers_and_trait_path().to_string(); + + if let Some((name, term)) = associated_ty { + // FIXME: this case overlaps with code in TyCtxt::note_and_explain_type_err. + // That should be extracted into a helper function. + if constraint.ends_with('>') { + constraint = format!("{}, {}={}>", &constraint[..constraint.len() - 1], name, term); + } else { + constraint.push_str(&format!("<{}={}>", name, term)); + } + } + let param = generics.params.iter().find(|p| p.name.ident().as_str() == param_name); // Skip, there is a param named Self 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 b012073f7719d..595c68166bc8f 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -25,8 +25,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, - ProjectionPredicate, ToPredicate, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable, - TypeVisitable, + ToPredicate, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitable, }; use rustc_middle::ty::{TypeAndMut, TypeckResults}; use rustc_session::Limit; @@ -174,7 +173,7 @@ pub trait InferCtxtExt<'tcx> { &self, err: &mut Diagnostic, trait_pred: ty::PolyTraitPredicate<'tcx>, - proj_pred: Option<ty::PolyProjectionPredicate<'tcx>>, + associated_item: Option<(&'static str, Ty<'tcx>)>, body_id: hir::HirId, ); @@ -467,7 +466,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { &self, mut err: &mut Diagnostic, trait_pred: ty::PolyTraitPredicate<'tcx>, - proj_pred: Option<ty::PolyProjectionPredicate<'tcx>>, + associated_ty: Option<(&'static str, Ty<'tcx>)>, body_id: hir::HirId, ) { let trait_pred = self.resolve_numeric_literals_with_default(trait_pred); @@ -604,21 +603,18 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { trait_pred.print_modifiers_and_trait_path().to_string() ); - if let Some(proj_pred) = proj_pred { - let ProjectionPredicate { projection_ty, term } = proj_pred.skip_binder(); - let item = self.tcx.associated_item(projection_ty.item_def_id); - + if let Some((name, term)) = associated_ty { // FIXME: this case overlaps with code in TyCtxt::note_and_explain_type_err. // That should be extracted into a helper function. if constraint.ends_with('>') { constraint = format!( "{}, {}={}>", &constraint[..constraint.len() - 1], - item.name, + name, term ); } else { - constraint.push_str(&format!("<{}={}>", item.name, term)); + constraint.push_str(&format!("<{}={}>", name, term)); } } @@ -648,7 +644,13 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { .. }) if !param_ty => { // Missing generic type parameter bound. - if suggest_arbitrary_trait_bound(self.tcx, generics, &mut err, trait_pred) { + if suggest_arbitrary_trait_bound( + self.tcx, + generics, + &mut err, + trait_pred, + associated_ty, + ) { return; } } diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs index c59638f5d6f9f..2196a799fd0b6 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs @@ -409,7 +409,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { rhs_span: opt_input_expr.map(|expr| expr.span), is_lit: opt_input_expr .map_or(false, |expr| matches!(expr.kind, ExprKind::Lit(_))), - output_pred: None, + output_ty: None, }, ), self.param_env, diff --git a/compiler/rustc_typeck/src/check/method/mod.rs b/compiler/rustc_typeck/src/check/method/mod.rs index c597efbe7468e..249e9c66ba72a 100644 --- a/compiler/rustc_typeck/src/check/method/mod.rs +++ b/compiler/rustc_typeck/src/check/method/mod.rs @@ -20,10 +20,7 @@ use rustc_hir::def_id::DefId; use rustc_infer::infer::{self, InferOk}; use rustc_middle::ty::subst::Subst; use rustc_middle::ty::subst::{InternalSubsts, SubstsRef}; -use rustc_middle::ty::{ - self, AssocKind, DefIdTree, GenericParamDefKind, ProjectionPredicate, ProjectionTy, - ToPredicate, Ty, TypeVisitable, -}; +use rustc_middle::ty::{self, DefIdTree, GenericParamDefKind, ToPredicate, Ty, TypeVisitable}; use rustc_span::symbol::Ident; use rustc_span::Span; use rustc_trait_selection::traits; @@ -337,22 +334,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Construct an obligation let poly_trait_ref = ty::Binder::dummy(trait_ref); - let opt_output_ty = - expected.only_has_type(self).and_then(|ty| (!ty.needs_infer()).then(|| ty)); - let opt_output_assoc_item = self.tcx.associated_items(trait_def_id).find_by_name_and_kind( - self.tcx, - Ident::from_str("Output"), - AssocKind::Type, - trait_def_id, - ); - let output_pred = - opt_output_ty.zip(opt_output_assoc_item).map(|(output_ty, output_assoc_item)| { - ty::Binder::dummy(ty::PredicateKind::Projection(ProjectionPredicate { - projection_ty: ProjectionTy { substs, item_def_id: output_assoc_item.def_id }, - term: output_ty.into(), - })) - .to_predicate(self.tcx) - }); + let output_ty = expected.only_has_type(self).and_then(|ty| (!ty.needs_infer()).then(|| ty)); ( traits::Obligation::new( @@ -363,7 +345,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { rhs_span: opt_input_expr.map(|expr| expr.span), is_lit: opt_input_expr .map_or(false, |expr| matches!(expr.kind, hir::ExprKind::Lit(_))), - output_pred, + output_ty, }, ), self.param_env, @@ -518,7 +500,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { rhs_span: opt_input_expr.map(|expr| expr.span), is_lit: opt_input_expr .map_or(false, |expr| matches!(expr.kind, hir::ExprKind::Lit(_))), - output_pred: None, + output_ty: None, }, ) } else { diff --git a/compiler/rustc_typeck/src/check/op.rs b/compiler/rustc_typeck/src/check/op.rs index a7e080c13c7d5..4754717c29aba 100644 --- a/compiler/rustc_typeck/src/check/op.rs +++ b/compiler/rustc_typeck/src/check/op.rs @@ -12,7 +12,7 @@ use rustc_middle::ty::adjustment::{ Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, }; use rustc_middle::ty::print::with_no_trimmed_paths; -use rustc_middle::ty::{self, Ty, TyCtxt, TypeFolder, TypeSuperFoldable, TypeVisitable}; +use rustc_middle::ty::{self, DefIdTree, Ty, TyCtxt, TypeFolder, TypeSuperFoldable, TypeVisitable}; use rustc_span::source_map::Spanned; use rustc_span::symbol::{sym, Ident}; use rustc_span::Span; @@ -310,10 +310,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // error types are considered "builtin" Err(_) if lhs_ty.references_error() || rhs_ty.references_error() => self.tcx.ty_error(), Err(errors) => { - let (_, item) = lang_item_for_op(self.tcx, Op::Binary(op, is_assign), op.span); - let missing_trait = - item.map(|def_id| with_no_trimmed_paths!(self.tcx.def_path_str(def_id))); - let (mut err, use_output) = match is_assign { + let (_, trait_def_id) = + lang_item_for_op(self.tcx, Op::Binary(op, is_assign), op.span); + let missing_trait = trait_def_id + .map(|def_id| with_no_trimmed_paths!(self.tcx.def_path_str(def_id))); + let (mut err, output_def_id) = match is_assign { IsAssign::Yes => { let mut err = struct_span_err!( self.tcx.sess, @@ -328,7 +329,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { format!("cannot use `{}=` on type `{}`", op.node.as_str(), lhs_ty), ); self.note_unmet_impls_on_type(&mut err, errors); - (err, false) + (err, None) } IsAssign::No => { let message = match op.node { @@ -368,11 +369,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { lhs_ty ), }; - let use_output = item.map_or(false, |def_id| { - self.tcx.associated_item_def_ids(def_id).iter().any(|item_def_id| { - self.tcx.opt_associated_item(*item_def_id).unwrap().name - == sym::Output - }) + let output_def_id = trait_def_id.and_then(|def_id| { + self.tcx + .associated_item_def_ids(def_id) + .iter() + .find(|item_def_id| { + self.tcx.associated_item(*item_def_id).name == sym::Output + }) + .cloned() }); let mut err = struct_span_err!(self.tcx.sess, op.span, E0369, "{message}"); if !lhs_expr.span.eq(&rhs_expr.span) { @@ -380,7 +384,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err.span_label(rhs_expr.span, rhs_ty.to_string()); } self.note_unmet_impls_on_type(&mut err, errors); - (err, use_output) + (err, output_def_id) } }; @@ -488,12 +492,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if let Some(trait_pred) = error.obligation.predicate.to_opt_poly_trait_pred() { - let proj_pred = match error.obligation.cause.code() { + let output_associated_item = match error.obligation.cause.code() + { ObligationCauseCode::BinOp { - output_pred: Some(output_pred), + output_ty: Some(output_ty), .. - } if use_output => { - output_pred.to_opt_poly_projection_pred() + } => { + // Make sure that we're attaching `Output = ..` to the right trait predicate + if let Some(output_def_id) = output_def_id + && let Some(trait_def_id) = trait_def_id + && self.tcx.parent(output_def_id) == trait_def_id + { + Some(("Output", *output_ty)) + } else { + None + } } _ => None, }; @@ -501,7 +514,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.suggest_restricting_param_bound( &mut err, trait_pred, - proj_pred, + output_associated_item, self.body_id, ); } diff --git a/src/test/ui/traits/resolution-in-overloaded-op.stderr b/src/test/ui/traits/resolution-in-overloaded-op.stderr index 34fae64e4d20f..b67e334d40ac2 100644 --- a/src/test/ui/traits/resolution-in-overloaded-op.stderr +++ b/src/test/ui/traits/resolution-in-overloaded-op.stderr @@ -8,8 +8,8 @@ LL | a * b | help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement | -LL | fn foo<T: MyMul<f64, f64>>(a: &T, b: f64) -> f64 where &T: Mul<f64> { - | ++++++++++++++++++ +LL | fn foo<T: MyMul<f64, f64>>(a: &T, b: f64) -> f64 where &T: Mul<f64, Output=f64> { + | ++++++++++++++++++++++++++++++ error: aborting due to previous error From 48281b003ff84b230947f64aad9054113f757795 Mon Sep 17 00:00:00 2001 From: Michael Goulet <michael@errs.io> Date: Sun, 4 Sep 2022 22:38:12 +0000 Subject: [PATCH 5/5] Adjust spacing in suggestion, add a test --- compiler/rustc_middle/src/ty/diagnostics.rs | 4 +-- .../src/traits/error_reporting/suggestions.rs | 4 +-- .../missing-bounds.fixed | 2 +- .../missing-bounds.stderr | 4 +-- src/test/ui/suggestions/issue-97677.fixed | 2 +- src/test/ui/suggestions/issue-97677.stderr | 4 +-- .../ui/suggestions/restrict-type-not-param.rs | 12 +++++++++ .../restrict-type-not-param.stderr | 26 +++++++++++++++++++ .../traits/resolution-in-overloaded-op.stderr | 4 +-- 9 files changed, 50 insertions(+), 12 deletions(-) create mode 100644 src/test/ui/suggestions/restrict-type-not-param.rs create mode 100644 src/test/ui/suggestions/restrict-type-not-param.stderr diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs index fc6a77d400d47..e4ad96b659b08 100644 --- a/compiler/rustc_middle/src/ty/diagnostics.rs +++ b/compiler/rustc_middle/src/ty/diagnostics.rs @@ -115,9 +115,9 @@ pub fn suggest_arbitrary_trait_bound<'tcx>( // FIXME: this case overlaps with code in TyCtxt::note_and_explain_type_err. // That should be extracted into a helper function. if constraint.ends_with('>') { - constraint = format!("{}, {}={}>", &constraint[..constraint.len() - 1], name, term); + constraint = format!("{}, {} = {}>", &constraint[..constraint.len() - 1], name, term); } else { - constraint.push_str(&format!("<{}={}>", name, term)); + constraint.push_str(&format!("<{} = {}>", name, term)); } } 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 595c68166bc8f..ecbeb9d79b118 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -608,13 +608,13 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { // That should be extracted into a helper function. if constraint.ends_with('>') { constraint = format!( - "{}, {}={}>", + "{}, {} = {}>", &constraint[..constraint.len() - 1], name, term ); } else { - constraint.push_str(&format!("<{}={}>", name, term)); + constraint.push_str(&format!("<{} = {}>", name, term)); } } diff --git a/src/test/ui/generic-associated-types/missing-bounds.fixed b/src/test/ui/generic-associated-types/missing-bounds.fixed index 2315810a47ace..ee758f19ec105 100644 --- a/src/test/ui/generic-associated-types/missing-bounds.fixed +++ b/src/test/ui/generic-associated-types/missing-bounds.fixed @@ -24,7 +24,7 @@ impl<B: Add + Add<Output = B>> Add for C<B> { struct D<B>(B); -impl<B: std::ops::Add<Output=B>> Add for D<B> { +impl<B: std::ops::Add<Output = B>> Add for D<B> { type Output = Self; fn add(self, rhs: Self) -> Self { diff --git a/src/test/ui/generic-associated-types/missing-bounds.stderr b/src/test/ui/generic-associated-types/missing-bounds.stderr index 138c642dd7952..c913483a8747c 100644 --- a/src/test/ui/generic-associated-types/missing-bounds.stderr +++ b/src/test/ui/generic-associated-types/missing-bounds.stderr @@ -66,8 +66,8 @@ LL | Self(self.0 + rhs.0) | help: consider restricting type parameter `B` | -LL | impl<B: std::ops::Add<Output=B>> Add for D<B> { - | +++++++++++++++++++++++++ +LL | impl<B: std::ops::Add<Output = B>> Add for D<B> { + | +++++++++++++++++++++++++++ error[E0308]: mismatched types --> $DIR/missing-bounds.rs:42:14 diff --git a/src/test/ui/suggestions/issue-97677.fixed b/src/test/ui/suggestions/issue-97677.fixed index 73ca9f97b43a3..1e7569fa45106 100644 --- a/src/test/ui/suggestions/issue-97677.fixed +++ b/src/test/ui/suggestions/issue-97677.fixed @@ -1,6 +1,6 @@ // run-rustfix -fn add_ten<N: std::ops::Add<i32, Output=N>>(n: N) -> N { +fn add_ten<N: std::ops::Add<i32, Output = N>>(n: N) -> N { n + 10 //~^ ERROR cannot add `{integer}` to `N` } diff --git a/src/test/ui/suggestions/issue-97677.stderr b/src/test/ui/suggestions/issue-97677.stderr index 069b184ac636c..575d79267f20d 100644 --- a/src/test/ui/suggestions/issue-97677.stderr +++ b/src/test/ui/suggestions/issue-97677.stderr @@ -8,8 +8,8 @@ LL | n + 10 | help: consider restricting type parameter `N` | -LL | fn add_ten<N: std::ops::Add<i32, Output=N>>(n: N) -> N { - | ++++++++++++++++++++++++++++++ +LL | fn add_ten<N: std::ops::Add<i32, Output = N>>(n: N) -> N { + | ++++++++++++++++++++++++++++++++ error: aborting due to previous error diff --git a/src/test/ui/suggestions/restrict-type-not-param.rs b/src/test/ui/suggestions/restrict-type-not-param.rs new file mode 100644 index 0000000000000..60f5ba45c268d --- /dev/null +++ b/src/test/ui/suggestions/restrict-type-not-param.rs @@ -0,0 +1,12 @@ +use std::ops::Add; + +struct Wrapper<T>(T); + +trait Foo {} + +fn qux<T>(a: Wrapper<T>, b: T) -> T { + a + b + //~^ ERROR cannot add `T` to `Wrapper<T>` +} + +fn main() {} diff --git a/src/test/ui/suggestions/restrict-type-not-param.stderr b/src/test/ui/suggestions/restrict-type-not-param.stderr new file mode 100644 index 0000000000000..e7d9c5ecbe482 --- /dev/null +++ b/src/test/ui/suggestions/restrict-type-not-param.stderr @@ -0,0 +1,26 @@ +error[E0369]: cannot add `T` to `Wrapper<T>` + --> $DIR/restrict-type-not-param.rs:8:7 + | +LL | a + b + | - ^ - T + | | + | Wrapper<T> + | +note: an implementation of `Add<_>` might be missing for `Wrapper<T>` + --> $DIR/restrict-type-not-param.rs:3:1 + | +LL | struct Wrapper<T>(T); + | ^^^^^^^^^^^^^^^^^ must implement `Add<_>` +note: the following trait must be implemented + --> $SRC_DIR/core/src/ops/arith.rs:LL:COL + | +LL | pub trait Add<Rhs = Self> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^ +help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement + | +LL | fn qux<T>(a: Wrapper<T>, b: T) -> T where Wrapper<T>: Add<T, Output = T> { + | ++++++++++++++++++++++++++++++++++++ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0369`. diff --git a/src/test/ui/traits/resolution-in-overloaded-op.stderr b/src/test/ui/traits/resolution-in-overloaded-op.stderr index b67e334d40ac2..fe5e1d6d2854d 100644 --- a/src/test/ui/traits/resolution-in-overloaded-op.stderr +++ b/src/test/ui/traits/resolution-in-overloaded-op.stderr @@ -8,8 +8,8 @@ LL | a * b | help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement | -LL | fn foo<T: MyMul<f64, f64>>(a: &T, b: f64) -> f64 where &T: Mul<f64, Output=f64> { - | ++++++++++++++++++++++++++++++ +LL | fn foo<T: MyMul<f64, f64>>(a: &T, b: f64) -> f64 where &T: Mul<f64, Output = f64> { + | ++++++++++++++++++++++++++++++++ error: aborting due to previous error