Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit fc36b0d

Browse files
authoredJun 11, 2022
Rollup merge of rust-lang#97778 - compiler-errors:misc-diagnostics-tidy, r=cjgillot
Tidy up miscellaneous bounds suggestions Just some small fixes to suggestions - Generalizes `Ty::is_suggestable` into a `TypeVisitor`, so that it can be called on things other than `Ty` - Makes `impl Trait` in arg position no longer suggestible (generalizing the fix in rust-lang#97640) - Fixes `impl Trait` not being replaced with fresh type param when it's deeply nested in function signature (fixes rust-lang#97760) - Fixes some poor handling of `where` clauses with no predicates (also rust-lang#97760) - Uses `InferCtxt::resolve_numeric_literals_with_default` so we suggest `i32` instead of `{integer}` (fixes rust-lang#97677) Sorry there aren't many tests the fixes. Most of them would just be duplicates of other tests with empty `where` clauses or `impl Trait` in arg position instead of generic params. Let me know if you'd want more test coverage.
2 parents d6e2d6d + 91b9988 commit fc36b0d

33 files changed

+363
-211
lines changed
 

‎compiler/rustc_ast_lowering/src/item.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1377,7 +1377,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
13771377

13781378
let mut params: SmallVec<[hir::GenericParam<'hir>; 4]> =
13791379
self.lower_generic_params_mut(&generics.params).collect();
1380-
let has_where_clause = !generics.where_clause.predicates.is_empty();
1380+
let has_where_clause_predicates = !generics.where_clause.predicates.is_empty();
13811381
let where_clause_span = self.lower_span(generics.where_clause.span);
13821382
let span = self.lower_span(generics.span);
13831383
let res = f(self);
@@ -1395,7 +1395,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
13951395
let lowered_generics = self.arena.alloc(hir::Generics {
13961396
params: self.arena.alloc_from_iter(params),
13971397
predicates: self.arena.alloc_from_iter(predicates),
1398-
has_where_clause,
1398+
has_where_clause_predicates,
13991399
where_clause_span,
14001400
span,
14011401
});

‎compiler/rustc_ast_lowering/src/lib.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1315,7 +1315,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
13151315
generics: self.arena.alloc(hir::Generics {
13161316
params: lifetime_defs,
13171317
predicates: &[],
1318-
has_where_clause: false,
1318+
has_where_clause_predicates: false,
13191319
where_clause_span: lctx.lower_span(span),
13201320
span: lctx.lower_span(span),
13211321
}),
@@ -1637,7 +1637,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
16371637
generics: this.arena.alloc(hir::Generics {
16381638
params: generic_params,
16391639
predicates: &[],
1640-
has_where_clause: false,
1640+
has_where_clause_predicates: false,
16411641
where_clause_span: this.lower_span(span),
16421642
span: this.lower_span(span),
16431643
}),

‎compiler/rustc_hir/src/hir.rs

+15-14
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,7 @@ pub struct GenericParamCount {
535535
pub struct Generics<'hir> {
536536
pub params: &'hir [GenericParam<'hir>],
537537
pub predicates: &'hir [WherePredicate<'hir>],
538-
pub has_where_clause: bool,
538+
pub has_where_clause_predicates: bool,
539539
pub where_clause_span: Span,
540540
pub span: Span,
541541
}
@@ -545,7 +545,7 @@ impl<'hir> Generics<'hir> {
545545
const NOPE: Generics<'_> = Generics {
546546
params: &[],
547547
predicates: &[],
548-
has_where_clause: false,
548+
has_where_clause_predicates: false,
549549
where_clause_span: DUMMY_SP,
550550
span: DUMMY_SP,
551551
};
@@ -581,21 +581,11 @@ impl<'hir> Generics<'hir> {
581581
}
582582
}
583583

584-
pub fn where_clause_span(&self) -> Option<Span> {
585-
if self.predicates.is_empty() { None } else { Some(self.where_clause_span) }
586-
}
587-
588-
/// The `where_span` under normal circumstances points at either the predicates or the empty
589-
/// space where the `where` clause should be. Only of use for diagnostic suggestions.
590-
pub fn span_for_predicates_or_empty_place(&self) -> Span {
591-
self.where_clause_span
592-
}
593-
594584
/// `Span` where further predicates would be suggested, accounting for trailing commas, like
595585
/// in `fn foo<T>(t: T) where T: Foo,` so we don't suggest two trailing commas.
596586
pub fn tail_span_for_predicate_suggestion(&self) -> Span {
597-
let end = self.span_for_predicates_or_empty_place().shrink_to_hi();
598-
if self.has_where_clause {
587+
let end = self.where_clause_span.shrink_to_hi();
588+
if self.has_where_clause_predicates {
599589
self.predicates
600590
.iter()
601591
.filter(|p| p.in_where_clause())
@@ -608,6 +598,17 @@ impl<'hir> Generics<'hir> {
608598
}
609599
}
610600

601+
pub fn add_where_or_trailing_comma(&self) -> &'static str {
602+
if self.has_where_clause_predicates {
603+
","
604+
} else if self.where_clause_span.is_empty() {
605+
" where"
606+
} else {
607+
// No where clause predicates, but we have `where` token
608+
""
609+
}
610+
}
611+
611612
pub fn bounds_for_param(
612613
&self,
613614
param_def_id: LocalDefId,

‎compiler/rustc_infer/src/infer/error_reporting/mod.rs

+1-5
Original file line numberDiff line numberDiff line change
@@ -2509,11 +2509,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
25092509
labeled_user_string
25102510
);
25112511
let pred = format!("{}: {}", bound_kind, sub);
2512-
let suggestion = format!(
2513-
"{} {}",
2514-
if !generics.predicates.is_empty() { "," } else { " where" },
2515-
pred,
2516-
);
2512+
let suggestion = format!("{} {}", generics.add_where_or_trailing_comma(), pred,);
25172513
err.span_suggestion(
25182514
generics.tail_span_for_predicate_suggestion(),
25192515
"consider adding a where clause",

‎compiler/rustc_infer/src/infer/error_reporting/note.rs

+3-8
Original file line numberDiff line numberDiff line change
@@ -367,17 +367,12 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
367367
.collect();
368368

369369
if !clauses.is_empty() {
370-
let where_clause_span = self
371-
.tcx
372-
.hir()
373-
.get_generics(impl_item_def_id)
374-
.unwrap()
375-
.where_clause_span
376-
.shrink_to_hi();
370+
let generics = self.tcx.hir().get_generics(impl_item_def_id).unwrap();
371+
let where_clause_span = generics.tail_span_for_predicate_suggestion();
377372

378373
let suggestion = format!(
379374
"{} {}",
380-
if !impl_predicates.is_empty() { "," } else { " where" },
375+
generics.add_where_or_trailing_comma(),
381376
clauses.join(", "),
382377
);
383378
err.span_suggestion(

‎compiler/rustc_lint/src/builtin.rs

+3-4
Original file line numberDiff line numberDiff line change
@@ -2293,10 +2293,9 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements {
22932293

22942294
// If all predicates are inferable, drop the entire clause
22952295
// (including the `where`)
2296-
if hir_generics.has_where_clause && dropped_predicate_count == num_predicates {
2297-
let where_span = hir_generics
2298-
.where_clause_span()
2299-
.expect("span of (nonempty) where clause should exist");
2296+
if hir_generics.has_where_clause_predicates && dropped_predicate_count == num_predicates
2297+
{
2298+
let where_span = hir_generics.where_clause_span;
23002299
// Extend the where clause back to the closing `>` of the
23012300
// generics, except for tuple struct, which have the `where`
23022301
// after the fields of the struct.

‎compiler/rustc_middle/src/ty/diagnostics.rs

+113-85
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
//! Diagnostics related methods for `Ty`.
22
3-
use crate::ty::subst::{GenericArg, GenericArgKind};
3+
use std::ops::ControlFlow;
4+
45
use crate::ty::{
5-
ConstKind, DefIdTree, ExistentialPredicate, ExistentialProjection, ExistentialTraitRef,
6-
InferTy, ProjectionTy, Term, Ty, TyCtxt, TypeAndMut,
6+
fold::TypeFoldable, Const, ConstKind, DefIdTree, ExistentialPredicate, InferTy,
7+
PolyTraitPredicate, Ty, TyCtxt, TypeVisitor,
78
};
89

910
use rustc_data_structures::fx::FxHashMap;
@@ -72,103 +73,55 @@ impl<'tcx> Ty<'tcx> {
7273
_ => self.is_simple_ty(),
7374
}
7475
}
76+
}
7577

76-
/// Whether the type can be safely suggested during error recovery.
77-
pub fn is_suggestable(self, tcx: TyCtxt<'tcx>) -> bool {
78-
fn generic_arg_is_suggestible<'tcx>(arg: GenericArg<'tcx>, tcx: TyCtxt<'tcx>) -> bool {
79-
match arg.unpack() {
80-
GenericArgKind::Type(ty) => ty.is_suggestable(tcx),
81-
GenericArgKind::Const(c) => const_is_suggestable(c.val()),
82-
_ => true,
83-
}
84-
}
85-
86-
fn const_is_suggestable(kind: ConstKind<'_>) -> bool {
87-
match kind {
88-
ConstKind::Infer(..)
89-
| ConstKind::Bound(..)
90-
| ConstKind::Placeholder(..)
91-
| ConstKind::Error(..) => false,
92-
_ => true,
93-
}
94-
}
95-
96-
// FIXME(compiler-errors): Some types are still not good to suggest,
97-
// specifically references with lifetimes within the function. Not
98-
//sure we have enough information to resolve whether a region is
99-
// temporary, so I'll leave this as a fixme.
78+
pub trait IsSuggestable<'tcx> {
79+
/// Whether this makes sense to suggest in a diagnostic.
80+
///
81+
/// We filter out certain types and constants since they don't provide
82+
/// meaningful rendered suggestions when pretty-printed. We leave some
83+
/// nonsense, such as region vars, since those render as `'_` and are
84+
/// usually okay to reinterpret as elided lifetimes.
85+
fn is_suggestable(self, tcx: TyCtxt<'tcx>) -> bool;
86+
}
10087

101-
match self.kind() {
102-
FnDef(..)
103-
| Closure(..)
104-
| Infer(..)
105-
| Generator(..)
106-
| GeneratorWitness(..)
107-
| Bound(_, _)
108-
| Placeholder(_)
109-
| Error(_) => false,
110-
Opaque(did, substs) => {
111-
let parent = tcx.parent(*did);
112-
if let hir::def::DefKind::TyAlias | hir::def::DefKind::AssocTy = tcx.def_kind(parent)
113-
&& let Opaque(parent_did, _) = tcx.type_of(parent).kind()
114-
&& parent_did == did
115-
{
116-
substs.iter().all(|a| generic_arg_is_suggestible(a, tcx))
117-
} else {
118-
false
119-
}
120-
}
121-
Dynamic(dty, _) => dty.iter().all(|pred| match pred.skip_binder() {
122-
ExistentialPredicate::Trait(ExistentialTraitRef { substs, .. }) => {
123-
substs.iter().all(|a| generic_arg_is_suggestible(a, tcx))
124-
}
125-
ExistentialPredicate::Projection(ExistentialProjection {
126-
substs, term, ..
127-
}) => {
128-
let term_is_suggestable = match term {
129-
Term::Ty(ty) => ty.is_suggestable(tcx),
130-
Term::Const(c) => const_is_suggestable(c.val()),
131-
};
132-
term_is_suggestable && substs.iter().all(|a| generic_arg_is_suggestible(a, tcx))
133-
}
134-
_ => true,
135-
}),
136-
Projection(ProjectionTy { substs: args, .. }) | Adt(_, args) => {
137-
args.iter().all(|a| generic_arg_is_suggestible(a, tcx))
138-
}
139-
Tuple(args) => args.iter().all(|ty| ty.is_suggestable(tcx)),
140-
Slice(ty) | RawPtr(TypeAndMut { ty, .. }) | Ref(_, ty, _) => ty.is_suggestable(tcx),
141-
Array(ty, c) => ty.is_suggestable(tcx) && const_is_suggestable(c.val()),
142-
_ => true,
143-
}
88+
impl<'tcx, T> IsSuggestable<'tcx> for T
89+
where
90+
T: TypeFoldable<'tcx>,
91+
{
92+
fn is_suggestable(self, tcx: TyCtxt<'tcx>) -> bool {
93+
self.visit_with(&mut IsSuggestableVisitor { tcx }).is_continue()
14494
}
14595
}
14696

147-
pub fn suggest_arbitrary_trait_bound(
97+
pub fn suggest_arbitrary_trait_bound<'tcx>(
98+
tcx: TyCtxt<'tcx>,
14899
generics: &hir::Generics<'_>,
149100
err: &mut Diagnostic,
150-
param_name: &str,
151-
constraint: &str,
101+
trait_pred: PolyTraitPredicate<'tcx>,
152102
) -> bool {
103+
if !trait_pred.is_suggestable(tcx) {
104+
return false;
105+
}
106+
107+
let param_name = trait_pred.skip_binder().self_ty().to_string();
108+
let constraint = trait_pred.print_modifiers_and_trait_path().to_string();
153109
let param = generics.params.iter().find(|p| p.name.ident().as_str() == param_name);
154-
match (param, param_name) {
155-
(Some(_), "Self") => return false,
156-
_ => {}
110+
111+
// Skip, there is a param named Self
112+
if param.is_some() && param_name == "Self" {
113+
return false;
157114
}
115+
158116
// Suggest a where clause bound for a non-type parameter.
159-
let (action, prefix) = if generics.has_where_clause {
160-
("extending the", ", ")
161-
} else {
162-
("introducing a", " where ")
163-
};
164117
err.span_suggestion_verbose(
165118
generics.tail_span_for_predicate_suggestion(),
166119
&format!(
167-
"consider {} `where` bound, but there might be an alternative better way to express \
120+
"consider {} `where` clause, but there might be an alternative better way to express \
168121
this requirement",
169-
action,
122+
if generics.where_clause_span.is_empty() { "introducing a" } else { "extending the" },
170123
),
171-
format!("{}{}: {}", prefix, param_name, constraint),
124+
format!("{} {}: {}", generics.add_where_or_trailing_comma(), param_name, constraint),
172125
Applicability::MaybeIncorrect,
173126
);
174127
true
@@ -321,7 +274,7 @@ pub fn suggest_constraining_type_params<'a>(
321274
continue;
322275
}
323276

324-
if generics.has_where_clause {
277+
if generics.has_where_clause_predicates {
325278
// This part is a bit tricky, because using the `where` clause user can
326279
// provide zero, one or many bounds for the same type parameter, so we
327280
// have following cases to consider:
@@ -463,3 +416,78 @@ impl<'v> hir::intravisit::Visitor<'v> for StaticLifetimeVisitor<'v> {
463416
}
464417
}
465418
}
419+
420+
pub struct IsSuggestableVisitor<'tcx> {
421+
tcx: TyCtxt<'tcx>,
422+
}
423+
424+
impl<'tcx> TypeVisitor<'tcx> for IsSuggestableVisitor<'tcx> {
425+
type BreakTy = ();
426+
427+
fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
428+
match t.kind() {
429+
FnDef(..)
430+
| Closure(..)
431+
| Infer(..)
432+
| Generator(..)
433+
| GeneratorWitness(..)
434+
| Bound(_, _)
435+
| Placeholder(_)
436+
| Error(_) => {
437+
return ControlFlow::Break(());
438+
}
439+
440+
Opaque(did, _) => {
441+
let parent = self.tcx.parent(*did);
442+
if let hir::def::DefKind::TyAlias | hir::def::DefKind::AssocTy = self.tcx.def_kind(parent)
443+
&& let Opaque(parent_did, _) = self.tcx.type_of(parent).kind()
444+
&& parent_did == did
445+
{
446+
// Okay
447+
} else {
448+
return ControlFlow::Break(());
449+
}
450+
}
451+
452+
Dynamic(dty, _) => {
453+
for pred in *dty {
454+
match pred.skip_binder() {
455+
ExistentialPredicate::Trait(_) | ExistentialPredicate::Projection(_) => {
456+
// Okay
457+
}
458+
_ => return ControlFlow::Break(()),
459+
}
460+
}
461+
}
462+
463+
Param(param) => {
464+
// FIXME: It would be nice to make this not use string manipulation,
465+
// but it's pretty hard to do this, since `ty::ParamTy` is missing
466+
// sufficient info to determine if it is synthetic, and we don't
467+
// always have a convenient way of getting `ty::Generics` at the call
468+
// sites we invoke `IsSuggestable::is_suggestable`.
469+
if param.name.as_str().starts_with("impl ") {
470+
return ControlFlow::Break(());
471+
}
472+
}
473+
474+
_ => {}
475+
}
476+
477+
t.super_visit_with(self)
478+
}
479+
480+
fn visit_const(&mut self, c: Const<'tcx>) -> ControlFlow<Self::BreakTy> {
481+
match c.val() {
482+
ConstKind::Infer(..)
483+
| ConstKind::Bound(..)
484+
| ConstKind::Placeholder(..)
485+
| ConstKind::Error(..) => {
486+
return ControlFlow::Break(());
487+
}
488+
_ => {}
489+
}
490+
491+
c.super_visit_with(self)
492+
}
493+
}

‎compiler/rustc_middle/src/ty/generics.rs

+7
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,13 @@ impl GenericParamDefKind {
3939
GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => true,
4040
}
4141
}
42+
43+
pub fn is_synthetic(&self) -> bool {
44+
match self {
45+
GenericParamDefKind::Type { synthetic, .. } => *synthetic,
46+
_ => false,
47+
}
48+
}
4249
}
4350

4451
#[derive(Clone, Debug, TyEncodable, TyDecodable, HashStable)]
There was a problem loading the remainder of the diff.

0 commit comments

Comments
 (0)
Failed to load comments.