From a30c2c26c08642dfb6b2c56121bea5ca45465c1b Mon Sep 17 00:00:00 2001 From: Ezra Shaw Date: Thu, 9 Mar 2023 20:42:45 +1300 Subject: [PATCH] feat: implement better error for manual impl of `Fn*` traits --- compiler/rustc_hir_analysis/locales/en-US.ftl | 3 + .../rustc_hir_analysis/src/astconv/errors.rs | 107 ++++++++++++------ .../src/astconv/generics.rs | 12 +- .../rustc_hir_analysis/src/astconv/mod.rs | 13 +-- compiler/rustc_hir_analysis/src/errors.rs | 14 ++- ...-gate-unboxed-closures-manual-impls.stderr | 6 + tests/ui/fn/issue-39259.rs | 13 +++ tests/ui/fn/issue-39259.stderr | 15 +++ tests/ui/lifetimes/issue-95023.stderr | 6 + tests/ui/traits/issue-87558.stderr | 6 + 10 files changed, 143 insertions(+), 52 deletions(-) create mode 100644 tests/ui/fn/issue-39259.rs create mode 100644 tests/ui/fn/issue-39259.stderr diff --git a/compiler/rustc_hir_analysis/locales/en-US.ftl b/compiler/rustc_hir_analysis/locales/en-US.ftl index 3c62529442c3f..84a6629404b6c 100644 --- a/compiler/rustc_hir_analysis/locales/en-US.ftl +++ b/compiler/rustc_hir_analysis/locales/en-US.ftl @@ -42,6 +42,9 @@ hir_analysis_assoc_type_binding_not_allowed = associated type bindings are not allowed here .label = associated type not allowed here +hir_analysis_parenthesized_fn_trait_expansion = + parenthesized trait syntax expands to `{$expanded_type}` + hir_analysis_typeof_reserved_keyword_used = `typeof` is a reserved keyword but unimplemented .suggestion = consider replacing `typeof(...)` with an actual type diff --git a/compiler/rustc_hir_analysis/src/astconv/errors.rs b/compiler/rustc_hir_analysis/src/astconv/errors.rs index c49e4d9d5818e..156334fe785b9 100644 --- a/compiler/rustc_hir_analysis/src/astconv/errors.rs +++ b/compiler/rustc_hir_analysis/src/astconv/errors.rs @@ -1,10 +1,14 @@ use crate::astconv::AstConv; -use crate::errors::{ManualImplementation, MissingTypeParams}; +use crate::errors::{ + AssocTypeBindingNotAllowed, ManualImplementation, MissingTypeParams, + ParenthesizedFnTraitExpansion, +}; use rustc_data_structures::fx::FxHashMap; use rustc_errors::{pluralize, struct_span_err, Applicability, Diagnostic, ErrorGuaranteed}; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_infer::traits::FulfillmentError; +use rustc_middle::ty::TyCtxt; use rustc_middle::ty::{self, Ty}; use rustc_session::parse::feature_err; use rustc_span::edit_distance::find_best_match_for_name; @@ -78,43 +82,10 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { // Do not suggest the other syntax if we are in trait impl: // the desugaring would contain an associated type constraint. if !is_impl { - let args = trait_segment - .args - .as_ref() - .and_then(|args| args.args.get(0)) - .and_then(|arg| match arg { - hir::GenericArg::Type(ty) => match ty.kind { - hir::TyKind::Tup(t) => t - .iter() - .map(|e| sess.source_map().span_to_snippet(e.span)) - .collect::, _>>() - .map(|a| a.join(", ")), - _ => sess.source_map().span_to_snippet(ty.span), - } - .map(|s| format!("({})", s)) - .ok(), - _ => None, - }) - .unwrap_or_else(|| "()".to_string()); - let ret = trait_segment - .args() - .bindings - .iter() - .find_map(|b| match (b.ident.name == sym::Output, &b.kind) { - (true, hir::TypeBindingKind::Equality { term }) => { - let span = match term { - hir::Term::Ty(ty) => ty.span, - hir::Term::Const(c) => self.tcx().hir().span(c.hir_id), - }; - sess.source_map().span_to_snippet(span).ok() - } - _ => None, - }) - .unwrap_or_else(|| "()".to_string()); err.span_suggestion( span, "use parenthetical notation instead", - format!("{}{} -> {}", trait_segment.ident, args, ret), + fn_trait_to_string(self.tcx(), trait_segment, true), Applicability::MaybeIncorrect, ); } @@ -629,3 +600,69 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { err.emit(); } } + +/// Emits an error regarding forbidden type binding associations +pub fn prohibit_assoc_ty_binding( + tcx: TyCtxt<'_>, + span: Span, + segment: Option<(&hir::PathSegment<'_>, Span)>, +) { + tcx.sess.emit_err(AssocTypeBindingNotAllowed { span, fn_trait_expansion: if let Some((segment, span)) = segment && segment.args().parenthesized { + Some(ParenthesizedFnTraitExpansion { span, expanded_type: fn_trait_to_string(tcx, segment, false) }) + } else { + None + }}); +} + +pub(crate) fn fn_trait_to_string( + tcx: TyCtxt<'_>, + trait_segment: &hir::PathSegment<'_>, + parenthesized: bool, +) -> String { + let args = trait_segment + .args + .as_ref() + .and_then(|args| args.args.get(0)) + .and_then(|arg| match arg { + hir::GenericArg::Type(ty) => match ty.kind { + hir::TyKind::Tup(t) => t + .iter() + .map(|e| tcx.sess.source_map().span_to_snippet(e.span)) + .collect::, _>>() + .map(|a| a.join(", ")), + _ => tcx.sess.source_map().span_to_snippet(ty.span), + } + .map(|s| { + // `s.empty()` checks to see if the type is the unit tuple, if so we don't want a comma + if parenthesized || s.is_empty() { format!("({})", s) } else { format!("({},)", s) } + }) + .ok(), + _ => None, + }) + .unwrap_or_else(|| "()".to_string()); + + let ret = trait_segment + .args() + .bindings + .iter() + .find_map(|b| match (b.ident.name == sym::Output, &b.kind) { + (true, hir::TypeBindingKind::Equality { term }) => { + let span = match term { + hir::Term::Ty(ty) => ty.span, + hir::Term::Const(c) => tcx.hir().span(c.hir_id), + }; + + (span != tcx.hir().span(trait_segment.hir_id)) + .then_some(tcx.sess.source_map().span_to_snippet(span).ok()) + .flatten() + } + _ => None, + }) + .unwrap_or_else(|| "()".to_string()); + + if parenthesized { + format!("{}{} -> {}", trait_segment.ident, args, ret) + } else { + format!("{}<{}, Output={}>", trait_segment.ident, args, ret) + } +} diff --git a/compiler/rustc_hir_analysis/src/astconv/generics.rs b/compiler/rustc_hir_analysis/src/astconv/generics.rs index 7f6518ffd7148..2f4963f6bc311 100644 --- a/compiler/rustc_hir_analysis/src/astconv/generics.rs +++ b/compiler/rustc_hir_analysis/src/astconv/generics.rs @@ -1,9 +1,8 @@ use super::IsMethodCall; use crate::astconv::{ - CreateSubstsForGenericArgsCtxt, ExplicitLateBound, GenericArgCountMismatch, - GenericArgCountResult, GenericArgPosition, + errors::prohibit_assoc_ty_binding, CreateSubstsForGenericArgsCtxt, ExplicitLateBound, + GenericArgCountMismatch, GenericArgCountResult, GenericArgPosition, }; -use crate::errors::AssocTypeBindingNotAllowed; use crate::structured_errors::{GenericArgsInfo, StructuredDiagnostic, WrongNumberOfGenericArgs}; use rustc_ast::ast::ParamKindOrd; use rustc_errors::{struct_span_err, Applicability, Diagnostic, ErrorGuaranteed, MultiSpan}; @@ -433,7 +432,7 @@ pub(crate) fn check_generic_arg_count( (gen_pos != GenericArgPosition::Type || infer_args) && !gen_args.has_lifetime_params(); if gen_pos != GenericArgPosition::Type && let Some(b) = gen_args.bindings.first() { - prohibit_assoc_ty_binding(tcx, b.span); + prohibit_assoc_ty_binding(tcx, b.span, None); } let explicit_late_bound = @@ -589,11 +588,6 @@ pub(crate) fn check_generic_arg_count( } } -/// Emits an error regarding forbidden type binding associations -pub fn prohibit_assoc_ty_binding(tcx: TyCtxt<'_>, span: Span) { - tcx.sess.emit_err(AssocTypeBindingNotAllowed { span }); -} - /// Prohibits explicit lifetime arguments if late-bound lifetime parameters /// are present. This is used both for datatypes and function calls. pub(crate) fn prohibit_explicit_late_bound_lifetimes( diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs index 899029d98e0c4..5db404608b36f 100644 --- a/compiler/rustc_hir_analysis/src/astconv/mod.rs +++ b/compiler/rustc_hir_analysis/src/astconv/mod.rs @@ -5,9 +5,8 @@ mod errors; pub mod generics; -use crate::astconv::generics::{ - check_generic_arg_count, create_substs_for_generic_args, prohibit_assoc_ty_binding, -}; +use crate::astconv::errors::prohibit_assoc_ty_binding; +use crate::astconv::generics::{check_generic_arg_count, create_substs_for_generic_args}; use crate::bounds::Bounds; use crate::collect::HirPlaceholderCollector; use crate::errors::{ @@ -295,7 +294,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { ty::BoundConstness::NotConst, ); if let Some(b) = item_segment.args().bindings.first() { - prohibit_assoc_ty_binding(self.tcx(), b.span); + prohibit_assoc_ty_binding(self.tcx(), b.span, Some((item_segment, span))); } substs @@ -631,7 +630,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { ); if let Some(b) = item_segment.args().bindings.first() { - prohibit_assoc_ty_binding(self.tcx(), b.span); + prohibit_assoc_ty_binding(self.tcx(), b.span, Some((item_segment, span))); } args @@ -825,7 +824,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { constness, ); if let Some(b) = trait_segment.args().bindings.first() { - prohibit_assoc_ty_binding(self.tcx(), b.span); + prohibit_assoc_ty_binding(self.tcx(), b.span, Some((trait_segment, span))); } self.tcx().mk_trait_ref(trait_def_id, substs) } @@ -2596,7 +2595,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { for segment in segments { // Only emit the first error to avoid overloading the user with error messages. if let Some(b) = segment.args().bindings.first() { - prohibit_assoc_ty_binding(self.tcx(), b.span); + prohibit_assoc_ty_binding(self.tcx(), b.span, None); return true; } } diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 74fec93d91e1e..bf2cadddc0d84 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -5,7 +5,7 @@ use rustc_errors::{ error_code, Applicability, DiagnosticBuilder, ErrorGuaranteed, Handler, IntoDiagnostic, MultiSpan, }; -use rustc_macros::Diagnostic; +use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_middle::ty::Ty; use rustc_span::{symbol::Ident, Span, Symbol}; @@ -129,6 +129,18 @@ pub struct AssocTypeBindingNotAllowed { #[primary_span] #[label] pub span: Span, + + #[subdiagnostic] + pub fn_trait_expansion: Option, +} + +#[derive(Subdiagnostic)] +#[help(hir_analysis_parenthesized_fn_trait_expansion)] +pub struct ParenthesizedFnTraitExpansion { + #[primary_span] + pub span: Span, + + pub expanded_type: String, } #[derive(Diagnostic)] diff --git a/tests/ui/feature-gates/feature-gate-unboxed-closures-manual-impls.stderr b/tests/ui/feature-gates/feature-gate-unboxed-closures-manual-impls.stderr index f647380ef9bc4..b1613f638d301 100644 --- a/tests/ui/feature-gates/feature-gate-unboxed-closures-manual-impls.stderr +++ b/tests/ui/feature-gates/feature-gate-unboxed-closures-manual-impls.stderr @@ -64,6 +64,12 @@ error[E0229]: associated type bindings are not allowed here | LL | impl FnOnce() for Foo1 { | ^^^^^^^^ associated type not allowed here + | +help: parenthesized trait syntax expands to `FnOnce<(), Output=()>` + --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:16:6 + | +LL | impl FnOnce() for Foo1 { + | ^^^^^^^^ error[E0658]: the precise format of `Fn`-family traits' type parameters is subject to change --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:23:6 diff --git a/tests/ui/fn/issue-39259.rs b/tests/ui/fn/issue-39259.rs new file mode 100644 index 0000000000000..5872f1007b015 --- /dev/null +++ b/tests/ui/fn/issue-39259.rs @@ -0,0 +1,13 @@ +#![feature(fn_traits)] +#![feature(unboxed_closures)] + +struct S; + +impl Fn(u32) -> u32 for S { +//~^ ERROR associated type bindings are not allowed here [E0229] + fn call(&self) -> u32 { + 5 + } +} + +fn main() {} diff --git a/tests/ui/fn/issue-39259.stderr b/tests/ui/fn/issue-39259.stderr new file mode 100644 index 0000000000000..b656b76bfe4eb --- /dev/null +++ b/tests/ui/fn/issue-39259.stderr @@ -0,0 +1,15 @@ +error[E0229]: associated type bindings are not allowed here + --> $DIR/issue-39259.rs:6:17 + | +LL | impl Fn(u32) -> u32 for S { + | ^^^ associated type not allowed here + | +help: parenthesized trait syntax expands to `Fn<(u32,), Output=u32>` + --> $DIR/issue-39259.rs:6:6 + | +LL | impl Fn(u32) -> u32 for S { + | ^^^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0229`. diff --git a/tests/ui/lifetimes/issue-95023.stderr b/tests/ui/lifetimes/issue-95023.stderr index 35c3797c77a85..5b93eff86142d 100644 --- a/tests/ui/lifetimes/issue-95023.stderr +++ b/tests/ui/lifetimes/issue-95023.stderr @@ -25,6 +25,12 @@ error[E0229]: associated type bindings are not allowed here | LL | impl Fn(&isize) for Error { | ^^^^^^^^^^ associated type not allowed here + | +help: parenthesized trait syntax expands to `Fn<(&isize,), Output=()>` + --> $DIR/issue-95023.rs:3:6 + | +LL | impl Fn(&isize) for Error { + | ^^^^^^^^^^ error[E0220]: associated type `B` not found for `Self` --> $DIR/issue-95023.rs:6:44 diff --git a/tests/ui/traits/issue-87558.stderr b/tests/ui/traits/issue-87558.stderr index 494274d8c3075..b647f9794bd95 100644 --- a/tests/ui/traits/issue-87558.stderr +++ b/tests/ui/traits/issue-87558.stderr @@ -17,6 +17,12 @@ error[E0229]: associated type bindings are not allowed here | LL | impl Fn(&isize) for Error { | ^^^^^^^^^^ associated type not allowed here + | +help: parenthesized trait syntax expands to `Fn<(&isize,), Output=()>` + --> $DIR/issue-87558.rs:3:6 + | +LL | impl Fn(&isize) for Error { + | ^^^^^^^^^^ error: aborting due to 3 previous errors