Address comments

This commit is contained in:
Michael Goulet 2022-06-06 21:01:06 -07:00
parent 38d7e2734f
commit 5f7474e6dc
8 changed files with 131 additions and 85 deletions

View File

@ -1378,7 +1378,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
let mut params: SmallVec<[hir::GenericParam<'hir>; 4]> =
self.lower_generic_params_mut(&generics.params).collect();
let has_where_clause_predicates = !generics.where_clause.predicates.is_empty();
let has_where_clause_token = generics.where_clause.has_where_token;
let where_clause_span = self.lower_span(generics.where_clause.span);
let span = self.lower_span(generics.span);
let res = f(self);
@ -1397,7 +1396,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
params: self.arena.alloc_from_iter(params),
predicates: self.arena.alloc_from_iter(predicates),
has_where_clause_predicates,
has_where_clause_token,
where_clause_span,
span,
});

View File

@ -1316,7 +1316,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
params: lifetime_defs,
predicates: &[],
has_where_clause_predicates: false,
has_where_clause_token: false,
where_clause_span: lctx.lower_span(span),
span: lctx.lower_span(span),
}),
@ -1639,7 +1638,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
params: generic_params,
predicates: &[],
has_where_clause_predicates: false,
has_where_clause_token: false,
where_clause_span: this.lower_span(span),
span: this.lower_span(span),
}),

View File

@ -536,7 +536,6 @@ pub struct Generics<'hir> {
pub params: &'hir [GenericParam<'hir>],
pub predicates: &'hir [WherePredicate<'hir>],
pub has_where_clause_predicates: bool,
pub has_where_clause_token: bool,
pub where_clause_span: Span,
pub span: Span,
}
@ -547,7 +546,6 @@ impl<'hir> Generics<'hir> {
params: &[],
predicates: &[],
has_where_clause_predicates: false,
has_where_clause_token: false,
where_clause_span: DUMMY_SP,
span: DUMMY_SP,
};
@ -583,10 +581,6 @@ impl<'hir> Generics<'hir> {
}
}
pub fn where_clause_span(&self) -> Option<Span> {
if self.predicates.is_empty() { None } else { Some(self.where_clause_span) }
}
/// `Span` where further predicates would be suggested, accounting for trailing commas, like
/// in `fn foo<T>(t: T) where T: Foo,` so we don't suggest two trailing commas.
pub fn tail_span_for_predicate_suggestion(&self) -> Span {
@ -607,10 +601,11 @@ impl<'hir> Generics<'hir> {
pub fn add_where_or_trailing_comma(&self) -> &'static str {
if self.has_where_clause_predicates {
","
} else if self.has_where_clause_token {
""
} else {
} else if self.where_clause_span.is_empty() {
" where"
} else {
// No where clause predicates, but we have `where` token
""
}
}

View File

@ -2295,9 +2295,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements {
// (including the `where`)
if hir_generics.has_where_clause_predicates && dropped_predicate_count == num_predicates
{
let where_span = hir_generics
.where_clause_span()
.expect("span of (nonempty) where clause should exist");
let where_span = hir_generics.where_clause_span;
// Extend the where clause back to the closing `>` of the
// generics, except for tuple struct, which have the `where`
// after the fields of the struct.

View File

@ -76,9 +76,13 @@ impl<'tcx> Ty<'tcx> {
}
pub trait IsSuggestable<'tcx> {
/// Whether this makes sense to suggest in a diagnostic.
///
/// We filter out certain types and constants since they don't provide
/// meaningful rendered suggestions when pretty-printed. We leave some
/// nonsense, such as region vars, since those render as `'_` and are
/// usually okay to reinterpret as elided lifetimes.
fn is_suggestable(self, tcx: TyCtxt<'tcx>) -> bool;
fn is_suggestable_modulo_impl_trait(self, tcx: TyCtxt<'tcx>, bound_str: &str) -> bool;
}
impl<'tcx, T> IsSuggestable<'tcx> for T
@ -86,11 +90,7 @@ where
T: TypeFoldable<'tcx>,
{
fn is_suggestable(self, tcx: TyCtxt<'tcx>) -> bool {
self.visit_with(&mut IsSuggestableVisitor { tcx, bound_str: None }).is_continue()
}
fn is_suggestable_modulo_impl_trait(self, tcx: TyCtxt<'tcx>, bound_str: &str) -> bool {
self.visit_with(&mut IsSuggestableVisitor { tcx, bound_str: Some(bound_str) }).is_continue()
self.visit_with(&mut IsSuggestableVisitor { tcx }).is_continue()
}
}
@ -119,7 +119,7 @@ pub fn suggest_arbitrary_trait_bound<'tcx>(
&format!(
"consider {} `where` clause, but there might be an alternative better way to express \
this requirement",
if generics.has_where_clause_token { "extending the" } else { "introducing a" },
if generics.where_clause_span.is_empty() { "introducing a" } else { "extending the" },
),
format!("{} {}: {}", generics.add_where_or_trailing_comma(), param_name, constraint),
Applicability::MaybeIncorrect,
@ -417,12 +417,11 @@ impl<'v> hir::intravisit::Visitor<'v> for StaticLifetimeVisitor<'v> {
}
}
pub struct IsSuggestableVisitor<'tcx, 's> {
pub struct IsSuggestableVisitor<'tcx> {
tcx: TyCtxt<'tcx>,
bound_str: Option<&'s str>,
}
impl<'tcx> TypeVisitor<'tcx> for IsSuggestableVisitor<'tcx, '_> {
impl<'tcx> TypeVisitor<'tcx> for IsSuggestableVisitor<'tcx> {
type BreakTy = ();
fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
@ -462,12 +461,13 @@ impl<'tcx> TypeVisitor<'tcx> for IsSuggestableVisitor<'tcx, '_> {
}
Param(param) => {
if let Some(found_bound_str) =
param.name.as_str().strip_prefix("impl ").map(|s| s.trim_start())
{
if self.bound_str.map_or(true, |bound_str| bound_str != found_bound_str) {
return ControlFlow::Break(());
}
// FIXME: It would be nice to make this not use string manipulation,
// but it's pretty hard to do this, since `ty::ParamTy` is missing
// sufficient info to determine if it is synthetic, and we don't
// always have a convenient way of getting `ty::Generics` at the call
// sites we invoke `IsSuggestable::is_suggestable`.
if param.name.as_str().starts_with("impl ") {
return ControlFlow::Break(());
}
}

View File

@ -39,6 +39,13 @@ impl GenericParamDefKind {
GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => true,
}
}
pub fn is_synthetic(&self) -> bool {
match self {
GenericParamDefKind::Type { synthetic, .. } => *synthetic,
_ => false,
}
}
}
#[derive(Clone, Debug, TyEncodable, TyDecodable, HashStable)]

View File

@ -7,6 +7,7 @@ use crate::autoderef::Autoderef;
use crate::infer::InferCtxt;
use crate::traits::normalize_to;
use hir::HirId;
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_errors::{
@ -23,7 +24,7 @@ use rustc_middle::hir::map;
use rustc_middle::ty::{
self, suggest_arbitrary_trait_bound, suggest_constraining_type_param, AdtKind, DefIdTree,
GeneratorDiagnosticData, GeneratorInteriorTypeCause, Infer, InferTy, IsSuggestable,
ToPredicate, Ty, TyCtxt, TypeFoldable,
ToPredicate, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable,
};
use rustc_middle::ty::{TypeAndMut, TypeckResults};
use rustc_session::Limit;
@ -331,7 +332,8 @@ fn predicate_constraint(generics: &hir::Generics<'_>, pred: String) -> (Span, St
/// param for cleaner code.
fn suggest_restriction<'tcx>(
tcx: TyCtxt<'tcx>,
generics: &hir::Generics<'tcx>,
hir_id: HirId,
hir_generics: &hir::Generics<'tcx>,
msg: &str,
err: &mut Diagnostic,
fn_sig: Option<&hir::FnSig<'_>>,
@ -344,24 +346,37 @@ fn suggest_restriction<'tcx>(
// &Ident
super_traits: Option<(&Ident, &hir::GenericBounds<'_>)>,
) {
if generics.where_clause_span.from_expansion()
|| generics.where_clause_span.desugaring_kind().is_some()
if hir_generics.where_clause_span.from_expansion()
|| hir_generics.where_clause_span.desugaring_kind().is_some()
{
return;
}
let Some(item_id) = hir_id.as_owner() else { return; };
let generics = tcx.generics_of(item_id);
// Given `fn foo(t: impl Trait)` where `Trait` requires assoc type `A`...
if let Some((bound_str, fn_sig)) =
if let Some((param, bound_str, fn_sig)) =
fn_sig.zip(projection).and_then(|(sig, p)| match p.self_ty().kind() {
// Shenanigans to get the `Trait` from the `impl Trait`.
ty::Param(param) => {
// `fn foo(t: impl Trait)`
// ^^^^^ get this string
param.name.as_str().strip_prefix("impl ").map(|s| (s.trim_start().to_string(), sig))
let param_def = generics.type_param(param, tcx);
if param_def.kind.is_synthetic() {
let bound_str =
param_def.name.as_str().strip_prefix("impl ")?.trim_start().to_string();
return Some((param_def, bound_str, sig));
}
None
}
_ => None,
})
{
if !trait_pred.is_suggestable_modulo_impl_trait(tcx, &bound_str) {
let type_param_name = hir_generics.params.next_type_param_name(Some(&bound_str));
let trait_pred = trait_pred.fold_with(&mut ReplaceImplTraitFolder {
tcx,
param,
replace_ty: ty::ParamTy::new(generics.count() as u32, Symbol::intern(&type_param_name))
.to_ty(tcx),
});
if !trait_pred.is_suggestable(tcx) {
return;
}
// We know we have an `impl Trait` that doesn't satisfy a required projection.
@ -373,52 +388,21 @@ fn suggest_restriction<'tcx>(
// where `T: Trait`.
let mut ty_spans = vec![];
for input in fn_sig.decl.inputs {
struct ReplaceImplTraitVisitor<'a> {
ty_spans: &'a mut Vec<Span>,
bound_str: &'a str,
}
impl<'a, 'hir> hir::intravisit::Visitor<'hir> for ReplaceImplTraitVisitor<'a> {
fn visit_ty(&mut self, t: &'hir hir::Ty<'hir>) {
if let hir::TyKind::Path(hir::QPath::Resolved(
None,
hir::Path { segments: [segment], .. },
)) = t.kind
{
if segment.ident.as_str().strip_prefix("impl ").map(|s| s.trim_start())
== Some(self.bound_str)
{
// `fn foo(t: impl Trait)`
// ^^^^^^^^^^ get this to suggest `T` instead
// There might be more than one `impl Trait`.
self.ty_spans.push(t.span);
return;
}
}
hir::intravisit::walk_ty(self, t);
}
}
ReplaceImplTraitVisitor { ty_spans: &mut ty_spans, bound_str: &bound_str }
ReplaceImplTraitVisitor { ty_spans: &mut ty_spans, param_did: param.def_id }
.visit_ty(input);
}
let type_param_name = generics.params.next_type_param_name(Some(&bound_str));
// The type param `T: Trait` we will suggest to introduce.
let type_param = format!("{}: {}", type_param_name, bound_str);
// FIXME: modify the `trait_pred` instead of string shenanigans.
// Turn `<impl Trait as Foo>::Bar: Qux` into `<T as Foo>::Bar: Qux`.
let pred = trait_pred.to_predicate(tcx).to_string();
let pred = pred.replace(&format!("impl {}", bound_str), &type_param_name);
let mut sugg = vec![
if let Some(span) = generics.span_for_param_suggestion() {
if let Some(span) = hir_generics.span_for_param_suggestion() {
(span, format!(", {}", type_param))
} else {
(generics.span, format!("<{}>", type_param))
(hir_generics.span, format!("<{}>", type_param))
},
// `fn foo(t: impl Trait)`
// ^ suggest `where <T as Trait>::A: Bound`
predicate_constraint(generics, pred),
predicate_constraint(hir_generics, trait_pred.to_predicate(tcx).to_string()),
];
sugg.extend(ty_spans.into_iter().map(|s| (s, type_param_name.to_string())));
@ -436,13 +420,15 @@ fn suggest_restriction<'tcx>(
}
// Trivial case: `T` needs an extra bound: `T: Bound`.
let (sp, suggestion) = match (
generics
hir_generics
.params
.iter()
.find(|p| !matches!(p.kind, hir::GenericParamKind::Type { synthetic: true, .. })),
super_traits,
) {
(_, None) => predicate_constraint(generics, trait_pred.to_predicate(tcx).to_string()),
(_, None) => {
predicate_constraint(hir_generics, trait_pred.to_predicate(tcx).to_string())
}
(None, Some((ident, []))) => (
ident.span.shrink_to_hi(),
format!(": {}", trait_pred.print_modifiers_and_trait_path()),
@ -452,7 +438,7 @@ fn suggest_restriction<'tcx>(
format!(" + {}", trait_pred.print_modifiers_and_trait_path()),
),
(Some(_), Some((_, []))) => (
generics.span.shrink_to_hi(),
hir_generics.span.shrink_to_hi(),
format!(": {}", trait_pred.print_modifiers_and_trait_path()),
),
};
@ -496,6 +482,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
// Restricting `Self` for a single method.
suggest_restriction(
self.tcx,
hir_id,
&generics,
"`Self`",
err,
@ -515,7 +502,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
assert!(param_ty);
// Restricting `Self` for a single method.
suggest_restriction(
self.tcx, &generics, "`Self`", err, None, projection, trait_pred, None,
self.tcx, hir_id, &generics, "`Self`", err, None, projection, trait_pred,
None,
);
return;
}
@ -536,6 +524,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
// Missing restriction on associated type of type parameter (unmet projection).
suggest_restriction(
self.tcx,
hir_id,
&generics,
"the associated type",
err,
@ -555,6 +544,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
// Missing restriction on associated type of type parameter (unmet projection).
suggest_restriction(
self.tcx,
hir_id,
&generics,
"the associated type",
err,
@ -583,6 +573,14 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
| hir::Node::ImplItem(hir::ImplItem { generics, .. })
if param_ty =>
{
// We skip the 0'th subst (self) because we do not want
// to consider the predicate as not suggestible if the
// self type is an arg position `impl Trait` -- instead,
// we handle that by adding ` + Bound` below.
// FIXME(compiler-errors): It would be nice to do the same
// this that we do in `suggest_restriction` and pull the
// `impl Trait` into a new generic if it shows up somewhere
// else in the predicate.
if !trait_pred.skip_binder().trait_ref.substs[1..]
.iter()
.all(|g| g.is_suggestable(self.tcx))
@ -2994,3 +2992,52 @@ fn suggest_trait_object_return_type_alternatives(
);
}
}
/// Collect the spans that we see the generic param `param_did`
struct ReplaceImplTraitVisitor<'a> {
ty_spans: &'a mut Vec<Span>,
param_did: DefId,
}
impl<'a, 'hir> hir::intravisit::Visitor<'hir> for ReplaceImplTraitVisitor<'a> {
fn visit_ty(&mut self, t: &'hir hir::Ty<'hir>) {
if let hir::TyKind::Path(hir::QPath::Resolved(
None,
hir::Path { res: hir::def::Res::Def(_, segment_did), .. },
)) = t.kind
{
if self.param_did == *segment_did {
// `fn foo(t: impl Trait)`
// ^^^^^^^^^^ get this to suggest `T` instead
// There might be more than one `impl Trait`.
self.ty_spans.push(t.span);
return;
}
}
hir::intravisit::walk_ty(self, t);
}
}
// Replace `param` with `replace_ty`
struct ReplaceImplTraitFolder<'tcx> {
tcx: TyCtxt<'tcx>,
param: &'tcx ty::GenericParamDef,
replace_ty: Ty<'tcx>,
}
impl<'tcx> TypeFolder<'tcx> for ReplaceImplTraitFolder<'tcx> {
fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
if let ty::Param(ty::ParamTy { index, .. }) = t.kind() {
if self.param.index == *index {
return self.replace_ty;
}
}
t.super_fold_with(self)
}
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx
}
}

View File

@ -211,7 +211,7 @@ fn check_main_fn_ty(tcx: TyCtxt<'_>, main_def_id: DefId) {
let hir_id = tcx.hir().local_def_id_to_hir_id(def_id.expect_local());
match tcx.hir().find(hir_id) {
Some(Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, ref generics, _), .. })) => {
generics.where_clause_span()
Some(generics.where_clause_span)
}
_ => {
span_bug!(tcx.def_span(def_id), "main has a non-function type");
@ -401,14 +401,17 @@ fn check_start_fn_ty(tcx: TyCtxt<'_>, start_def_id: DefId) {
.emit();
error = true;
}
if let Some(sp) = generics.where_clause_span() {
if generics.has_where_clause_predicates {
struct_span_err!(
tcx.sess,
sp,
generics.where_clause_span,
E0647,
"start function is not allowed to have a `where` clause"
)
.span_label(sp, "start function cannot have a `where` clause")
.span_label(
generics.where_clause_span,
"start function cannot have a `where` clause",
)
.emit();
error = true;
}