From 2faaea98f813827ca0f91266652039d2d859989d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 6 Oct 2023 19:07:05 +0000 Subject: [PATCH 01/15] Provide context when `?` can't be called because of `Result<_, E>` When a method chain ending in `?` causes an E0277 because the expression's `Result::Err` variant doesn't have a type that can be converted to the `Result<_, E>` type parameter in the return type, provide additional context of which parts of the chain can and can't support the `?` operator. ``` error[E0277]: `?` couldn't convert the error to `String` --> $DIR/question-mark-result-err-mismatch.rs:28:25 | LL | fn bar() -> Result<(), String> { | ------------------ expected `String` because of this LL | let x = foo(); | ----- this can be annotated with `?` because it has type `Result` LL | let one = x LL | .map(|s| ()) | ----------- this can be annotated with `?` because it has type `Result<(), String>` LL | .map_err(|_| ())?; | ---------------^ the trait `From<()>` is not implemented for `String` | | | this can't be annotated with `?` because it has type `Result<(), ()>` | = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait = help: the following other types implement trait `From`: > >> >> > > > = note: required for `Result<(), String>` to implement `FromResidual>` ``` Fix #72124. --- .../src/traits/error_reporting/suggestions.rs | 18 ++- .../error_reporting/type_err_ctxt_ext.rs | 142 +++++++++++++++++- .../question-mark-result-err-mismatch.rs | 59 ++++++++ .../question-mark-result-err-mismatch.stderr | 83 ++++++++++ tests/ui/try-block/try-block-bad-type.stderr | 4 +- tests/ui/try-trait/bad-interconversion.stderr | 4 +- tests/ui/try-trait/issue-32709.stderr | 4 +- 7 files changed, 302 insertions(+), 12 deletions(-) create mode 100644 tests/ui/traits/question-mark-result-err-mismatch.rs create mode 100644 tests/ui/traits/question-mark-result-err-mismatch.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 9bd1030ac74fa..939c06a1191fd 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -3640,14 +3640,16 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { && let Some(failed_pred) = failed_pred.to_opt_poly_projection_pred() && let Some(found) = failed_pred.skip_binder().term.ty() { - type_diffs = vec![Sorts(ty::error::ExpectedFound { - expected: Ty::new_alias( - self.tcx, - ty::Projection, - where_pred.skip_binder().projection_ty, - ), - found, - })]; + type_diffs = vec![ + Sorts(ty::error::ExpectedFound { + expected: Ty::new_alias( + self.tcx, + ty::Projection, + where_pred.skip_binder().projection_ty, + ), + found, + }), + ]; } } if let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 78c9ac157c0ad..faa806d54a57b 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -3,6 +3,7 @@ use super::suggestions::{get_explanation_based_on_obligation, TypeErrCtxtExt as use crate::errors::{ClosureFnMutLabel, ClosureFnOnceLabel, ClosureKindMismatch}; use crate::infer::error_reporting::{TyCategory, TypeAnnotationNeeded as ErrorCode}; use crate::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; +use crate::infer::InferCtxtExt as _; use crate::infer::{self, InferCtxt}; use crate::traits::error_reporting::infer_ctxt_ext::InferCtxtExt; use crate::traits::error_reporting::{ambiguity, ambiguity::Ambiguity::*}; @@ -40,7 +41,7 @@ use rustc_session::config::{DumpSolverProofTree, TraitSolver}; use rustc_session::Limit; use rustc_span::def_id::LOCAL_CRATE; use rustc_span::symbol::sym; -use rustc_span::{ExpnKind, Span, DUMMY_SP}; +use rustc_span::{BytePos, ExpnKind, Span, DUMMY_SP}; use std::borrow::Cow; use std::fmt; use std::iter; @@ -100,6 +101,13 @@ pub trait TypeErrCtxtExt<'tcx> { fn fn_arg_obligation(&self, obligation: &PredicateObligation<'tcx>) -> bool; + fn try_conversion_context( + &self, + obligation: &PredicateObligation<'tcx>, + trait_ref: ty::TraitRef<'tcx>, + err: &mut Diagnostic, + ); + fn report_const_param_not_wf( &self, ty: Ty<'tcx>, @@ -499,6 +507,10 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { let mut err = struct_span_err!(self.tcx.sess, span, E0277, "{}", err_msg); + if is_try_conversion { + self.try_conversion_context(&obligation, trait_ref.skip_binder(), &mut err); + } + if is_try_conversion && let Some(ret_span) = self.return_type_span(&obligation) { err.span_label( ret_span, @@ -945,6 +957,134 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { false } + /// When the `E` of the resulting `Result` in an expression `foo().bar().baz()?`, + /// identify thoe method chain sub-expressions that could or could not have been annotated + /// with `?`. + fn try_conversion_context( + &self, + obligation: &PredicateObligation<'tcx>, + trait_ref: ty::TraitRef<'tcx>, + err: &mut Diagnostic, + ) { + let span = obligation.cause.span; + struct V<'v> { + search_span: Span, + found: Option<&'v hir::Expr<'v>>, + } + impl<'v> Visitor<'v> for V<'v> { + fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) { + if let hir::ExprKind::Match(expr, _arms, hir::MatchSource::TryDesugar(_)) = ex.kind + { + if ex.span.with_lo(ex.span.hi() - BytePos(1)).source_equal(self.search_span) { + if let hir::ExprKind::Call(_, [expr, ..]) = expr.kind { + self.found = Some(expr); + return; + } + } + } + hir::intravisit::walk_expr(self, ex); + } + } + let hir_id = self.tcx.hir().local_def_id_to_hir_id(obligation.cause.body_id); + let body_id = match self.tcx.hir().find(hir_id) { + Some(hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, _, body_id), .. })) => { + body_id + } + _ => return, + }; + let mut v = V { search_span: span, found: None }; + v.visit_body(self.tcx.hir().body(*body_id)); + let Some(expr) = v.found else { + return; + }; + let Some(typeck) = &self.typeck_results else { + return; + }; + let Some((ObligationCauseCode::QuestionMark, Some(y))) = obligation.cause.code().parent() + else { + return; + }; + if !self.tcx.is_diagnostic_item(sym::FromResidual, y.def_id()) { + return; + } + let self_ty = trait_ref.self_ty(); + + let mut prev_ty = self.resolve_vars_if_possible( + typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)), + ); + let mut annotate_expr = |span: Span, prev_ty: Ty<'tcx>, self_ty: Ty<'tcx>| -> bool { + // We always look at the `E` type, because that's the only one affected by `?`. If the + // incorrect `Result` is because of the `T`, we'll get an E0308 on the whole + // expression, after the `?` has "unwrapped" the `T`. + let ty::Adt(def, args) = prev_ty.kind() else { + return false; + }; + let Some(arg) = args.get(1) else { + return false; + }; + if !self.tcx.is_diagnostic_item(sym::Result, def.did()) { + return false; + } + let can = if self + .infcx + .type_implements_trait( + self.tcx.get_diagnostic_item(sym::From).unwrap(), + [self_ty.into(), *arg], + obligation.param_env, + ) + .must_apply_modulo_regions() + { + "can" + } else { + "can't" + }; + err.span_label( + span, + format!("this {can} be annotated with `?` because it has type `{prev_ty}`"), + ); + true + }; + + // The following logic is simlar to `point_at_chain`, but that's focused on associated types + let mut expr = expr; + while let hir::ExprKind::MethodCall(_path_segment, rcvr_expr, _args, span) = expr.kind { + // Point at every method call in the chain with the `Result` type. + // let foo = bar.iter().map(mapper)?; + // ------ ----------- + expr = rcvr_expr; + if !annotate_expr(span, prev_ty, self_ty) { + break; + } + + prev_ty = self.resolve_vars_if_possible( + typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)), + ); + + if let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind + && let hir::Path { res: hir::def::Res::Local(hir_id), .. } = path + && let Some(hir::Node::Pat(binding)) = self.tcx.hir().find(*hir_id) + && let Some(parent) = self.tcx.hir().find_parent(binding.hir_id) + { + // We've reached the root of the method call chain... + if let hir::Node::Local(local) = parent && let Some(binding_expr) = local.init { + // ...and it is a binding. Get the binding creation and continue the chain. + expr = binding_expr; + } + if let hir::Node::Param(_param) = parent { + // ...and it is a an fn argument. + break; + } + } + } + // `expr` is now the "root" expression of the method call chain, which can be any + // expression kind, like a method call or a path. If this expression is `Result` as + // well, then we also point at it. + prev_ty = self.resolve_vars_if_possible( + typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)), + ); + annotate_expr(expr.span, prev_ty, self_ty); + } + fn report_const_param_not_wf( &self, ty: Ty<'tcx>, diff --git a/tests/ui/traits/question-mark-result-err-mismatch.rs b/tests/ui/traits/question-mark-result-err-mismatch.rs new file mode 100644 index 0000000000000..7b364580858be --- /dev/null +++ b/tests/ui/traits/question-mark-result-err-mismatch.rs @@ -0,0 +1,59 @@ +fn foo() -> Result { //~ NOTE expected `String` because of this + let test = String::from("one,two"); + let x = test + .split_whitespace() + .next() + .ok_or_else(|| { //~ NOTE this can be annotated with `?` because it has type `Result<&str, &str>` + "Couldn't split the test string" + }); + let one = x + .map(|s| ()) //~ NOTE this can be annotated with `?` because it has type `Result<(), &str>` + .map_err(|_| ()) //~ NOTE this can't be annotated with `?` because it has type `Result<(), ()>` + .map(|()| "")?; //~ ERROR `?` couldn't convert the error to `String` + //~^ NOTE in this expansion of desugaring of operator `?` + //~| NOTE in this expansion of desugaring of operator `?` + //~| NOTE in this expansion of desugaring of operator `?` + //~| NOTE in this expansion of desugaring of operator `?` + //~| NOTE this can't be annotated with `?` because it has type `Result<&str, ()>` + //~| NOTE the trait `From<()>` is not implemented for `String` + //~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait + //~| NOTE required for `Result` to implement `FromResidual>` + Ok(one.to_string()) +} + +fn bar() -> Result<(), String> { //~ NOTE expected `String` because of this + let x = foo(); //~ NOTE this can be annotated with `?` because it has type `Result` + let one = x + .map(|s| ()) //~ NOTE this can be annotated with `?` because it has type `Result<(), String>` + .map_err(|_| ())?; //~ ERROR `?` couldn't convert the error to `String` + //~^ NOTE in this expansion of desugaring of operator `?` + //~| NOTE in this expansion of desugaring of operator `?` + //~| NOTE in this expansion of desugaring of operator `?` + //~| NOTE in this expansion of desugaring of operator `?` + //~| NOTE this can't be annotated with `?` because it has type `Result<(), ()>` + //~| NOTE the trait `From<()>` is not implemented for `String` + //~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait + //~| NOTE required for `Result<(), String>` to implement `FromResidual>` + Ok(one) +} + +fn baz() -> Result { //~ NOTE expected `String` because of this + let test = String::from("one,two"); + let one = test + .split_whitespace() + .next() + .ok_or_else(|| { //~ NOTE this can't be annotated with `?` because it has type `Result<&str, ()>` + "Couldn't split the test string"; + })?; + //~^ ERROR `?` couldn't convert the error to `String` + //~| NOTE in this expansion of desugaring of operator `?` + //~| NOTE in this expansion of desugaring of operator `?` + //~| NOTE in this expansion of desugaring of operator `?` + //~| NOTE in this expansion of desugaring of operator `?` + //~| NOTE the trait `From<()>` is not implemented for `String` + //~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait + //~| NOTE required for `Result` to implement `FromResidual>` + Ok(one.to_string()) +} + +fn main() {} diff --git a/tests/ui/traits/question-mark-result-err-mismatch.stderr b/tests/ui/traits/question-mark-result-err-mismatch.stderr new file mode 100644 index 0000000000000..f6acbc6dd0785 --- /dev/null +++ b/tests/ui/traits/question-mark-result-err-mismatch.stderr @@ -0,0 +1,83 @@ +error[E0277]: `?` couldn't convert the error to `String` + --> $DIR/question-mark-result-err-mismatch.rs:12:22 + | +LL | fn foo() -> Result { + | ---------------------- expected `String` because of this +... +LL | .ok_or_else(|| { + | __________- +LL | | "Couldn't split the test string" +LL | | }); + | |__________- this can be annotated with `?` because it has type `Result<&str, &str>` +LL | let one = x +LL | .map(|s| ()) + | ----------- this can be annotated with `?` because it has type `Result<(), &str>` +LL | .map_err(|_| ()) + | --------------- this can't be annotated with `?` because it has type `Result<(), ()>` +LL | .map(|()| "")?; + | ------------^ the trait `From<()>` is not implemented for `String` + | | + | this can't be annotated with `?` because it has type `Result<&str, ()>` + | + = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait + = help: the following other types implement trait `From`: + > + >> + >> + > + > + > + = note: required for `Result` to implement `FromResidual>` + +error[E0277]: `?` couldn't convert the error to `String` + --> $DIR/question-mark-result-err-mismatch.rs:28:25 + | +LL | fn bar() -> Result<(), String> { + | ------------------ expected `String` because of this +LL | let x = foo(); + | ----- this can be annotated with `?` because it has type `Result` +LL | let one = x +LL | .map(|s| ()) + | ----------- this can be annotated with `?` because it has type `Result<(), String>` +LL | .map_err(|_| ())?; + | ---------------^ the trait `From<()>` is not implemented for `String` + | | + | this can't be annotated with `?` because it has type `Result<(), ()>` + | + = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait + = help: the following other types implement trait `From`: + > + >> + >> + > + > + > + = note: required for `Result<(), String>` to implement `FromResidual>` + +error[E0277]: `?` couldn't convert the error to `String` + --> $DIR/question-mark-result-err-mismatch.rs:47:11 + | +LL | fn baz() -> Result { + | ---------------------- expected `String` because of this +... +LL | .ok_or_else(|| { + | __________- +LL | | "Couldn't split the test string"; +LL | | })?; + | | -^ the trait `From<()>` is not implemented for `String` + | |__________| + | this can't be annotated with `?` because it has type `Result<&str, ()>` + | + = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait + = help: the following other types implement trait `From`: + > + >> + >> + > + > + > + = note: required for `Result` to implement `FromResidual>` + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/try-block/try-block-bad-type.stderr b/tests/ui/try-block/try-block-bad-type.stderr index b41bf86d3d911..d58a011ff5501 100644 --- a/tests/ui/try-block/try-block-bad-type.stderr +++ b/tests/ui/try-block/try-block-bad-type.stderr @@ -2,7 +2,9 @@ error[E0277]: `?` couldn't convert the error to `TryFromSliceError` --> $DIR/try-block-bad-type.rs:7:16 | LL | Err("")?; - | ^ the trait `From<&str>` is not implemented for `TryFromSliceError` + | -------^ the trait `From<&str>` is not implemented for `TryFromSliceError` + | | + | this can't be annotated with `?` because it has type `Result<_, &str>` | = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait = help: the trait `From` is implemented for `TryFromSliceError` diff --git a/tests/ui/try-trait/bad-interconversion.stderr b/tests/ui/try-trait/bad-interconversion.stderr index d8b9431becc6b..97fbbdbf8f8a8 100644 --- a/tests/ui/try-trait/bad-interconversion.stderr +++ b/tests/ui/try-trait/bad-interconversion.stderr @@ -4,7 +4,9 @@ error[E0277]: `?` couldn't convert the error to `u8` LL | fn result_to_result() -> Result { | --------------- expected `u8` because of this LL | Ok(Err(123_i32)?) - | ^ the trait `From` is not implemented for `u8` + | ------------^ the trait `From` is not implemented for `u8` + | | + | this can't be annotated with `?` because it has type `Result<_, i32>` | = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait = help: the following other types implement trait `From`: diff --git a/tests/ui/try-trait/issue-32709.stderr b/tests/ui/try-trait/issue-32709.stderr index 94e8f9295fdd5..6e0f464d16152 100644 --- a/tests/ui/try-trait/issue-32709.stderr +++ b/tests/ui/try-trait/issue-32709.stderr @@ -4,7 +4,9 @@ error[E0277]: `?` couldn't convert the error to `()` LL | fn a() -> Result { | --------------- expected `()` because of this LL | Err(5)?; - | ^ the trait `From<{integer}>` is not implemented for `()` + | ------^ the trait `From<{integer}>` is not implemented for `()` + | | + | this can't be annotated with `?` because it has type `Result<_, {integer}>` | = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait = help: the following other types implement trait `From`: From 276b74d1fff0c688ffb51bcce04d7cca65cdd3af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 7 Oct 2023 01:14:43 +0000 Subject: [PATCH 02/15] Point at fewer methods in the chain, only those that change the E type --- .../error_reporting/type_err_ctxt_ext.rs | 76 ++++++++++++------- .../question-mark-result-err-mismatch.rs | 15 ++-- .../question-mark-result-err-mismatch.stderr | 26 +++---- 3 files changed, 64 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index faa806d54a57b..875cb71f3ace9 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -1012,39 +1012,25 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { let mut prev_ty = self.resolve_vars_if_possible( typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)), ); - let mut annotate_expr = |span: Span, prev_ty: Ty<'tcx>, self_ty: Ty<'tcx>| -> bool { - // We always look at the `E` type, because that's the only one affected by `?`. If the - // incorrect `Result` is because of the `T`, we'll get an E0308 on the whole - // expression, after the `?` has "unwrapped" the `T`. + + // We always look at the `E` type, because that's the only one affected by `?`. If the + // incorrect `Result` is because of the `T`, we'll get an E0308 on the whole + // expression, after the `?` has "unwrapped" the `T`. + let get_e_type = |prev_ty: Ty<'tcx>| -> Option> { let ty::Adt(def, args) = prev_ty.kind() else { - return false; + return None; }; let Some(arg) = args.get(1) else { - return false; + return None; }; if !self.tcx.is_diagnostic_item(sym::Result, def.did()) { - return false; + return None; } - let can = if self - .infcx - .type_implements_trait( - self.tcx.get_diagnostic_item(sym::From).unwrap(), - [self_ty.into(), *arg], - obligation.param_env, - ) - .must_apply_modulo_regions() - { - "can" - } else { - "can't" - }; - err.span_label( - span, - format!("this {can} be annotated with `?` because it has type `{prev_ty}`"), - ); - true + Some(arg.as_type()?) }; + let mut chain = vec![]; + // The following logic is simlar to `point_at_chain`, but that's focused on associated types let mut expr = expr; while let hir::ExprKind::MethodCall(_path_segment, rcvr_expr, _args, span) = expr.kind { @@ -1052,9 +1038,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { // let foo = bar.iter().map(mapper)?; // ------ ----------- expr = rcvr_expr; - if !annotate_expr(span, prev_ty, self_ty) { - break; - } + chain.push((span, prev_ty)); prev_ty = self.resolve_vars_if_possible( typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)), @@ -1082,7 +1066,41 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { prev_ty = self.resolve_vars_if_possible( typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)), ); - annotate_expr(expr.span, prev_ty, self_ty); + chain.push((expr.span, prev_ty)); + + let mut prev = None; + for (span, err_ty) in chain.into_iter().rev() { + let err_ty = get_e_type(err_ty); + let err_ty = match (err_ty, prev) { + (Some(err_ty), Some(prev)) if !self.can_eq(obligation.param_env, err_ty, prev) => { + err_ty + } + (Some(err_ty), None) => err_ty, + _ => { + prev = err_ty; + continue; + } + }; + if self + .infcx + .type_implements_trait( + self.tcx.get_diagnostic_item(sym::From).unwrap(), + [self_ty, err_ty], + obligation.param_env, + ) + .must_apply_modulo_regions() + { + err.span_label(span, format!("this has type `Result<_, {err_ty}>`")); + } else { + err.span_label( + span, + format!( + "this can't be annotated with `?` because it has type `Result<_, {err_ty}>`", + ), + ); + } + prev = Some(err_ty); + } } fn report_const_param_not_wf( diff --git a/tests/ui/traits/question-mark-result-err-mismatch.rs b/tests/ui/traits/question-mark-result-err-mismatch.rs index 7b364580858be..e5ccca2e5f7c7 100644 --- a/tests/ui/traits/question-mark-result-err-mismatch.rs +++ b/tests/ui/traits/question-mark-result-err-mismatch.rs @@ -3,18 +3,17 @@ fn foo() -> Result { //~ NOTE expected `String` because of this let x = test .split_whitespace() .next() - .ok_or_else(|| { //~ NOTE this can be annotated with `?` because it has type `Result<&str, &str>` + .ok_or_else(|| { //~ NOTE this has type `Result<_, &str>` "Couldn't split the test string" }); let one = x - .map(|s| ()) //~ NOTE this can be annotated with `?` because it has type `Result<(), &str>` - .map_err(|_| ()) //~ NOTE this can't be annotated with `?` because it has type `Result<(), ()>` + .map(|s| ()) + .map_err(|_| ()) //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>` .map(|()| "")?; //~ ERROR `?` couldn't convert the error to `String` //~^ NOTE in this expansion of desugaring of operator `?` //~| NOTE in this expansion of desugaring of operator `?` //~| NOTE in this expansion of desugaring of operator `?` //~| NOTE in this expansion of desugaring of operator `?` - //~| NOTE this can't be annotated with `?` because it has type `Result<&str, ()>` //~| NOTE the trait `From<()>` is not implemented for `String` //~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait //~| NOTE required for `Result` to implement `FromResidual>` @@ -22,15 +21,15 @@ fn foo() -> Result { //~ NOTE expected `String` because of this } fn bar() -> Result<(), String> { //~ NOTE expected `String` because of this - let x = foo(); //~ NOTE this can be annotated with `?` because it has type `Result` + let x = foo(); //~ NOTE this has type `Result<_, String>` let one = x - .map(|s| ()) //~ NOTE this can be annotated with `?` because it has type `Result<(), String>` + .map(|s| ()) .map_err(|_| ())?; //~ ERROR `?` couldn't convert the error to `String` //~^ NOTE in this expansion of desugaring of operator `?` //~| NOTE in this expansion of desugaring of operator `?` //~| NOTE in this expansion of desugaring of operator `?` //~| NOTE in this expansion of desugaring of operator `?` - //~| NOTE this can't be annotated with `?` because it has type `Result<(), ()>` + //~| NOTE this can't be annotated with `?` because it has type `Result<_, ()>` //~| NOTE the trait `From<()>` is not implemented for `String` //~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait //~| NOTE required for `Result<(), String>` to implement `FromResidual>` @@ -42,7 +41,7 @@ fn baz() -> Result { //~ NOTE expected `String` because of this let one = test .split_whitespace() .next() - .ok_or_else(|| { //~ NOTE this can't be annotated with `?` because it has type `Result<&str, ()>` + .ok_or_else(|| { //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>` "Couldn't split the test string"; })?; //~^ ERROR `?` couldn't convert the error to `String` diff --git a/tests/ui/traits/question-mark-result-err-mismatch.stderr b/tests/ui/traits/question-mark-result-err-mismatch.stderr index f6acbc6dd0785..fc3b2e6b46b38 100644 --- a/tests/ui/traits/question-mark-result-err-mismatch.stderr +++ b/tests/ui/traits/question-mark-result-err-mismatch.stderr @@ -8,16 +8,12 @@ LL | .ok_or_else(|| { | __________- LL | | "Couldn't split the test string" LL | | }); - | |__________- this can be annotated with `?` because it has type `Result<&str, &str>` -LL | let one = x -LL | .map(|s| ()) - | ----------- this can be annotated with `?` because it has type `Result<(), &str>` + | |__________- this has type `Result<_, &str>` +... LL | .map_err(|_| ()) - | --------------- this can't be annotated with `?` because it has type `Result<(), ()>` + | --------------- this can't be annotated with `?` because it has type `Result<_, ()>` LL | .map(|()| "")?; - | ------------^ the trait `From<()>` is not implemented for `String` - | | - | this can't be annotated with `?` because it has type `Result<&str, ()>` + | ^ the trait `From<()>` is not implemented for `String` | = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait = help: the following other types implement trait `From`: @@ -30,19 +26,17 @@ LL | .map(|()| "")?; = note: required for `Result` to implement `FromResidual>` error[E0277]: `?` couldn't convert the error to `String` - --> $DIR/question-mark-result-err-mismatch.rs:28:25 + --> $DIR/question-mark-result-err-mismatch.rs:27:25 | LL | fn bar() -> Result<(), String> { | ------------------ expected `String` because of this LL | let x = foo(); - | ----- this can be annotated with `?` because it has type `Result` -LL | let one = x -LL | .map(|s| ()) - | ----------- this can be annotated with `?` because it has type `Result<(), String>` + | ----- this has type `Result<_, String>` +... LL | .map_err(|_| ())?; | ---------------^ the trait `From<()>` is not implemented for `String` | | - | this can't be annotated with `?` because it has type `Result<(), ()>` + | this can't be annotated with `?` because it has type `Result<_, ()>` | = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait = help: the following other types implement trait `From`: @@ -55,7 +49,7 @@ LL | .map_err(|_| ())?; = note: required for `Result<(), String>` to implement `FromResidual>` error[E0277]: `?` couldn't convert the error to `String` - --> $DIR/question-mark-result-err-mismatch.rs:47:11 + --> $DIR/question-mark-result-err-mismatch.rs:46:11 | LL | fn baz() -> Result { | ---------------------- expected `String` because of this @@ -66,7 +60,7 @@ LL | | "Couldn't split the test string"; LL | | })?; | | -^ the trait `From<()>` is not implemented for `String` | |__________| - | this can't be annotated with `?` because it has type `Result<&str, ()>` + | this can't be annotated with `?` because it has type `Result<_, ()>` | = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait = help: the following other types implement trait `From`: From e6887429a25d4b1bd7a44033a1681bdc42f7069a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 7 Oct 2023 03:47:02 +0000 Subject: [PATCH 03/15] Detect incorrect `;` in `Option::ok_or_else` and `Result::map_err` Fix #72124. --- compiler/rustc_span/src/symbol.rs | 2 + .../error_reporting/type_err_ctxt_ext.rs | 66 ++++++++++++++++++- .../question-mark-result-err-mismatch.rs | 9 ++- .../question-mark-result-err-mismatch.stderr | 15 +++-- 4 files changed, 82 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 302be85a429f4..1e1fa6778f04a 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -988,6 +988,7 @@ symbols! { managed_boxes, manually_drop, map, + map_err, marker, marker_trait_attr, masked, @@ -1152,6 +1153,7 @@ symbols! { offset, offset_of, offset_of_enum, + ok_or_else, omit_gdb_pretty_printer_section, on, on_unimplemented, diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 875cb71f3ace9..d9a2fd1810545 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -41,7 +41,7 @@ use rustc_session::config::{DumpSolverProofTree, TraitSolver}; use rustc_session::Limit; use rustc_span::def_id::LOCAL_CRATE; use rustc_span::symbol::sym; -use rustc_span::{BytePos, ExpnKind, Span, DUMMY_SP}; +use rustc_span::{BytePos, ExpnKind, Span, Symbol, DUMMY_SP}; use std::borrow::Cow; use std::fmt; use std::iter; @@ -1008,6 +1008,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { return; } let self_ty = trait_ref.self_ty(); + let found_ty = trait_ref.args.get(1).and_then(|a| a.as_type()); let mut prev_ty = self.resolve_vars_if_possible( typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)), @@ -1033,17 +1034,76 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { // The following logic is simlar to `point_at_chain`, but that's focused on associated types let mut expr = expr; - while let hir::ExprKind::MethodCall(_path_segment, rcvr_expr, _args, span) = expr.kind { + while let hir::ExprKind::MethodCall(path_segment, rcvr_expr, args, span) = expr.kind { // Point at every method call in the chain with the `Result` type. // let foo = bar.iter().map(mapper)?; // ------ ----------- expr = rcvr_expr; chain.push((span, prev_ty)); - prev_ty = self.resolve_vars_if_possible( + let next_ty = self.resolve_vars_if_possible( typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)), ); + let is_diagnostic_item = |symbol: Symbol, ty: Ty<'tcx>| { + let ty::Adt(def, _) = ty.kind() else { + return false; + }; + self.tcx.is_diagnostic_item(symbol, def.did()) + }; + // For each method in the chain, see if this is `Result::map_err` or + // `Option::ok_or_else` and if it is, see if the closure passed to it has an incorrect + // trailing `;`. + // Ideally we would instead use `FnCtxt::lookup_method_for_diagnostic` for 100% + // accurate check, but we are in the wrong stage to do that and looking for + // `Result::map_err` by checking the Self type and the path segment is enough. + // sym::ok_or_else + if let Some(ty) = get_e_type(prev_ty) + && let Some(found_ty) = found_ty + && ( + ( + path_segment.ident.name == sym::map_err + && is_diagnostic_item(sym::Result, next_ty) + ) || ( + path_segment.ident.name == sym::ok_or_else + && is_diagnostic_item(sym::Option, next_ty) + ) + ) + && [sym::map_err, sym::ok_or_else].contains(&path_segment.ident.name) + && let ty::Tuple(tys) = found_ty.kind() + && tys.is_empty() + && self.can_eq(obligation.param_env, ty, found_ty) + && args.len() == 1 + && let Some(arg) = args.get(0) + && let hir::ExprKind::Closure(closure) = arg.kind + && let body = self.tcx.hir().body(closure.body) + && let hir::ExprKind::Block(block, _) = body.value.kind + && let None = block.expr + && let [.., stmt] = block.stmts + && let hir::StmtKind::Semi(expr) = stmt.kind + && let expr_ty = self.resolve_vars_if_possible( + typeck.expr_ty_adjusted_opt(expr) + .unwrap_or(Ty::new_misc_error(self.tcx)), + ) + && self + .infcx + .type_implements_trait( + self.tcx.get_diagnostic_item(sym::From).unwrap(), + [self_ty, expr_ty], + obligation.param_env, + ) + .must_apply_modulo_regions() + { + err.span_suggestion_short( + stmt.span.with_lo(expr.span.hi()), + "remove this semicolon", + String::new(), + Applicability::MachineApplicable, + ); + } + + prev_ty = next_ty; + if let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind && let hir::Path { res: hir::def::Res::Local(hir_id), .. } = path && let Some(hir::Node::Pat(binding)) = self.tcx.hir().find(*hir_id) diff --git a/tests/ui/traits/question-mark-result-err-mismatch.rs b/tests/ui/traits/question-mark-result-err-mismatch.rs index e5ccca2e5f7c7..317029e004613 100644 --- a/tests/ui/traits/question-mark-result-err-mismatch.rs +++ b/tests/ui/traits/question-mark-result-err-mismatch.rs @@ -8,7 +8,9 @@ fn foo() -> Result { //~ NOTE expected `String` because of this }); let one = x .map(|s| ()) - .map_err(|_| ()) //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>` + .map_err(|e| { //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>` + e; //~ HELP remove this semicolon + }) .map(|()| "")?; //~ ERROR `?` couldn't convert the error to `String` //~^ NOTE in this expansion of desugaring of operator `?` //~| NOTE in this expansion of desugaring of operator `?` @@ -17,6 +19,7 @@ fn foo() -> Result { //~ NOTE expected `String` because of this //~| NOTE the trait `From<()>` is not implemented for `String` //~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait //~| NOTE required for `Result` to implement `FromResidual>` + //~| HELP the following other types implement trait `From`: Ok(one.to_string()) } @@ -33,6 +36,7 @@ fn bar() -> Result<(), String> { //~ NOTE expected `String` because of this //~| NOTE the trait `From<()>` is not implemented for `String` //~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait //~| NOTE required for `Result<(), String>` to implement `FromResidual>` + //~| HELP the following other types implement trait `From`: Ok(one) } @@ -42,7 +46,7 @@ fn baz() -> Result { //~ NOTE expected `String` because of this .split_whitespace() .next() .ok_or_else(|| { //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>` - "Couldn't split the test string"; + "Couldn't split the test string"; //~ HELP remove this semicolon })?; //~^ ERROR `?` couldn't convert the error to `String` //~| NOTE in this expansion of desugaring of operator `?` @@ -52,6 +56,7 @@ fn baz() -> Result { //~ NOTE expected `String` because of this //~| NOTE the trait `From<()>` is not implemented for `String` //~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait //~| NOTE required for `Result` to implement `FromResidual>` + //~| HELP the following other types implement trait `From`: Ok(one.to_string()) } diff --git a/tests/ui/traits/question-mark-result-err-mismatch.stderr b/tests/ui/traits/question-mark-result-err-mismatch.stderr index fc3b2e6b46b38..1f9495a505ac3 100644 --- a/tests/ui/traits/question-mark-result-err-mismatch.stderr +++ b/tests/ui/traits/question-mark-result-err-mismatch.stderr @@ -1,5 +1,5 @@ error[E0277]: `?` couldn't convert the error to `String` - --> $DIR/question-mark-result-err-mismatch.rs:12:22 + --> $DIR/question-mark-result-err-mismatch.rs:14:22 | LL | fn foo() -> Result { | ---------------------- expected `String` because of this @@ -10,8 +10,12 @@ LL | | "Couldn't split the test string" LL | | }); | |__________- this has type `Result<_, &str>` ... -LL | .map_err(|_| ()) - | --------------- this can't be annotated with `?` because it has type `Result<_, ()>` +LL | .map_err(|e| { + | __________- +LL | | e; + | | - help: remove this semicolon +LL | | }) + | |__________- this can't be annotated with `?` because it has type `Result<_, ()>` LL | .map(|()| "")?; | ^ the trait `From<()>` is not implemented for `String` | @@ -26,7 +30,7 @@ LL | .map(|()| "")?; = note: required for `Result` to implement `FromResidual>` error[E0277]: `?` couldn't convert the error to `String` - --> $DIR/question-mark-result-err-mismatch.rs:27:25 + --> $DIR/question-mark-result-err-mismatch.rs:30:25 | LL | fn bar() -> Result<(), String> { | ------------------ expected `String` because of this @@ -49,7 +53,7 @@ LL | .map_err(|_| ())?; = note: required for `Result<(), String>` to implement `FromResidual>` error[E0277]: `?` couldn't convert the error to `String` - --> $DIR/question-mark-result-err-mismatch.rs:46:11 + --> $DIR/question-mark-result-err-mismatch.rs:50:11 | LL | fn baz() -> Result { | ---------------------- expected `String` because of this @@ -57,6 +61,7 @@ LL | fn baz() -> Result { LL | .ok_or_else(|| { | __________- LL | | "Couldn't split the test string"; + | | - help: remove this semicolon LL | | })?; | | -^ the trait `From<()>` is not implemented for `String` | |__________| From cc49398f9724d3357b2f6128ad0b2b5918f05c3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 7 Oct 2023 03:58:45 +0000 Subject: [PATCH 04/15] Reduce verbosity of error --- .../error_reporting/type_err_ctxt_ext.rs | 37 +++++++++++-------- .../question-mark-result-err-mismatch.rs | 6 +-- .../question-mark-result-err-mismatch.stderr | 24 +----------- 3 files changed, 24 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index d9a2fd1810545..89fee338a8b05 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -106,7 +106,7 @@ pub trait TypeErrCtxtExt<'tcx> { obligation: &PredicateObligation<'tcx>, trait_ref: ty::TraitRef<'tcx>, err: &mut Diagnostic, - ); + ) -> bool; fn report_const_param_not_wf( &self, @@ -507,8 +507,9 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { let mut err = struct_span_err!(self.tcx.sess, span, E0277, "{}", err_msg); + let mut suggested = false; if is_try_conversion { - self.try_conversion_context(&obligation, trait_ref.skip_binder(), &mut err); + suggested = self.try_conversion_context(&obligation, trait_ref.skip_binder(), &mut err); } if is_try_conversion && let Some(ret_span) = self.return_type_span(&obligation) { @@ -611,8 +612,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { self.suggest_floating_point_literal(&obligation, &mut err, &trait_ref); self.suggest_dereferencing_index(&obligation, &mut err, trait_predicate); - let mut suggested = - self.suggest_dereferences(&obligation, &mut err, trait_predicate); + suggested |= self.suggest_dereferences(&obligation, &mut err, trait_predicate); suggested |= self.suggest_fn_call(&obligation, &mut err, trait_predicate); let impl_candidates = self.find_similar_impl_candidates(trait_predicate); suggested = if let &[cand] = &impl_candidates[..] { @@ -965,7 +965,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { obligation: &PredicateObligation<'tcx>, trait_ref: ty::TraitRef<'tcx>, err: &mut Diagnostic, - ) { + ) -> bool { let span = obligation.cause.span; struct V<'v> { search_span: Span, @@ -990,22 +990,22 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { Some(hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, _, body_id), .. })) => { body_id } - _ => return, + _ => return false, }; let mut v = V { search_span: span, found: None }; v.visit_body(self.tcx.hir().body(*body_id)); let Some(expr) = v.found else { - return; + return false; }; let Some(typeck) = &self.typeck_results else { - return; + return false; }; let Some((ObligationCauseCode::QuestionMark, Some(y))) = obligation.cause.code().parent() else { - return; + return false; }; if !self.tcx.is_diagnostic_item(sym::FromResidual, y.def_id()) { - return; + return false; } let self_ty = trait_ref.self_ty(); let found_ty = trait_ref.args.get(1).and_then(|a| a.as_type()); @@ -1030,6 +1030,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { Some(arg.as_type()?) }; + let mut suggested = false; let mut chain = vec![]; // The following logic is simlar to `point_at_chain`, but that's focused on associated types @@ -1094,6 +1095,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { ) .must_apply_modulo_regions() { + suggested = true; err.span_suggestion_short( stmt.span.with_lo(expr.span.hi()), "remove this semicolon", @@ -1150,17 +1152,20 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { ) .must_apply_modulo_regions() { - err.span_label(span, format!("this has type `Result<_, {err_ty}>`")); + if !suggested { + err.span_label(span, format!("this has type `Result<_, {err_ty}>`")); + } } else { err.span_label( - span, - format!( - "this can't be annotated with `?` because it has type `Result<_, {err_ty}>`", - ), - ); + span, + format!( + "this can't be annotated with `?` because it has type `Result<_, {err_ty}>`", + ), + ); } prev = Some(err_ty); } + suggested } fn report_const_param_not_wf( diff --git a/tests/ui/traits/question-mark-result-err-mismatch.rs b/tests/ui/traits/question-mark-result-err-mismatch.rs index 317029e004613..0ca18b5b0ddce 100644 --- a/tests/ui/traits/question-mark-result-err-mismatch.rs +++ b/tests/ui/traits/question-mark-result-err-mismatch.rs @@ -3,7 +3,7 @@ fn foo() -> Result { //~ NOTE expected `String` because of this let x = test .split_whitespace() .next() - .ok_or_else(|| { //~ NOTE this has type `Result<_, &str>` + .ok_or_else(|| { "Couldn't split the test string" }); let one = x @@ -15,11 +15,9 @@ fn foo() -> Result { //~ NOTE expected `String` because of this //~^ NOTE in this expansion of desugaring of operator `?` //~| NOTE in this expansion of desugaring of operator `?` //~| NOTE in this expansion of desugaring of operator `?` - //~| NOTE in this expansion of desugaring of operator `?` //~| NOTE the trait `From<()>` is not implemented for `String` //~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait //~| NOTE required for `Result` to implement `FromResidual>` - //~| HELP the following other types implement trait `From`: Ok(one.to_string()) } @@ -52,11 +50,9 @@ fn baz() -> Result { //~ NOTE expected `String` because of this //~| NOTE in this expansion of desugaring of operator `?` //~| NOTE in this expansion of desugaring of operator `?` //~| NOTE in this expansion of desugaring of operator `?` - //~| NOTE in this expansion of desugaring of operator `?` //~| NOTE the trait `From<()>` is not implemented for `String` //~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait //~| NOTE required for `Result` to implement `FromResidual>` - //~| HELP the following other types implement trait `From`: Ok(one.to_string()) } diff --git a/tests/ui/traits/question-mark-result-err-mismatch.stderr b/tests/ui/traits/question-mark-result-err-mismatch.stderr index 1f9495a505ac3..3059e0beca3e4 100644 --- a/tests/ui/traits/question-mark-result-err-mismatch.stderr +++ b/tests/ui/traits/question-mark-result-err-mismatch.stderr @@ -4,12 +4,6 @@ error[E0277]: `?` couldn't convert the error to `String` LL | fn foo() -> Result { | ---------------------- expected `String` because of this ... -LL | .ok_or_else(|| { - | __________- -LL | | "Couldn't split the test string" -LL | | }); - | |__________- this has type `Result<_, &str>` -... LL | .map_err(|e| { | __________- LL | | e; @@ -20,17 +14,10 @@ LL | .map(|()| "")?; | ^ the trait `From<()>` is not implemented for `String` | = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait - = help: the following other types implement trait `From`: - > - >> - >> - > - > - > = note: required for `Result` to implement `FromResidual>` error[E0277]: `?` couldn't convert the error to `String` - --> $DIR/question-mark-result-err-mismatch.rs:30:25 + --> $DIR/question-mark-result-err-mismatch.rs:28:25 | LL | fn bar() -> Result<(), String> { | ------------------ expected `String` because of this @@ -53,7 +40,7 @@ LL | .map_err(|_| ())?; = note: required for `Result<(), String>` to implement `FromResidual>` error[E0277]: `?` couldn't convert the error to `String` - --> $DIR/question-mark-result-err-mismatch.rs:50:11 + --> $DIR/question-mark-result-err-mismatch.rs:48:11 | LL | fn baz() -> Result { | ---------------------- expected `String` because of this @@ -68,13 +55,6 @@ LL | | })?; | this can't be annotated with `?` because it has type `Result<_, ()>` | = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait - = help: the following other types implement trait `From`: - > - >> - >> - > - > - > = note: required for `Result` to implement `FromResidual>` error: aborting due to 3 previous errors From d92c2b5ec16e4e77052c3c815f0331cff5635641 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 7 Oct 2023 04:55:20 +0000 Subject: [PATCH 05/15] add comments --- .../error_reporting/type_err_ctxt_ext.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 89fee338a8b05..abe589b5ab210 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -1055,31 +1055,35 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { // For each method in the chain, see if this is `Result::map_err` or // `Option::ok_or_else` and if it is, see if the closure passed to it has an incorrect // trailing `;`. - // Ideally we would instead use `FnCtxt::lookup_method_for_diagnostic` for 100% - // accurate check, but we are in the wrong stage to do that and looking for - // `Result::map_err` by checking the Self type and the path segment is enough. - // sym::ok_or_else if let Some(ty) = get_e_type(prev_ty) && let Some(found_ty) = found_ty + // Ideally we would instead use `FnCtxt::lookup_method_for_diagnostic` for 100% + // accurate check, but we are in the wrong stage to do that and looking for + // `Result::map_err` by checking the Self type and the path segment is enough. + // sym::ok_or_else && ( - ( + ( // Result::map_err path_segment.ident.name == sym::map_err && is_diagnostic_item(sym::Result, next_ty) - ) || ( + ) || ( // Option::ok_or_else path_segment.ident.name == sym::ok_or_else && is_diagnostic_item(sym::Option, next_ty) ) ) - && [sym::map_err, sym::ok_or_else].contains(&path_segment.ident.name) + // Found `Result<_, ()>?` && let ty::Tuple(tys) = found_ty.kind() && tys.is_empty() + // The current method call returns `Result<_, ()>` && self.can_eq(obligation.param_env, ty, found_ty) + // There's a single argument in the method call and it is a closure && args.len() == 1 && let Some(arg) = args.get(0) && let hir::ExprKind::Closure(closure) = arg.kind + // The closure has a block for its body with no tail expression && let body = self.tcx.hir().body(closure.body) && let hir::ExprKind::Block(block, _) = body.value.kind && let None = block.expr + // The last statement is of a type that can be converted to the return error type && let [.., stmt] = block.stmts && let hir::StmtKind::Semi(expr) = stmt.kind && let expr_ty = self.resolve_vars_if_possible( From cce82d862cc422707073f9ed80e7695f9c44ff8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 16 Nov 2023 17:01:21 +0000 Subject: [PATCH 06/15] let-chain fmt --- .../src/traits/error_reporting/suggestions.rs | 18 ++++++++---------- .../error_reporting/type_err_ctxt_ext.rs | 4 +++- 2 files changed, 11 insertions(+), 11 deletions(-) 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 939c06a1191fd..9bd1030ac74fa 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -3640,16 +3640,14 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { && let Some(failed_pred) = failed_pred.to_opt_poly_projection_pred() && let Some(found) = failed_pred.skip_binder().term.ty() { - type_diffs = vec![ - Sorts(ty::error::ExpectedFound { - expected: Ty::new_alias( - self.tcx, - ty::Projection, - where_pred.skip_binder().projection_ty, - ), - found, - }), - ]; + type_diffs = vec![Sorts(ty::error::ExpectedFound { + expected: Ty::new_alias( + self.tcx, + ty::Projection, + where_pred.skip_binder().projection_ty, + ), + found, + })]; } } if let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index abe589b5ab210..cfc9d32498e87 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -1116,7 +1116,9 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { && let Some(parent) = self.tcx.hir().find_parent(binding.hir_id) { // We've reached the root of the method call chain... - if let hir::Node::Local(local) = parent && let Some(binding_expr) = local.init { + if let hir::Node::Local(local) = parent + && let Some(binding_expr) = local.init + { // ...and it is a binding. Get the binding creation and continue the chain. expr = binding_expr; } From d1583eba66bf1ccb65c3aeaa5de30bcc90ae208c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 21 Nov 2023 08:00:26 +0100 Subject: [PATCH 07/15] lib features ending in '_internals?' are internal --- compiler/rustc_feature/src/unstable.rs | 18 ++++++++++++------ tests/ui/lint/internal_features.rs | 11 +++++++++++ tests/ui/lint/internal_features.stderr | 23 +++++++++++++++++++++++ 3 files changed, 46 insertions(+), 6 deletions(-) create mode 100644 tests/ui/lint/internal_features.rs create mode 100644 tests/ui/lint/internal_features.stderr diff --git a/compiler/rustc_feature/src/unstable.rs b/compiler/rustc_feature/src/unstable.rs index b11b190bdedad..40611102250b2 100644 --- a/compiler/rustc_feature/src/unstable.rs +++ b/compiler/rustc_feature/src/unstable.rs @@ -54,8 +54,10 @@ macro_rules! declare_features { #[derive(Clone, Default, Debug)] pub struct Features { /// `#![feature]` attrs for language features, for error reporting. + /// "declared" here means that the feature is actually enabled in the current crate. pub declared_lang_features: Vec<(Symbol, Span, Option)>, /// `#![feature]` attrs for non-language (library) features. + /// "declared" here means that the feature is actually enabled in the current crate. pub declared_lib_features: Vec<(Symbol, Span)>, /// `declared_lang_features` + `declared_lib_features`. pub declared_features: FxHashSet, @@ -125,9 +127,16 @@ macro_rules! declare_features { $( sym::$feature => status_to_enum!($status) == FeatureStatus::Internal, )* - // Accepted/removed features aren't in this file but are never internal - // (a removed feature might have been internal, but that's now irrelevant). - _ if self.declared_features.contains(&feature) => false, + _ if self.declared_features.contains(&feature) => { + // This could be accepted/removed, or a libs feature. + // Accepted/removed features aren't in this file but are never internal + // (a removed feature might have been internal, but that's now irrelevant). + // Libs features are internal if they end in `_internal` or `_internals`. + // We just always test the name; it's not a big deal if we accidentally hit + // an accepted/removed lang feature that way. + let name = feature.as_str(); + name.ends_with("_internal") || name.ends_with("_internals") + } _ => panic!("`{}` was not listed in `declare_features`", feature), } } @@ -207,9 +216,6 @@ declare_features! ( (internal, test_2018_feature, "1.31.0", None, Some(Edition::Edition2018)), /// Added for testing unstable lints; perma-unstable. (internal, test_unstable_lint, "1.60.0", None, None), - /// Allows non-`unsafe` —and thus, unsound— access to `Pin` constructions. - /// Marked `internal` since perma-unstable and unsound. - (internal, unsafe_pin_internals, "1.60.0", None, None), /// Use for stable + negative coherence and strict coherence depending on trait's /// rustc_strict_coherence value. (unstable, with_negative_coherence, "1.60.0", None, None), diff --git a/tests/ui/lint/internal_features.rs b/tests/ui/lint/internal_features.rs new file mode 100644 index 0000000000000..32ce9540cb364 --- /dev/null +++ b/tests/ui/lint/internal_features.rs @@ -0,0 +1,11 @@ +#![forbid(internal_features)] +// A lang feature and a lib feature. +#![feature(intrinsics, panic_internals)] +//~^ ERROR: internal +//~| ERROR: internal + +extern "rust-intrinsic" { + fn copy_nonoverlapping(src: *const T, dst: *mut T, count: usize); +} + +fn main() {} diff --git a/tests/ui/lint/internal_features.stderr b/tests/ui/lint/internal_features.stderr new file mode 100644 index 0000000000000..8b52bef8f033b --- /dev/null +++ b/tests/ui/lint/internal_features.stderr @@ -0,0 +1,23 @@ +error: the feature `intrinsics` is internal to the compiler or standard library + --> $DIR/internal_features.rs:3:12 + | +LL | #![feature(intrinsics, panic_internals)] + | ^^^^^^^^^^ + | + = note: using it is strongly discouraged +note: the lint level is defined here + --> $DIR/internal_features.rs:1:11 + | +LL | #![forbid(internal_features)] + | ^^^^^^^^^^^^^^^^^ + +error: the feature `panic_internals` is internal to the compiler or standard library + --> $DIR/internal_features.rs:3:24 + | +LL | #![feature(intrinsics, panic_internals)] + | ^^^^^^^^^^^^^^^ + | + = note: using it is strongly discouraged + +error: aborting due to 2 previous errors + From 74834a9d74ce2a26bf0ee609ce2790299226f742 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 22 Nov 2023 07:30:09 +0100 Subject: [PATCH 08/15] also make 'core_intrinsics' internal --- compiler/rustc_feature/src/unstable.rs | 4 +++- compiler/rustc_query_system/src/lib.rs | 2 +- library/alloc/tests/lib.rs | 1 + library/core/src/intrinsics.rs | 7 +++++++ library/core/tests/lib.rs | 1 + src/tools/rust-analyzer/crates/proc-macro-srv/src/lib.rs | 2 +- 6 files changed, 14 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_feature/src/unstable.rs b/compiler/rustc_feature/src/unstable.rs index 40611102250b2..b4a7ea203fda6 100644 --- a/compiler/rustc_feature/src/unstable.rs +++ b/compiler/rustc_feature/src/unstable.rs @@ -132,10 +132,12 @@ macro_rules! declare_features { // Accepted/removed features aren't in this file but are never internal // (a removed feature might have been internal, but that's now irrelevant). // Libs features are internal if they end in `_internal` or `_internals`. + // As a special exception we also consider `core_intrinsics` internal; + // renaming that age-old feature is just not worth the hassle. // We just always test the name; it's not a big deal if we accidentally hit // an accepted/removed lang feature that way. let name = feature.as_str(); - name.ends_with("_internal") || name.ends_with("_internals") + name == "core_intrinsics" || name.ends_with("_internal") || name.ends_with("_internals") } _ => panic!("`{}` was not listed in `declare_features`", feature), } diff --git a/compiler/rustc_query_system/src/lib.rs b/compiler/rustc_query_system/src/lib.rs index 2ed420f356439..1e317f9ce3336 100644 --- a/compiler/rustc_query_system/src/lib.rs +++ b/compiler/rustc_query_system/src/lib.rs @@ -3,7 +3,7 @@ #![feature(hash_raw_entry)] #![feature(min_specialization)] #![feature(let_chains)] -#![allow(rustc::potential_query_instability)] +#![allow(rustc::potential_query_instability, internal_features)] #![deny(rustc::untranslatable_diagnostic)] #![deny(rustc::diagnostic_outside_of_impl)] diff --git a/library/alloc/tests/lib.rs b/library/alloc/tests/lib.rs index 6d5c17ef02303..af825e40bafbb 100644 --- a/library/alloc/tests/lib.rs +++ b/library/alloc/tests/lib.rs @@ -40,6 +40,7 @@ #![feature(thin_box)] #![feature(strict_provenance)] #![feature(drain_keep_rest)] +#![allow(internal_features)] #![deny(fuzzy_provenance_casts)] #![deny(unsafe_op_in_unsafe_fn)] diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index f25ca9e2b1866..43e316c662b05 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -1900,6 +1900,7 @@ extern "rust-intrinsic" { /// /// ``` /// #![feature(core_intrinsics)] + /// # #![allow(internal_features)] /// /// use std::intrinsics::ctlz; /// @@ -1912,6 +1913,7 @@ extern "rust-intrinsic" { /// /// ``` /// #![feature(core_intrinsics)] + /// # #![allow(internal_features)] /// /// use std::intrinsics::ctlz; /// @@ -1933,6 +1935,7 @@ extern "rust-intrinsic" { /// /// ``` /// #![feature(core_intrinsics)] + /// # #![allow(internal_features)] /// /// use std::intrinsics::ctlz_nonzero; /// @@ -1959,6 +1962,7 @@ extern "rust-intrinsic" { /// /// ``` /// #![feature(core_intrinsics)] + /// # #![allow(internal_features)] /// /// use std::intrinsics::cttz; /// @@ -1971,6 +1975,7 @@ extern "rust-intrinsic" { /// /// ``` /// #![feature(core_intrinsics)] + /// # #![allow(internal_features)] /// /// use std::intrinsics::cttz; /// @@ -1992,6 +1997,7 @@ extern "rust-intrinsic" { /// /// ``` /// #![feature(core_intrinsics)] + /// # #![allow(internal_features)] /// /// use std::intrinsics::cttz_nonzero; /// @@ -2453,6 +2459,7 @@ extern "rust-intrinsic" { /// ```no_run /// #![feature(const_eval_select)] /// #![feature(core_intrinsics)] + /// # #![allow(internal_features)] /// use std::hint::unreachable_unchecked; /// use std::intrinsics::const_eval_select; /// diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index c167240ba2439..0f8583c443563 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -116,6 +116,7 @@ #![feature(get_many_mut)] #![feature(offset_of)] #![feature(iter_map_windows)] +#![allow(internal_features)] #![deny(unsafe_op_in_unsafe_fn)] #![deny(fuzzy_provenance_casts)] diff --git a/src/tools/rust-analyzer/crates/proc-macro-srv/src/lib.rs b/src/tools/rust-analyzer/crates/proc-macro-srv/src/lib.rs index 84bd15efb8bee..58833cb7e9264 100644 --- a/src/tools/rust-analyzer/crates/proc-macro-srv/src/lib.rs +++ b/src/tools/rust-analyzer/crates/proc-macro-srv/src/lib.rs @@ -13,7 +13,7 @@ #![cfg(feature = "sysroot-abi")] #![feature(proc_macro_internals, proc_macro_diagnostic, proc_macro_span)] #![warn(rust_2018_idioms, unused_lifetimes, semicolon_in_expressions_from_macros)] -#![allow(unreachable_pub)] +#![allow(unreachable_pub, internal_features)] extern crate proc_macro; From d627e2a4e8317fc11ccd69832d482b9ae591084a Mon Sep 17 00:00:00 2001 From: sjwang05 <63834813+sjwang05@users.noreply.github.com> Date: Sun, 3 Dec 2023 20:43:31 -0800 Subject: [PATCH 09/15] Fix parser ICE when recovering `dyn`/`impl` after `for<...>` --- compiler/rustc_parse/src/parser/ty.rs | 22 ++++++++++--------- .../parser/recover-hrtb-before-dyn-impl-kw.rs | 4 ++++ .../recover-hrtb-before-dyn-impl-kw.stderr | 8 ++++++- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs index b1a57c3dfd97c..90e4a2aac8367 100644 --- a/compiler/rustc_parse/src/parser/ty.rs +++ b/compiler/rustc_parse/src/parser/ty.rs @@ -304,23 +304,25 @@ impl<'a> Parser<'a> { if self.may_recover() && (self.eat_keyword_noexpect(kw::Impl) || self.eat_keyword_noexpect(kw::Dyn)) { - let kw = self.prev_token.ident().unwrap().0.name; + let kw = self.prev_token.ident().unwrap().0; + let removal_span = kw.span.with_hi(self.token.span.lo()); + let path = self.parse_path(PathStyle::Type)?; + let parse_plus = allow_plus == AllowPlus::Yes && self.check_plus(); + let kind = + self.parse_remaining_bounds_path(lifetime_defs, path, lo, parse_plus)?; let mut err = self.sess.create_err(errors::TransposeDynOrImpl { - span: self.prev_token.span, - kw: kw.as_str(), + span: kw.span, + kw: kw.name.as_str(), sugg: errors::TransposeDynOrImplSugg { - removal_span: self.prev_token.span.with_hi(self.token.span.lo()), + removal_span, insertion_span: for_span.shrink_to_lo(), - kw: kw.as_str(), + kw: kw.name.as_str(), }, }); - let path = self.parse_path(PathStyle::Type)?; - let parse_plus = allow_plus == AllowPlus::Yes && self.check_plus(); - let kind = - self.parse_remaining_bounds_path(lifetime_defs, path, lo, parse_plus)?; + // Take the parsed bare trait object and turn it either // into a `dyn` object or an `impl Trait`. - let kind = match (kind, kw) { + let kind = match (kind, kw.name) { (TyKind::TraitObject(bounds, _), kw::Dyn) => { TyKind::TraitObject(bounds, TraitObjectSyntax::Dyn) } diff --git a/tests/ui/parser/recover-hrtb-before-dyn-impl-kw.rs b/tests/ui/parser/recover-hrtb-before-dyn-impl-kw.rs index fe363a6887f58..b9e3c5783ebd7 100644 --- a/tests/ui/parser/recover-hrtb-before-dyn-impl-kw.rs +++ b/tests/ui/parser/recover-hrtb-before-dyn-impl-kw.rs @@ -6,4 +6,8 @@ fn test(_: &for<'a> dyn Trait) {} fn test2(_: for<'a> impl Trait) {} //~^ ERROR `for<...>` expected after `impl`, not before +// Issue #118564 +type A2 = dyn dyn>; +//~^ ERROR expected identifier, found `>` + fn main() {} diff --git a/tests/ui/parser/recover-hrtb-before-dyn-impl-kw.stderr b/tests/ui/parser/recover-hrtb-before-dyn-impl-kw.stderr index 6fc1259b91061..a012220e8c7c9 100644 --- a/tests/ui/parser/recover-hrtb-before-dyn-impl-kw.stderr +++ b/tests/ui/parser/recover-hrtb-before-dyn-impl-kw.stderr @@ -22,5 +22,11 @@ LL - fn test2(_: for<'a> impl Trait) {} LL + fn test2(_: impl for<'a> Trait) {} | -error: aborting due to 2 previous errors +error: expected identifier, found `>` + --> $DIR/recover-hrtb-before-dyn-impl-kw.rs:10:24 + | +LL | type A2 = dyn dyn>; + | ^ expected identifier + +error: aborting due to 3 previous errors From 65212a07e7ffa86024895420b7ffa66b77af6c1a Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Mon, 4 Dec 2023 14:22:05 +0000 Subject: [PATCH 10/15] Remove `#[rustc_host]`, use internal desugaring --- compiler/rustc_ast_lowering/src/item.rs | 55 ++++++------------- compiler/rustc_ast_lowering/src/lib.rs | 12 +--- compiler/rustc_feature/src/builtin_attrs.rs | 6 -- compiler/rustc_hir/src/hir.rs | 1 + compiler/rustc_hir/src/intravisit.rs | 2 +- .../rustc_hir_analysis/src/check/wfcheck.rs | 2 +- compiler/rustc_hir_analysis/src/collect.rs | 2 +- .../src/collect/generics_of.rs | 16 +++--- .../src/collect/resolve_bound_vars.rs | 2 +- compiler/rustc_hir_pretty/src/lib.rs | 2 +- compiler/rustc_lint/src/nonstandard_style.rs | 6 +- compiler/rustc_middle/src/ty/context.rs | 2 +- compiler/rustc_passes/src/lang_items.rs | 9 ++- compiler/rustc_span/src/symbol.rs | 1 - src/librustdoc/clean/mod.rs | 13 +++-- .../effects/helloworld.rs | 13 +++-- 16 files changed, 56 insertions(+), 88 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index f0f3e2c3c746a..19a4a871bf802 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -561,11 +561,10 @@ impl<'hir> LoweringContext<'_, 'hir> { .params .iter() .find(|param| { - parent_hir - .attrs - .get(param.hir_id.local_id) - .iter() - .any(|attr| attr.has_name(sym::rustc_host)) + matches!( + param.kind, + hir::GenericParamKind::Const { is_host_effect: true, .. } + ) }) .map(|param| param.def_id); } @@ -1352,27 +1351,17 @@ impl<'hir> LoweringContext<'_, 'hir> { let host_param_parts = if let Const::Yes(span) = constness && self.tcx.features().effects { - if let Some(param) = - generics.params.iter().find(|x| x.attrs.iter().any(|x| x.has_name(sym::rustc_host))) - { - // user has manually specified a `rustc_host` param, in this case, we set - // the param id so that lowering logic can use that. But we don't create - // another host param, so this gives `None`. - self.host_param_id = Some(self.local_def_id(param.id)); - None - } else { - let param_node_id = self.next_node_id(); - let hir_id = self.next_id(); - let def_id = self.create_def( - self.local_def_id(parent_node_id), - param_node_id, - sym::host, - DefKind::ConstParam, - span, - ); - self.host_param_id = Some(def_id); - Some((span, hir_id, def_id)) - } + let param_node_id = self.next_node_id(); + let hir_id = self.next_id(); + let def_id = self.create_def( + self.local_def_id(parent_node_id), + param_node_id, + sym::host, + DefKind::ConstParam, + span, + ); + self.host_param_id = Some(def_id); + Some((span, hir_id, def_id)) } else { None }; @@ -1436,19 +1425,6 @@ impl<'hir> LoweringContext<'_, 'hir> { self.children.push((def_id, hir::MaybeOwner::NonOwner(hir_id))); self.children.push((anon_const, hir::MaybeOwner::NonOwner(const_id))); - let attr_id = self.tcx.sess.parse_sess.attr_id_generator.mk_attr_id(); - - let attrs = self.arena.alloc_from_iter([Attribute { - kind: AttrKind::Normal(P(NormalAttr::from_ident(Ident::new( - sym::rustc_host, - span, - )))), - span, - id: attr_id, - style: AttrStyle::Outer, - }]); - self.attrs.insert(hir_id.local_id, attrs); - let const_body = self.lower_body(|this| { ( &[], @@ -1490,6 +1466,7 @@ impl<'hir> LoweringContext<'_, 'hir> { hir_id: const_id, body: const_body, }), + is_host_effect: true, }, colon_span: None, pure_wrt_drop: false, diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index aa8ad9784513d..9a2a4fae320ae 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -2108,7 +2108,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let default = default.as_ref().map(|def| self.lower_anon_const(def)); ( hir::ParamName::Plain(self.lower_ident(param.ident)), - hir::GenericParamKind::Const { ty, default }, + hir::GenericParamKind::Const { ty, default, is_host_effect: false }, ) } } @@ -2536,15 +2536,6 @@ impl<'hir> GenericArgsCtor<'hir> { }) }); - let attr_id = lcx.tcx.sess.parse_sess.attr_id_generator.mk_attr_id(); - let attr = lcx.arena.alloc(Attribute { - kind: AttrKind::Normal(P(NormalAttr::from_ident(Ident::new(sym::rustc_host, span)))), - span, - id: attr_id, - style: AttrStyle::Outer, - }); - lcx.attrs.insert(hir_id.local_id, std::slice::from_ref(attr)); - let def_id = lcx.create_def( lcx.current_hir_id_owner.def_id, id, @@ -2552,6 +2543,7 @@ impl<'hir> GenericArgsCtor<'hir> { DefKind::AnonConst, span, ); + lcx.children.push((def_id, hir::MaybeOwner::NonOwner(hir_id))); self.args.push(hir::GenericArg::Const(hir::ConstArg { value: hir::AnonConst { def_id, hir_id, body }, diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 9754f7acaae17..5523543cd4fb9 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -719,12 +719,6 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ and it is only intended to be used in `alloc`." ), - rustc_attr!( - rustc_host, AttributeType::Normal, template!(Word), ErrorFollowing, - "#[rustc_host] annotates const generic parameters as the `host` effect param, \ - and it is only intended for internal use and as a desugaring." - ), - BuiltinAttribute { name: sym::rustc_diagnostic_item, // FIXME: This can be `true` once we always use `tcx.is_diagnostic_item`. diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 81733d8f64e2c..76e79a883ee96 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -487,6 +487,7 @@ pub enum GenericParamKind<'hir> { ty: &'hir Ty<'hir>, /// Optional default value for the const generic param default: Option, + is_host_effect: bool, }, } diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index 963b324ca1339..9cf1db166a581 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -874,7 +874,7 @@ pub fn walk_generic_param<'v, V: Visitor<'v>>(visitor: &mut V, param: &'v Generi match param.kind { GenericParamKind::Lifetime { .. } => {} GenericParamKind::Type { ref default, .. } => walk_list!(visitor, visit_ty, default), - GenericParamKind::Const { ref ty, ref default } => { + GenericParamKind::Const { ref ty, ref default, is_host_effect: _ } => { visitor.visit_ty(ty); if let Some(ref default) = default { visitor.visit_const_param_default(param.hir_id, default); diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs index 92619ae417b31..3e805ab00b9c7 100644 --- a/compiler/rustc_hir_analysis/src/check/wfcheck.rs +++ b/compiler/rustc_hir_analysis/src/check/wfcheck.rs @@ -859,7 +859,7 @@ fn check_param_wf(tcx: TyCtxt<'_>, param: &hir::GenericParam<'_>) -> Result<(), hir::GenericParamKind::Lifetime { .. } | hir::GenericParamKind::Type { .. } => Ok(()), // Const parameters are well formed if their type is structural match. - hir::GenericParamKind::Const { ty: hir_ty, default: _ } => { + hir::GenericParamKind::Const { ty: hir_ty, default: _, is_host_effect: _ } => { let ty = tcx.type_of(param.def_id).instantiate_identity(); if tcx.features().adt_const_params { diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index 0a1b8c8eea58a..ae35d6ebd75ad 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -1383,7 +1383,7 @@ fn impl_trait_ref( let last_segment = path_segments.len() - 1; let mut args = *path_segments[last_segment].args(); let last_arg = args.args.len() - 1; - assert!(matches!(args.args[last_arg], hir::GenericArg::Const(anon_const) if tcx.has_attr(anon_const.value.def_id, sym::rustc_host))); + assert!(matches!(args.args[last_arg], hir::GenericArg::Const(anon_const) if anon_const.is_desugared_from_effects)); args.args = &args.args[..args.args.len() - 1]; path_segments[last_segment].args = Some(&args); let path = hir::Path { diff --git a/compiler/rustc_hir_analysis/src/collect/generics_of.rs b/compiler/rustc_hir_analysis/src/collect/generics_of.rs index 9fc994dfe50c4..114c5147e1193 100644 --- a/compiler/rustc_hir_analysis/src/collect/generics_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/generics_of.rs @@ -9,7 +9,7 @@ use rustc_hir::def_id::LocalDefId; use rustc_middle::ty::{self, TyCtxt}; use rustc_session::lint; use rustc_span::symbol::{kw, Symbol}; -use rustc_span::{sym, Span}; +use rustc_span::Span; pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics { use rustc_hir::*; @@ -298,13 +298,11 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics { kind, }) } - GenericParamKind::Const { default, .. } => { - let is_host_param = tcx.has_attr(param.def_id, sym::rustc_host); - + GenericParamKind::Const { ty: _, default, is_host_effect } => { if !matches!(allow_defaults, Defaults::Allowed) && default.is_some() - // `rustc_host` effect params are allowed to have defaults. - && !is_host_param + // `host` effect params are allowed to have defaults. + && !is_host_effect { tcx.sess.span_err( param.span, @@ -315,7 +313,7 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics { let index = next_index(); - if is_host_param { + if is_host_effect { if let Some(idx) = host_effect_index { bug!("parent also has host effect param? index: {idx}, def: {def_id:?}"); } @@ -330,7 +328,7 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics { pure_wrt_drop: param.pure_wrt_drop, kind: ty::GenericParamDefKind::Const { has_default: default.is_some(), - is_host_effect: is_host_param, + is_host_effect, }, }) } @@ -489,7 +487,7 @@ struct AnonConstInParamTyDetector { impl<'v> Visitor<'v> for AnonConstInParamTyDetector { fn visit_generic_param(&mut self, p: &'v hir::GenericParam<'v>) { - if let GenericParamKind::Const { ty, default: _ } = p.kind { + if let GenericParamKind::Const { ty, default: _, is_host_effect: _ } = p.kind { let prev = self.in_param_ty; self.in_param_ty = true; self.visit_ty(ty); diff --git a/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs b/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs index c6ea853b9bab6..e939b7abd9031 100644 --- a/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs +++ b/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs @@ -992,7 +992,7 @@ impl<'a, 'tcx> Visitor<'tcx> for BoundVarContext<'a, 'tcx> { self.visit_ty(ty); } } - GenericParamKind::Const { ty, default } => { + GenericParamKind::Const { ty, default, is_host_effect: _ } => { self.visit_ty(ty); if let Some(default) = default { self.visit_body(self.tcx.hir().body(default.body)); diff --git a/compiler/rustc_hir_pretty/src/lib.rs b/compiler/rustc_hir_pretty/src/lib.rs index b7e7d258a9047..1c4534287ebf3 100644 --- a/compiler/rustc_hir_pretty/src/lib.rs +++ b/compiler/rustc_hir_pretty/src/lib.rs @@ -2126,7 +2126,7 @@ impl<'a> State<'a> { self.print_type(default); } } - GenericParamKind::Const { ty, ref default } => { + GenericParamKind::Const { ty, ref default, is_host_effect: _ } => { self.word_space(":"); self.print_type(ty); if let Some(default) = default { diff --git a/compiler/rustc_lint/src/nonstandard_style.rs b/compiler/rustc_lint/src/nonstandard_style.rs index d6ed0250efc75..59f27a88aec63 100644 --- a/compiler/rustc_lint/src/nonstandard_style.rs +++ b/compiler/rustc_lint/src/nonstandard_style.rs @@ -534,9 +534,9 @@ impl<'tcx> LateLintPass<'tcx> for NonUpperCaseGlobals { } fn check_generic_param(&mut self, cx: &LateContext<'_>, param: &hir::GenericParam<'_>) { - if let GenericParamKind::Const { .. } = param.kind { - // `rustc_host` params are explicitly allowed to be lowercase. - if cx.tcx.has_attr(param.def_id, sym::rustc_host) { + if let GenericParamKind::Const { is_host_effect, .. } = param.kind { + // `host` params are explicitly allowed to be lowercase. + if is_host_effect { return; } NonUpperCaseGlobals::check_upper_case(cx, "const parameter", ¶m.name.ident()); diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 8e71327f82e53..4a01a24fd6135 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -2186,7 +2186,7 @@ impl<'tcx> TyCtxt<'tcx> { hir::Node::Item(hir::Item { kind: hir::ItemKind::Impl(hir::Impl { generics, .. }), .. - }) if generics.params.iter().any(|p| self.has_attr(p.def_id, sym::rustc_host)) + }) if generics.params.iter().any(|p| matches!(p.kind, hir::GenericParamKind::Const { is_host_effect: true, .. })) ) } diff --git a/compiler/rustc_passes/src/lang_items.rs b/compiler/rustc_passes/src/lang_items.rs index 23baf14aea1e4..d0b782ba4caca 100644 --- a/compiler/rustc_passes/src/lang_items.rs +++ b/compiler/rustc_passes/src/lang_items.rs @@ -21,7 +21,7 @@ use rustc_hir::{LangItem, LanguageItems, Target}; use rustc_middle::ty::TyCtxt; use rustc_session::cstore::ExternCrate; use rustc_span::symbol::kw::Empty; -use rustc_span::{sym, Span}; +use rustc_span::Span; use rustc_middle::query::Providers; @@ -162,7 +162,12 @@ impl<'tcx> LanguageItemCollector<'tcx> { generics .params .iter() - .filter(|p| !self.tcx.has_attr(p.def_id, sym::rustc_host)) + .filter(|p| { + !matches!( + p.kind, + hir::GenericParamKind::Const { is_host_effect: true, .. } + ) + }) .count(), generics.span, ), diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index d7e822382ef92..2809aaed43c6a 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1393,7 +1393,6 @@ symbols! { rustc_expected_cgu_reuse, rustc_has_incoherent_inherent_impls, rustc_hidden_type_of_opaques, - rustc_host, rustc_if_this_changed, rustc_inherit_overflow_checks, rustc_insignificant_dtor, diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index fe1f43835ef3a..688751627f3ee 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -595,13 +595,13 @@ fn clean_generic_param<'tcx>( }, ) } - hir::GenericParamKind::Const { ty, default } => ( + hir::GenericParamKind::Const { ty, default, is_host_effect } => ( param.name.ident().name, GenericParamDefKind::Const { ty: Box::new(clean_ty(ty, cx)), default: default .map(|ct| Box::new(ty::Const::from_anon_const(cx.tcx, ct.def_id).to_string())), - is_host_effect: cx.tcx.has_attr(param.def_id, sym::rustc_host), + is_host_effect, }, ), }; @@ -2536,11 +2536,12 @@ fn clean_generic_args<'tcx>( } hir::GenericArg::Lifetime(_) => GenericArg::Lifetime(Lifetime::elided()), hir::GenericArg::Type(ty) => GenericArg::Type(clean_ty(ty, cx)), - // Checking for `#[rustc_host]` on the `AnonConst` not only accounts for the case + // Checking for `is_desugared_from_effects` on the `AnonConst` not only accounts for the case // where the argument is `host` but for all possible cases (e.g., `true`, `false`). - hir::GenericArg::Const(ct) - if cx.tcx.has_attr(ct.value.def_id, sym::rustc_host) => - { + hir::GenericArg::Const(hir::ConstArg { + is_desugared_from_effects: true, + .. + }) => { return None; } hir::GenericArg::Const(ct) => GenericArg::Const(Box::new(clean_const(ct, cx))), diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/helloworld.rs b/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/helloworld.rs index e7ba0505d9b71..17f203e1565ec 100644 --- a/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/helloworld.rs +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/helloworld.rs @@ -3,15 +3,16 @@ // gate-test-effects // ^ effects doesn't have a gate so we will trick tidy into thinking this is a gate test -#![feature(const_trait_impl, effects, rustc_attrs)] +#![feature(const_trait_impl, effects, core_intrinsics, const_eval_select)] // ensure we are passing in the correct host effect in always const contexts. -pub const fn hmm() -> usize { - if host { - 1 - } else { - 0 +pub const fn hmm() -> usize { + // FIXME(const_trait_impl): maybe we should have a way to refer to the (hidden) effect param + fn one() -> usize { 1 } + const fn zero() -> usize { 0 } + unsafe { + std::intrinsics::const_eval_select((), zero, one) } } From a0ba895f2d6bf94da9a083eb4afce7bf6fb730a4 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 5 Dec 2023 12:11:29 +0100 Subject: [PATCH 11/15] bootstrap(builder.rs): Don't explicitly warn against `semicolon_in_expressions_from_macros` This already wasn't passed in bootstrap.py and the lint itself already warns-by-default for 2 years now and has already been added to the future-incompat group in Rust 1.68. See https://github.com/rust-lang/rust/issues/79813 for the tracking issue. --- src/bootstrap/src/core/builder.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index 65af2aed6de37..80dc5439b3bb3 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -1872,7 +1872,6 @@ impl<'a> Builder<'a> { // some code doesn't go through this `rustc` wrapper. lint_flags.push("-Wrust_2018_idioms"); lint_flags.push("-Wunused_lifetimes"); - lint_flags.push("-Wsemicolon_in_expressions_from_macros"); if self.config.deny_warnings { lint_flags.push("-Dwarnings"); From 334577f0910f64f3865f3803d124b299bf0f3fe5 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 27 Nov 2023 03:20:45 +0000 Subject: [PATCH 12/15] Add deeply_normalize_for_diagnostics, use it in coherence --- .../rustc_trait_selection/src/solve/mod.rs | 4 +++- .../src/solve/normalize.rs | 16 +++++++++++++- .../src/traits/coherence.rs | 14 ++++++++++-- .../normalize-for-errors.current.stderr | 14 ++++++++++++ .../normalize-for-errors.next.stderr | 14 ++++++++++++ tests/ui/coherence/normalize-for-errors.rs | 22 +++++++++++++++++++ 6 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 tests/ui/coherence/normalize-for-errors.current.stderr create mode 100644 tests/ui/coherence/normalize-for-errors.next.stderr create mode 100644 tests/ui/coherence/normalize-for-errors.rs diff --git a/compiler/rustc_trait_selection/src/solve/mod.rs b/compiler/rustc_trait_selection/src/solve/mod.rs index b9e256236e940..bf3b72caeb4db 100644 --- a/compiler/rustc_trait_selection/src/solve/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/mod.rs @@ -41,7 +41,9 @@ mod trait_goals; pub use eval_ctxt::{EvalCtxt, GenerateProofTree, InferCtxtEvalExt, InferCtxtSelectExt}; pub use fulfill::FulfillmentCtxt; -pub(crate) use normalize::{deeply_normalize, deeply_normalize_with_skipped_universes}; +pub(crate) use normalize::{ + deeply_normalize, deeply_normalize_for_diagnostics, deeply_normalize_with_skipped_universes, +}; #[derive(Debug, Clone, Copy)] enum SolverMode { diff --git a/compiler/rustc_trait_selection/src/solve/normalize.rs b/compiler/rustc_trait_selection/src/solve/normalize.rs index b0a348985708d..af522799db820 100644 --- a/compiler/rustc_trait_selection/src/solve/normalize.rs +++ b/compiler/rustc_trait_selection/src/solve/normalize.rs @@ -4,10 +4,11 @@ use crate::traits::{needs_normalization, BoundVarReplacer, PlaceholderReplacer}; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_infer::infer::at::At; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; +use rustc_infer::infer::InferCtxt; use rustc_infer::traits::TraitEngineExt; use rustc_infer::traits::{FulfillmentError, Obligation, TraitEngine}; use rustc_middle::infer::unify_key::{ConstVariableOrigin, ConstVariableOriginKind}; -use rustc_middle::traits::Reveal; +use rustc_middle::traits::{ObligationCause, Reveal}; use rustc_middle::ty::{self, AliasTy, Ty, TyCtxt, UniverseIndex}; use rustc_middle::ty::{FallibleTypeFolder, TypeSuperFoldable}; use rustc_middle::ty::{TypeFoldable, TypeVisitableExt}; @@ -41,6 +42,19 @@ pub(crate) fn deeply_normalize_with_skipped_universes<'tcx, T: TypeFoldable>>( + infcx: &InferCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + t: T, +) -> T { + infcx + .commit_if_ok(|_| { + deeply_normalize(infcx.at(&ObligationCause::dummy(), param_env), t.clone()) + }) + .unwrap_or(t) +} + struct NormalizationFolder<'me, 'tcx> { at: At<'me, 'tcx>, fulfill_cx: FulfillmentCtxt<'tcx>, diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 8e97333ad0f3e..9f2e1df8ef8fd 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -6,8 +6,8 @@ use crate::infer::outlives::env::OutlivesEnvironment; use crate::infer::InferOk; -use crate::solve::inspect; use crate::solve::inspect::{InspectGoal, ProofTreeInferCtxtExt, ProofTreeVisitor}; +use crate::solve::{deeply_normalize_for_diagnostics, inspect}; use crate::traits::engine::TraitEngineExt; use crate::traits::query::evaluate_obligation::InferCtxtExt; use crate::traits::select::{IntercrateAmbiguityCause, TreatInductiveCycleAs}; @@ -308,7 +308,13 @@ fn overlap<'tcx>( .iter() .any(|c| c.0.involves_placeholders()); - let impl_header = selcx.infcx.resolve_vars_if_possible(impl1_header); + let mut impl_header = infcx.resolve_vars_if_possible(impl1_header); + + // Deeply normalize the impl header for diagnostics, ignoring any errors if this fails. + if infcx.next_trait_solver() { + impl_header = deeply_normalize_for_diagnostics(&infcx, param_env, impl_header); + } + Some(OverlapResult { impl_header, intercrate_ambiguity_causes, involves_placeholder }) } @@ -1084,6 +1090,10 @@ impl<'a, 'tcx> ProofTreeVisitor<'tcx> for AmbiguityCausesVisitor<'a, 'tcx> { Ok(Ok(())) => warn!("expected an unknowable trait ref: {trait_ref:?}"), Ok(Err(conflict)) => { if !trait_ref.references_error() { + // Normalize the trait ref for diagnostics, ignoring any errors if this fails. + let trait_ref = + deeply_normalize_for_diagnostics(infcx, param_env, trait_ref); + let self_ty = trait_ref.self_ty(); let self_ty = self_ty.has_concrete_skeleton().then(|| self_ty); ambiguity_cause = Some(match conflict { diff --git a/tests/ui/coherence/normalize-for-errors.current.stderr b/tests/ui/coherence/normalize-for-errors.current.stderr new file mode 100644 index 0000000000000..df71d80e7fbed --- /dev/null +++ b/tests/ui/coherence/normalize-for-errors.current.stderr @@ -0,0 +1,14 @@ +error[E0119]: conflicting implementations of trait `MyTrait` for type `Box<(MyType,)>` + --> $DIR/normalize-for-errors.rs:17:1 + | +LL | impl MyTrait for T {} + | --------------------------- first implementation here +LL | +LL | impl MyTrait for Box<<(MyType,) as Mirror>::Assoc> {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `Box<(MyType,)>` + | + = note: upstream crates may add a new impl of trait `std::marker::Copy` for type `std::boxed::Box<(MyType,)>` in future versions + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0119`. diff --git a/tests/ui/coherence/normalize-for-errors.next.stderr b/tests/ui/coherence/normalize-for-errors.next.stderr new file mode 100644 index 0000000000000..df71d80e7fbed --- /dev/null +++ b/tests/ui/coherence/normalize-for-errors.next.stderr @@ -0,0 +1,14 @@ +error[E0119]: conflicting implementations of trait `MyTrait` for type `Box<(MyType,)>` + --> $DIR/normalize-for-errors.rs:17:1 + | +LL | impl MyTrait for T {} + | --------------------------- first implementation here +LL | +LL | impl MyTrait for Box<<(MyType,) as Mirror>::Assoc> {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `Box<(MyType,)>` + | + = note: upstream crates may add a new impl of trait `std::marker::Copy` for type `std::boxed::Box<(MyType,)>` in future versions + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0119`. diff --git a/tests/ui/coherence/normalize-for-errors.rs b/tests/ui/coherence/normalize-for-errors.rs new file mode 100644 index 0000000000000..f488ad3d46862 --- /dev/null +++ b/tests/ui/coherence/normalize-for-errors.rs @@ -0,0 +1,22 @@ +// revisions: current next +//[next] compile-flags: -Ztrait-solver=next + +struct MyType; +trait MyTrait { +} + +trait Mirror { + type Assoc; +} +impl Mirror for T { + type Assoc = T; +} + +impl MyTrait for T {} +//~^ NOTE first implementation here +impl MyTrait for Box<<(MyType,) as Mirror>::Assoc> {} +//~^ ERROR conflicting implementations of trait `MyTrait` for type `Box<(MyType,)>` +//~| NOTE conflicting implementation for `Box<(MyType,)> +//~| NOTE upstream crates may add a new impl of trait `std::marker::Copy` for type `std::boxed::Box<(MyType,)>` in future versions + +fn main() {} From 3448284f8df0f136835500d220addc1326ab98d6 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 28 Nov 2023 21:45:37 +0000 Subject: [PATCH 13/15] Continue folding if deep normalizer fails --- .../src/solve/normalize.rs | 54 ++++++++++++++----- .../normalize-for-errors.current.stderr | 12 ++--- .../normalize-for-errors.next.stderr | 12 ++--- tests/ui/coherence/normalize-for-errors.rs | 11 ++-- 4 files changed, 57 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/normalize.rs b/compiler/rustc_trait_selection/src/solve/normalize.rs index af522799db820..ea512ba5fa777 100644 --- a/compiler/rustc_trait_selection/src/solve/normalize.rs +++ b/compiler/rustc_trait_selection/src/solve/normalize.rs @@ -10,7 +10,7 @@ use rustc_infer::traits::{FulfillmentError, Obligation, TraitEngine}; use rustc_middle::infer::unify_key::{ConstVariableOrigin, ConstVariableOriginKind}; use rustc_middle::traits::{ObligationCause, Reveal}; use rustc_middle::ty::{self, AliasTy, Ty, TyCtxt, UniverseIndex}; -use rustc_middle::ty::{FallibleTypeFolder, TypeSuperFoldable}; +use rustc_middle::ty::{FallibleTypeFolder, TypeFolder, TypeSuperFoldable}; use rustc_middle::ty::{TypeFoldable, TypeVisitableExt}; use super::FulfillmentCtxt; @@ -42,19 +42,6 @@ pub(crate) fn deeply_normalize_with_skipped_universes<'tcx, T: TypeFoldable>>( - infcx: &InferCtxt<'tcx>, - param_env: ty::ParamEnv<'tcx>, - t: T, -) -> T { - infcx - .commit_if_ok(|_| { - deeply_normalize(infcx.at(&ObligationCause::dummy(), param_env), t.clone()) - }) - .unwrap_or(t) -} - struct NormalizationFolder<'me, 'tcx> { at: At<'me, 'tcx>, fulfill_cx: FulfillmentCtxt<'tcx>, @@ -244,3 +231,42 @@ impl<'tcx> FallibleTypeFolder> for NormalizationFolder<'_, 'tcx> { } } } + +// Deeply normalize a value and return it +pub(crate) fn deeply_normalize_for_diagnostics<'tcx, T: TypeFoldable>>( + infcx: &InferCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + t: T, +) -> T { + t.fold_with(&mut DeeplyNormalizeForDiagnosticsFolder { + at: infcx.at(&ObligationCause::dummy(), param_env), + }) +} + +struct DeeplyNormalizeForDiagnosticsFolder<'a, 'tcx> { + at: At<'a, 'tcx>, +} + +impl<'tcx> TypeFolder> for DeeplyNormalizeForDiagnosticsFolder<'_, 'tcx> { + fn interner(&self) -> TyCtxt<'tcx> { + self.at.infcx.tcx + } + + fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> { + deeply_normalize_with_skipped_universes( + self.at, + ty, + vec![None; ty.outer_exclusive_binder().as_usize()], + ) + .unwrap_or_else(|_| ty.super_fold_with(self)) + } + + fn fold_const(&mut self, ct: ty::Const<'tcx>) -> ty::Const<'tcx> { + deeply_normalize_with_skipped_universes( + self.at, + ct, + vec![None; ct.outer_exclusive_binder().as_usize()], + ) + .unwrap_or_else(|_| ct.super_fold_with(self)) + } +} diff --git a/tests/ui/coherence/normalize-for-errors.current.stderr b/tests/ui/coherence/normalize-for-errors.current.stderr index df71d80e7fbed..0e48aaed87991 100644 --- a/tests/ui/coherence/normalize-for-errors.current.stderr +++ b/tests/ui/coherence/normalize-for-errors.current.stderr @@ -1,11 +1,11 @@ -error[E0119]: conflicting implementations of trait `MyTrait` for type `Box<(MyType,)>` - --> $DIR/normalize-for-errors.rs:17:1 +error[E0119]: conflicting implementations of trait `MyTrait<_>` for type `(Box<(MyType,)>, _)` + --> $DIR/normalize-for-errors.rs:16:1 | -LL | impl MyTrait for T {} - | --------------------------- first implementation here +LL | impl MyTrait for (T, S::Item) {} + | ------------------------------------------------------ first implementation here LL | -LL | impl MyTrait for Box<<(MyType,) as Mirror>::Assoc> {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `Box<(MyType,)>` +LL | impl MyTrait for (Box<<(MyType,) as Mirror>::Assoc>, S::Item) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `(Box<(MyType,)>, _)` | = note: upstream crates may add a new impl of trait `std::marker::Copy` for type `std::boxed::Box<(MyType,)>` in future versions diff --git a/tests/ui/coherence/normalize-for-errors.next.stderr b/tests/ui/coherence/normalize-for-errors.next.stderr index df71d80e7fbed..a8a7d437b32d3 100644 --- a/tests/ui/coherence/normalize-for-errors.next.stderr +++ b/tests/ui/coherence/normalize-for-errors.next.stderr @@ -1,11 +1,11 @@ -error[E0119]: conflicting implementations of trait `MyTrait` for type `Box<(MyType,)>` - --> $DIR/normalize-for-errors.rs:17:1 +error[E0119]: conflicting implementations of trait `MyTrait<_>` for type `(Box<(MyType,)>, <_ as Iterator>::Item)` + --> $DIR/normalize-for-errors.rs:16:1 | -LL | impl MyTrait for T {} - | --------------------------- first implementation here +LL | impl MyTrait for (T, S::Item) {} + | ------------------------------------------------------ first implementation here LL | -LL | impl MyTrait for Box<<(MyType,) as Mirror>::Assoc> {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `Box<(MyType,)>` +LL | impl MyTrait for (Box<<(MyType,) as Mirror>::Assoc>, S::Item) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `(Box<(MyType,)>, <_ as Iterator>::Item)` | = note: upstream crates may add a new impl of trait `std::marker::Copy` for type `std::boxed::Box<(MyType,)>` in future versions diff --git a/tests/ui/coherence/normalize-for-errors.rs b/tests/ui/coherence/normalize-for-errors.rs index f488ad3d46862..367d34251ae30 100644 --- a/tests/ui/coherence/normalize-for-errors.rs +++ b/tests/ui/coherence/normalize-for-errors.rs @@ -2,8 +2,7 @@ //[next] compile-flags: -Ztrait-solver=next struct MyType; -trait MyTrait { -} +trait MyTrait {} trait Mirror { type Assoc; @@ -12,11 +11,11 @@ impl Mirror for T { type Assoc = T; } -impl MyTrait for T {} +impl MyTrait for (T, S::Item) {} //~^ NOTE first implementation here -impl MyTrait for Box<<(MyType,) as Mirror>::Assoc> {} -//~^ ERROR conflicting implementations of trait `MyTrait` for type `Box<(MyType,)>` -//~| NOTE conflicting implementation for `Box<(MyType,)> +impl MyTrait for (Box<<(MyType,) as Mirror>::Assoc>, S::Item) {} +//~^ ERROR conflicting implementations of trait `MyTrait<_>` for type `(Box<(MyType,)>, +//~| NOTE conflicting implementation for `(Box<(MyType,)>, //~| NOTE upstream crates may add a new impl of trait `std::marker::Copy` for type `std::boxed::Box<(MyType,)>` in future versions fn main() {} From b97ff8eb16c4b07dc8d24cf784b7c12bd3903e29 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 24 Nov 2023 22:09:59 +0000 Subject: [PATCH 14/15] Add print_trait_sugared --- .../rustc_hir_analysis/src/astconv/mod.rs | 5 +- .../src/astconv/object_safety.rs | 1 + .../rustc_hir_typeck/src/method/suggest.rs | 2 +- .../src/infer/error_reporting/mod.rs | 4 +- compiler/rustc_middle/src/ty/print/pretty.rs | 46 ++++++++++++++++++- .../error_reporting/on_unimplemented.rs | 9 +--- .../error_reporting/type_err_ctxt_ext.rs | 6 +-- tests/ui/error-codes/E0401.stderr | 2 +- .../normalize-under-binder/issue-71955.stderr | 16 +++---- tests/ui/lifetimes/issue-105675.stderr | 16 +++---- tests/ui/lifetimes/issue-79187-2.stderr | 4 +- tests/ui/lifetimes/issue-79187.stderr | 4 +- .../lifetime-errors/issue_74400.stderr | 4 +- .../mismatched_types/closure-mismatch.stderr | 8 ++-- ...missing-universe-cause-issue-114907.stderr | 8 ++-- .../ui/rfcs/rfc-1623-static/rfc1623-2.stderr | 8 ++-- tests/ui/traits/issue-85735.stderr | 4 +- 17 files changed, 91 insertions(+), 56 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs index fd3e6bd44e794..0f06407f44559 100644 --- a/compiler/rustc_hir_analysis/src/astconv/mod.rs +++ b/compiler/rustc_hir_analysis/src/astconv/mod.rs @@ -1182,10 +1182,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { if let Some(bound_span) = bound_span { err.span_label( bound_span, - format!( - "ambiguous `{assoc_name}` from `{}`", - bound.print_only_trait_path(), - ), + format!("ambiguous `{assoc_name}` from `{}`", bound.print_trait_sugared(),), ); if let Some(constraint) = &is_equality { where_bounds.push(format!( diff --git a/compiler/rustc_hir_analysis/src/astconv/object_safety.rs b/compiler/rustc_hir_analysis/src/astconv/object_safety.rs index 6cb38c741b755..ce5426b51427f 100644 --- a/compiler/rustc_hir_analysis/src/astconv/object_safety.rs +++ b/compiler/rustc_hir_analysis/src/astconv/object_safety.rs @@ -106,6 +106,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { trait here instead: `trait NewTrait: {} {{}}`", regular_traits .iter() + // FIXME: This should `print_sugared`, but also needs to integrate projection bounds... .map(|t| t.trait_ref().print_only_trait_path().to_string()) .collect::>() .join(" + "), diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index 858385246dd70..37b1d6e0fcf7c 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -2288,7 +2288,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ty::Adt(def, _) if def.did().is_local() => { spans.push_span_label( self.tcx.def_span(def.did()), - format!("must implement `{}`", pred.trait_ref.print_only_trait_path()), + format!("must implement `{}`", pred.trait_ref.print_trait_sugared()), ); } _ => {} diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 9cdf78484d47d..ecaf9f4e16937 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -2219,8 +2219,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { infer::ExistentialProjection(exp_found) => self.expected_found_str(exp_found), infer::PolyTraitRefs(exp_found) => { let pretty_exp_found = ty::error::ExpectedFound { - expected: exp_found.expected.print_only_trait_path(), - found: exp_found.found.print_only_trait_path(), + expected: exp_found.expected.print_trait_sugared(), + found: exp_found.found.print_trait_sugared(), }; match self.expected_found_str(pretty_exp_found) { Some((expected, found, _, _)) if expected == found => { diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs index 25423348638a9..63b706e6b3d1d 100644 --- a/compiler/rustc_middle/src/ty/print/pretty.rs +++ b/compiler/rustc_middle/src/ty/print/pretty.rs @@ -2640,6 +2640,23 @@ impl<'tcx> fmt::Debug for TraitRefPrintOnlyTraitPath<'tcx> { } } +/// Wrapper type for `ty::TraitRef` which opts-in to pretty printing only +/// the trait path, and additionally tries to "sugar" `Fn(...)` trait bounds. +#[derive(Copy, Clone, TypeFoldable, TypeVisitable, Lift)] +pub struct TraitRefPrintSugared<'tcx>(ty::TraitRef<'tcx>); + +impl<'tcx> rustc_errors::IntoDiagnosticArg for TraitRefPrintSugared<'tcx> { + fn into_diagnostic_arg(self) -> rustc_errors::DiagnosticArgValue<'static> { + self.to_string().into_diagnostic_arg() + } +} + +impl<'tcx> fmt::Debug for TraitRefPrintSugared<'tcx> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(self, f) + } +} + /// Wrapper type for `ty::TraitRef` which opts-in to pretty printing only /// the trait name. That is, it will print `Trait` instead of /// `>`. @@ -2657,6 +2674,10 @@ impl<'tcx> ty::TraitRef<'tcx> { TraitRefPrintOnlyTraitPath(self) } + pub fn print_trait_sugared(self) -> TraitRefPrintSugared<'tcx> { + TraitRefPrintSugared(self) + } + pub fn print_only_trait_name(self) -> TraitRefPrintOnlyTraitName<'tcx> { TraitRefPrintOnlyTraitName(self) } @@ -2666,6 +2687,10 @@ impl<'tcx> ty::Binder<'tcx, ty::TraitRef<'tcx>> { pub fn print_only_trait_path(self) -> ty::Binder<'tcx, TraitRefPrintOnlyTraitPath<'tcx>> { self.map_bound(|tr| tr.print_only_trait_path()) } + + pub fn print_trait_sugared(self) -> ty::Binder<'tcx, TraitRefPrintSugared<'tcx>> { + self.map_bound(|tr| tr.print_trait_sugared()) + } } #[derive(Copy, Clone, TypeFoldable, TypeVisitable, Lift)] @@ -2745,6 +2770,7 @@ forward_display_to_print! { ty::PolyExistentialTraitRef<'tcx>, ty::Binder<'tcx, ty::TraitRef<'tcx>>, ty::Binder<'tcx, TraitRefPrintOnlyTraitPath<'tcx>>, + ty::Binder<'tcx, TraitRefPrintSugared<'tcx>>, ty::Binder<'tcx, ty::FnSig<'tcx>>, ty::Binder<'tcx, ty::TraitPredicate<'tcx>>, ty::Binder<'tcx, TraitPredPrintModifiersAndPath<'tcx>>, @@ -2844,6 +2870,24 @@ define_print_and_forward_display! { p!(print_def_path(self.0.def_id, self.0.args)); } + TraitRefPrintSugared<'tcx> { + if !with_no_queries() + && let Some(kind) = cx.tcx().fn_trait_kind_from_def_id(self.0.def_id) + && let ty::Tuple(args) = self.0.args.type_at(1).kind() + { + p!(write("{}", kind.as_str()), "("); + for (i, arg) in args.iter().enumerate() { + if i > 0 { + p!(", "); + } + p!(print(arg)); + } + p!(")"); + } else { + p!(print_def_path(self.0.def_id, self.0.args)); + } + } + TraitRefPrintOnlyTraitName<'tcx> { p!(print_def_path(self.0.def_id, &[])); } @@ -2892,7 +2936,7 @@ define_print_and_forward_display! { if let ty::ImplPolarity::Negative = self.polarity { p!("!"); } - p!(print(self.trait_ref.print_only_trait_path())) + p!(print(self.trait_ref.print_trait_sugared())) } ty::ProjectionPredicate<'tcx> { diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs index fbe6e2bd5b892..c07db12b25b25 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs @@ -184,14 +184,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { flags.push((sym::cause, Some("MainFunctionType".to_string()))); } - if let Some(kind) = self.tcx.fn_trait_kind_from_def_id(trait_ref.def_id) - && let ty::Tuple(args) = trait_ref.args.type_at(1).kind() - { - let args = args.iter().map(|ty| ty.to_string()).collect::>().join(", "); - flags.push((sym::Trait, Some(format!("{}({args})", kind.as_str())))); - } else { - flags.push((sym::Trait, Some(trait_ref.print_only_trait_path().to_string()))); - } + flags.push((sym::Trait, Some(trait_ref.print_trait_sugared().to_string()))); // Add all types without trimmed paths or visible paths, ensuring they end up with // their "canonical" def path. diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index b3910a2770b3a..0c36cba35482a 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -622,7 +622,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { span.shrink_to_hi(), format!( "the trait `{}` is implemented for fn pointer `{}`, try casting using `as`", - cand.print_only_trait_path(), + cand.print_trait_sugared(), cand.self_ty(), ), format!(" as {}", cand.self_ty()), @@ -1785,7 +1785,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { ct_op: |ct| ct.normalize(self.tcx, ty::ParamEnv::empty()), }); err.highlighted_help(vec![ - (format!("the trait `{}` ", cand.print_only_trait_path()), Style::NoStyle), + (format!("the trait `{}` ", cand.print_trait_sugared()), Style::NoStyle), ("is".to_string(), Style::Highlight), (" implemented for `".to_string(), Style::NoStyle), (cand.self_ty().to_string(), Style::Highlight), @@ -1821,7 +1821,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { _ => (" implemented for `", ""), }; err.highlighted_help(vec![ - (format!("the trait `{}` ", cand.print_only_trait_path()), Style::NoStyle), + (format!("the trait `{}` ", cand.print_trait_sugared()), Style::NoStyle), ("is".to_string(), Style::Highlight), (desc.to_string(), Style::NoStyle), (cand.self_ty().to_string(), Style::Highlight), diff --git a/tests/ui/error-codes/E0401.stderr b/tests/ui/error-codes/E0401.stderr index 0a069e8d35063..d27fade487fe2 100644 --- a/tests/ui/error-codes/E0401.stderr +++ b/tests/ui/error-codes/E0401.stderr @@ -55,7 +55,7 @@ error[E0283]: type annotations needed LL | bfnr(x); | ^^^^ cannot infer type of the type parameter `W` declared on the function `bfnr` | - = note: multiple `impl`s satisfying `_: Fn<()>` found in the following crates: `alloc`, `core`: + = note: multiple `impl`s satisfying `_: Fn()` found in the following crates: `alloc`, `core`: - impl Fn for &F where A: Tuple, F: Fn, F: ?Sized; - impl Fn for Box diff --git a/tests/ui/higher-ranked/trait-bounds/normalize-under-binder/issue-71955.stderr b/tests/ui/higher-ranked/trait-bounds/normalize-under-binder/issue-71955.stderr index 4ef96cd954106..1cf364aa9f6a1 100644 --- a/tests/ui/higher-ranked/trait-bounds/normalize-under-binder/issue-71955.stderr +++ b/tests/ui/higher-ranked/trait-bounds/normalize-under-binder/issue-71955.stderr @@ -4,8 +4,8 @@ error[E0308]: mismatched types LL | foo(bar, "string", |s| s.len() == 5); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a, 'b> FnOnce<(&'a &'b str,)>` - found trait `for<'a> FnOnce<(&'a &str,)>` + = note: expected trait `for<'a, 'b> FnOnce(&'a &'b str)` + found trait `for<'a> FnOnce(&'a &str)` note: this closure does not fulfill the lifetime requirements --> $DIR/issue-71955.rs:45:24 | @@ -23,8 +23,8 @@ error[E0308]: mismatched types LL | foo(bar, "string", |s| s.len() == 5); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a, 'b> FnOnce<(&'a &'b str,)>` - found trait `for<'a> FnOnce<(&'a &str,)>` + = note: expected trait `for<'a, 'b> FnOnce(&'a &'b str)` + found trait `for<'a> FnOnce(&'a &str)` note: this closure does not fulfill the lifetime requirements --> $DIR/issue-71955.rs:45:24 | @@ -42,8 +42,8 @@ error[E0308]: mismatched types LL | foo(baz, "string", |s| s.0.len() == 5); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a, 'b> FnOnce<(&'a Wrapper<'b>,)>` - found trait `for<'a> FnOnce<(&'a Wrapper<'_>,)>` + = note: expected trait `for<'a, 'b> FnOnce(&'a Wrapper<'b>)` + found trait `for<'a> FnOnce(&'a Wrapper<'_>)` note: this closure does not fulfill the lifetime requirements --> $DIR/issue-71955.rs:48:24 | @@ -61,8 +61,8 @@ error[E0308]: mismatched types LL | foo(baz, "string", |s| s.0.len() == 5); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a, 'b> FnOnce<(&'a Wrapper<'b>,)>` - found trait `for<'a> FnOnce<(&'a Wrapper<'_>,)>` + = note: expected trait `for<'a, 'b> FnOnce(&'a Wrapper<'b>)` + found trait `for<'a> FnOnce(&'a Wrapper<'_>)` note: this closure does not fulfill the lifetime requirements --> $DIR/issue-71955.rs:48:24 | diff --git a/tests/ui/lifetimes/issue-105675.stderr b/tests/ui/lifetimes/issue-105675.stderr index 54ecd35ed6acd..f1fa5a5986060 100644 --- a/tests/ui/lifetimes/issue-105675.stderr +++ b/tests/ui/lifetimes/issue-105675.stderr @@ -4,8 +4,8 @@ error[E0308]: mismatched types LL | thing(f); | ^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32, u32)>` - found trait `for<'a> FnOnce<(&u32, &'a u32, u32)>` + = note: expected trait `for<'a, 'b> FnOnce(&'a u32, &'b u32, u32)` + found trait `for<'a> FnOnce(&u32, &'a u32, u32)` note: this closure does not fulfill the lifetime requirements --> $DIR/issue-105675.rs:4:13 | @@ -27,8 +27,8 @@ error[E0308]: mismatched types LL | thing(f); | ^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32, u32)>` - found trait `for<'a> FnOnce<(&u32, &'a u32, u32)>` + = note: expected trait `for<'a, 'b> FnOnce(&'a u32, &'b u32, u32)` + found trait `for<'a> FnOnce(&u32, &'a u32, u32)` note: this closure does not fulfill the lifetime requirements --> $DIR/issue-105675.rs:4:13 | @@ -46,8 +46,8 @@ error[E0308]: mismatched types LL | thing(f); | ^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32, u32)>` - found trait `FnOnce<(&u32, &u32, u32)>` + = note: expected trait `for<'a, 'b> FnOnce(&'a u32, &'b u32, u32)` + found trait `FnOnce(&u32, &u32, u32)` note: this closure does not fulfill the lifetime requirements --> $DIR/issue-105675.rs:8:13 | @@ -69,8 +69,8 @@ error[E0308]: mismatched types LL | thing(f); | ^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32, u32)>` - found trait `FnOnce<(&u32, &u32, u32)>` + = note: expected trait `for<'a, 'b> FnOnce(&'a u32, &'b u32, u32)` + found trait `FnOnce(&u32, &u32, u32)` note: this closure does not fulfill the lifetime requirements --> $DIR/issue-105675.rs:8:13 | diff --git a/tests/ui/lifetimes/issue-79187-2.stderr b/tests/ui/lifetimes/issue-79187-2.stderr index 75fd87b3fe9b3..86a4ac4132e44 100644 --- a/tests/ui/lifetimes/issue-79187-2.stderr +++ b/tests/ui/lifetimes/issue-79187-2.stderr @@ -31,8 +31,8 @@ error[E0308]: mismatched types LL | take_foo(|a| a); | ^^^^^^^^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a> Fn<(&'a i32,)>` - found trait `Fn<(&i32,)>` + = note: expected trait `for<'a> Fn(&'a i32)` + found trait `Fn(&i32)` note: this closure does not fulfill the lifetime requirements --> $DIR/issue-79187-2.rs:8:14 | diff --git a/tests/ui/lifetimes/issue-79187.stderr b/tests/ui/lifetimes/issue-79187.stderr index 209f2b7b7398a..14bdfe75c08be 100644 --- a/tests/ui/lifetimes/issue-79187.stderr +++ b/tests/ui/lifetimes/issue-79187.stderr @@ -4,8 +4,8 @@ error[E0308]: mismatched types LL | thing(f); | ^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a> FnOnce<(&'a u32,)>` - found trait `FnOnce<(&u32,)>` + = note: expected trait `for<'a> FnOnce(&'a u32)` + found trait `FnOnce(&u32)` note: this closure does not fulfill the lifetime requirements --> $DIR/issue-79187.rs:4:13 | diff --git a/tests/ui/lifetimes/lifetime-errors/issue_74400.stderr b/tests/ui/lifetimes/lifetime-errors/issue_74400.stderr index dbc587dd004a4..677f918fe932e 100644 --- a/tests/ui/lifetimes/lifetime-errors/issue_74400.stderr +++ b/tests/ui/lifetimes/lifetime-errors/issue_74400.stderr @@ -18,8 +18,8 @@ error[E0308]: mismatched types LL | f(data, identity) | ^^^^^^^^^^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a> Fn<(&'a T,)>` - found trait `Fn<(&T,)>` + = note: expected trait `for<'a> Fn(&'a T)` + found trait `Fn(&T)` note: the lifetime requirement is introduced here --> $DIR/issue_74400.rs:8:34 | diff --git a/tests/ui/mismatched_types/closure-mismatch.stderr b/tests/ui/mismatched_types/closure-mismatch.stderr index c5b8270ba84d5..74033c1857377 100644 --- a/tests/ui/mismatched_types/closure-mismatch.stderr +++ b/tests/ui/mismatched_types/closure-mismatch.stderr @@ -13,8 +13,8 @@ error[E0308]: mismatched types LL | baz(|_| ()); | ^^^^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a> Fn<(&'a (),)>` - found trait `Fn<(&(),)>` + = note: expected trait `for<'a> Fn(&'a ())` + found trait `Fn(&())` note: this closure does not fulfill the lifetime requirements --> $DIR/closure-mismatch.rs:8:9 | @@ -45,8 +45,8 @@ error[E0308]: mismatched types LL | baz(|x| ()); | ^^^^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a> Fn<(&'a (),)>` - found trait `Fn<(&(),)>` + = note: expected trait `for<'a> Fn(&'a ())` + found trait `Fn(&())` note: this closure does not fulfill the lifetime requirements --> $DIR/closure-mismatch.rs:11:9 | diff --git a/tests/ui/nll/missing-universe-cause-issue-114907.stderr b/tests/ui/nll/missing-universe-cause-issue-114907.stderr index 988eee61027bc..a616d29c4fea6 100644 --- a/tests/ui/nll/missing-universe-cause-issue-114907.stderr +++ b/tests/ui/nll/missing-universe-cause-issue-114907.stderr @@ -4,8 +4,8 @@ error[E0308]: mismatched types LL | accept(callback); | ^^^^^^^^^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a> FnOnce<(&'a (),)>` - found trait `FnOnce<(&(),)>` + = note: expected trait `for<'a> FnOnce(&'a ())` + found trait `FnOnce(&())` note: this closure does not fulfill the lifetime requirements --> $DIR/missing-universe-cause-issue-114907.rs:32:20 | @@ -46,8 +46,8 @@ error[E0308]: mismatched types LL | accept(callback); | ^^^^^^^^^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a> FnOnce<(&'a (),)>` - found trait `FnOnce<(&(),)>` + = note: expected trait `for<'a> FnOnce(&'a ())` + found trait `FnOnce(&())` note: this closure does not fulfill the lifetime requirements --> $DIR/missing-universe-cause-issue-114907.rs:32:20 | diff --git a/tests/ui/rfcs/rfc-1623-static/rfc1623-2.stderr b/tests/ui/rfcs/rfc-1623-static/rfc1623-2.stderr index 5f8c5dbe61938..52c700c326e30 100644 --- a/tests/ui/rfcs/rfc-1623-static/rfc1623-2.stderr +++ b/tests/ui/rfcs/rfc-1623-static/rfc1623-2.stderr @@ -4,8 +4,8 @@ error[E0308]: mismatched types LL | f: &id, | ^^^ one type is more general than the other | - = note: expected trait `for<'a, 'b> Fn<(&'a Foo<'b>,)>` - found trait `Fn<(&Foo<'_>,)>` + = note: expected trait `for<'a, 'b> Fn(&'a Foo<'b>)` + found trait `Fn(&Foo<'_>)` error[E0308]: mismatched types --> $DIR/rfc1623-2.rs:28:8 @@ -13,8 +13,8 @@ error[E0308]: mismatched types LL | f: &id, | ^^^ one type is more general than the other | - = note: expected trait `for<'a, 'b> Fn<(&'a Foo<'b>,)>` - found trait `Fn<(&Foo<'_>,)>` + = note: expected trait `for<'a, 'b> Fn(&'a Foo<'b>)` + found trait `Fn(&Foo<'_>)` = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error: implementation of `FnOnce` is not general enough diff --git a/tests/ui/traits/issue-85735.stderr b/tests/ui/traits/issue-85735.stderr index 1aba418334848..0980c5c33bdb4 100644 --- a/tests/ui/traits/issue-85735.stderr +++ b/tests/ui/traits/issue-85735.stderr @@ -1,10 +1,10 @@ -error[E0283]: type annotations needed: cannot satisfy `T: FnMut<(&'a (),)>` +error[E0283]: type annotations needed: cannot satisfy `T: FnMut(&'a ())` --> $DIR/issue-85735.rs:7:8 | LL | T: FnMut(&'a ()), | ^^^^^^^^^^^^^ | -note: multiple `impl`s or `where` clauses satisfying `T: FnMut<(&'a (),)>` found +note: multiple `impl`s or `where` clauses satisfying `T: FnMut(&'a ())` found --> $DIR/issue-85735.rs:7:8 | LL | T: FnMut(&'a ()), From f6c30b3a54630ce4fa49ad2333a82afe1a0e8453 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 24 Nov 2023 22:36:25 +0000 Subject: [PATCH 15/15] Add more --- compiler/rustc_hir_analysis/src/coherence/unsafety.rs | 8 ++++---- compiler/rustc_hir_typeck/src/method/suggest.rs | 2 +- compiler/rustc_trait_selection/src/traits/coherence.rs | 2 +- .../src/traits/error_reporting/type_err_ctxt_ext.rs | 2 +- compiler/rustc_trait_selection/src/traits/select/mod.rs | 4 ++-- .../rustc_trait_selection/src/traits/specialize/mod.rs | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/coherence/unsafety.rs b/compiler/rustc_hir_analysis/src/coherence/unsafety.rs index 42e2818b63f39..8a02bab92aaf7 100644 --- a/compiler/rustc_hir_analysis/src/coherence/unsafety.rs +++ b/compiler/rustc_hir_analysis/src/coherence/unsafety.rs @@ -23,7 +23,7 @@ pub(super) fn check_item(tcx: TyCtxt<'_>, def_id: LocalDefId) { tcx.def_span(def_id), E0199, "implementing the trait `{}` is not unsafe", - trait_ref.print_only_trait_path() + trait_ref.print_trait_sugared() ) .span_suggestion_verbose( item.span.with_hi(item.span.lo() + rustc_span::BytePos(7)), @@ -40,13 +40,13 @@ pub(super) fn check_item(tcx: TyCtxt<'_>, def_id: LocalDefId) { tcx.def_span(def_id), E0200, "the trait `{}` requires an `unsafe impl` declaration", - trait_ref.print_only_trait_path() + trait_ref.print_trait_sugared() ) .note(format!( "the trait `{}` enforces invariants that the compiler can't check. \ Review the trait documentation and make sure this implementation \ upholds those invariants before adding the `unsafe` keyword", - trait_ref.print_only_trait_path() + trait_ref.print_trait_sugared() )) .span_suggestion_verbose( item.span.shrink_to_lo(), @@ -69,7 +69,7 @@ pub(super) fn check_item(tcx: TyCtxt<'_>, def_id: LocalDefId) { "the trait `{}` enforces invariants that the compiler can't check. \ Review the trait documentation and make sure this implementation \ upholds those invariants before adding the `unsafe` keyword", - trait_ref.print_only_trait_path() + trait_ref.print_trait_sugared() )) .span_suggestion_verbose( item.span.shrink_to_lo(), diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index 37b1d6e0fcf7c..143454c71e1c4 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -2299,7 +2299,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let msg = if preds.len() == 1 { format!( "an implementation of `{}` might be missing for `{}`", - preds[0].trait_ref.print_only_trait_path(), + preds[0].trait_ref.print_trait_sugared(), preds[0].self_ty() ) } else { diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 8e97333ad0f3e..8e2ec2e46b24a 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -245,7 +245,7 @@ fn overlap<'tcx>( let trait_ref = infcx.resolve_vars_if_possible(trait_ref); format!( "of `{}` for `{}`", - trait_ref.print_only_trait_path(), + trait_ref.print_trait_sugared(), trait_ref.self_ty() ) } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 0c36cba35482a..857740130628f 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -1854,7 +1854,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { let end = if candidates.len() <= 9 { candidates.len() } else { 8 }; err.help(format!( "the following {other}types implement trait `{}`:{}{}", - trait_ref.print_only_trait_path(), + trait_ref.print_trait_sugared(), candidates[..end].join(""), if candidates.len() > 9 { format!("\nand {} others", candidates.len() - 8) diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index b26c781100a09..8e4f7d5d9655d 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -78,7 +78,7 @@ impl<'tcx> IntercrateAmbiguityCause<'tcx> { IntercrateAmbiguityCause::DownstreamCrate { trait_ref, self_ty } => { format!( "downstream crates may implement trait `{trait_desc}`{self_desc}", - trait_desc = trait_ref.print_only_trait_path(), + trait_desc = trait_ref.print_trait_sugared(), self_desc = if let Some(self_ty) = self_ty { format!(" for type `{self_ty}`") } else { @@ -90,7 +90,7 @@ impl<'tcx> IntercrateAmbiguityCause<'tcx> { format!( "upstream crates may add a new impl of trait `{trait_desc}`{self_desc} \ in future versions", - trait_desc = trait_ref.print_only_trait_path(), + trait_desc = trait_ref.print_trait_sugared(), self_desc = if let Some(self_ty) = self_ty { format!(" for type `{self_ty}`") } else { diff --git a/compiler/rustc_trait_selection/src/traits/specialize/mod.rs b/compiler/rustc_trait_selection/src/traits/specialize/mod.rs index 56f5057608f6e..39f5ff52eba69 100644 --- a/compiler/rustc_trait_selection/src/traits/specialize/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/specialize/mod.rs @@ -412,7 +412,7 @@ fn report_conflicting_impls<'tcx>( let msg = DelayDm(|| { format!( "conflicting implementations of trait `{}`{}{}", - overlap.trait_ref.print_only_trait_path(), + overlap.trait_ref.print_trait_sugared(), overlap.self_ty.map_or_else(String::new, |ty| format!(" for type `{ty}`")), match used_to_be_allowed { Some(FutureCompatOverlapErrorKind::Issue33140) => ": (E0119)",