Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
compiler-errors committed Jun 7, 2022
1 parent d69aed1 commit 91b9988
Show file tree
Hide file tree
Showing 8 changed files with 131 additions and 85 deletions.
2 changes: 0 additions & 2 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1377,7 +1377,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
let mut params: SmallVec<[hir::GenericParam<'hir>; 4]> =
self.lower_generic_params_mut(&generics.params).collect();
let has_where_clause_predicates = !generics.where_clause.predicates.is_empty();
let has_where_clause_token = generics.where_clause.has_where_token;
let where_clause_span = self.lower_span(generics.where_clause.span);
let span = self.lower_span(generics.span);
let res = f(self);
Expand All @@ -1396,7 +1395,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
params: self.arena.alloc_from_iter(params),
predicates: self.arena.alloc_from_iter(predicates),
has_where_clause_predicates,
has_where_clause_token,
where_clause_span,
span,
});
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1333,7 +1333,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
params: lifetime_defs,
predicates: &[],
has_where_clause_predicates: false,
has_where_clause_token: false,
where_clause_span: lctx.lower_span(span),
span: lctx.lower_span(span),
}),
Expand Down Expand Up @@ -1656,7 +1655,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
params: generic_params,
predicates: &[],
has_where_clause_predicates: false,
has_where_clause_token: false,
where_clause_span: this.lower_span(span),
span: this.lower_span(span),
}),
Expand Down
13 changes: 4 additions & 9 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -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,
};
Expand Down Expand Up @@ -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 {
Expand All @@ -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
""
}
}

Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2266,9 +2266,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements {
// (including the `where`)
if hir_generics.has_where_clause_predicates && dropped_predicate_count == num_predicates
{
let where_span = hir_generics
.where_clause_span()
.expect("span of (nonempty) where clause should exist");
let where_span = hir_generics.where_clause_span;
// Extend the where clause back to the closing `>` of the
// generics, except for tuple struct, which have the `where`
// after the fields of the struct.
Expand Down
34 changes: 17 additions & 17 deletions compiler/rustc_middle/src/ty/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,21 +76,21 @@ 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
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()
}
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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(());
}
}

Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_middle/src/ty/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
Loading

0 comments on commit 91b9988

Please sign in to comment.