Skip to content

Commit

Permalink
Auto merge of rust-lang#133522 - estebank:dont-suggest-unstable-trait…
Browse files Browse the repository at this point in the history
…, r=compiler-errors

Don't suggest restricting bound with unstable traits on stable and mention it's unstable on nightly

On nightly, we mention the trait is unstable

```
error[E0277]: the trait bound `T: Unstable` is not satisfied
  --> $DIR/unstable-trait-suggestion.rs:13:9
   |
LL |     foo(t)
   |     --- ^ the trait `Unstable` is not implemented for `T`
   |     |
   |     required by a bound introduced by this call
   |
note: required by a bound in `foo`
  --> $DIR/unstable-trait-suggestion.rs:9:11
   |
LL | fn foo<T: Unstable>(_: T) {}
   |           ^^^^^^^^ required by this bound in `foo`
help: consider restricting type parameter `T` but it is an `unstable` trait
   |
LL | pub fn demo<T: Unstable>(t: T) {
   |              ++++++++++
```

On stable, we don't suggest the trait at all

```
error[E0277]: the trait bound `T: Unstable` is not satisfied
  --> $DIR/unstable-trait-suggestion.rs:13:9
   |
LL |     foo(t)
   |     --- ^ the trait `Unstable` is not implemented for `T`
   |     |
   |     required by a bound introduced by this call
   |
note: required by a bound in `foo`
  --> $DIR/unstable-trait-suggestion.rs:9:11
   |
LL | fn foo<T: Unstable>(_: T) {}
   |           ^^^^^^^^ required by this bound in `foo`
```

Fix rust-lang#133511.
  • Loading branch information
bors committed Dec 8, 2024
2 parents 728f2da + 25ad047 commit f415c07
Show file tree
Hide file tree
Showing 160 changed files with 486 additions and 374 deletions.
7 changes: 4 additions & 3 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1450,6 +1450,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
ty::Param(param_ty) => Ok((
generics.type_param(param_ty, tcx),
predicate.trait_ref.print_trait_sugared().to_string(),
Some(predicate.trait_ref.def_id),
)),
_ => Err(()),
}
Expand All @@ -1463,9 +1464,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
tcx,
hir_generics,
err,
predicates
.iter()
.map(|(param, constraint)| (param.name.as_str(), &**constraint, None)),
predicates.iter().map(|(param, constraint, def_id)| {
(param.name.as_str(), &**constraint, *def_id)
}),
None,
);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> {
err,
param_ty.name.as_str(),
&constraint,
None,
Some(trait_ref.def_id),
None,
);
}
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,13 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
} else {
let mut err = self.dcx().create_err(err);
if suggest_constraining_type_param(
tcx, generics, &mut err, &qself_str, &trait_ref, None, None,
tcx,
generics,
&mut err,
&qself_str,
&trait_ref,
Some(best_trait),
None,
) && !identically_named
{
// We suggested constraining a type parameter, but the associated item on it
Expand Down
126 changes: 99 additions & 27 deletions compiler/rustc_middle/src/ty/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
//! Diagnostics related methods for `Ty`.
use std::borrow::Cow;
use std::fmt::Write;
use std::ops::ControlFlow;

use rustc_data_structures::fx::FxHashMap;
use rustc_errors::{Applicability, Diag, DiagArgValue, IntoDiagArg, into_diag_arg_using_display};
use rustc_errors::{
Applicability, Diag, DiagArgValue, IntoDiagArg, into_diag_arg_using_display, pluralize,
};
use rustc_hir::def::DefKind;
use rustc_hir::def_id::DefId;
use rustc_hir::{self as hir, LangItem, PredicateOrigin, WherePredicateKind};
Expand Down Expand Up @@ -161,7 +162,7 @@ pub fn suggest_arbitrary_trait_bound<'tcx>(
true
}

#[derive(Debug)]
#[derive(Debug, Clone, Copy)]
enum SuggestChangingConstraintsMessage<'a> {
RestrictBoundFurther,
RestrictType { ty: &'a str },
Expand All @@ -172,7 +173,7 @@ enum SuggestChangingConstraintsMessage<'a> {

fn suggest_changing_unsized_bound(
generics: &hir::Generics<'_>,
suggestions: &mut Vec<(Span, String, SuggestChangingConstraintsMessage<'_>)>,
suggestions: &mut Vec<(Span, String, String, SuggestChangingConstraintsMessage<'_>)>,
param: &hir::GenericParam<'_>,
def_id: Option<DefId>,
) {
Expand Down Expand Up @@ -207,7 +208,8 @@ fn suggest_changing_unsized_bound(
continue;
}

let mut push_suggestion = |sp, msg| suggestions.push((sp, String::new(), msg));
let mut push_suggestion =
|sp, msg| suggestions.push((sp, "Sized".to_string(), String::new(), msg));

if predicate.bounds.len() == unsized_bounds.len() {
// All the bounds are unsized bounds, e.g.
Expand Down Expand Up @@ -278,8 +280,25 @@ pub fn suggest_constraining_type_params<'a>(
span_to_replace: Option<Span>,
) -> bool {
let mut grouped = FxHashMap::default();
let mut unstable_suggestion = false;
param_names_and_constraints.for_each(|(param_name, constraint, def_id)| {
grouped.entry(param_name).or_insert(Vec::new()).push((constraint, def_id))
let stable = match def_id {
Some(def_id) => match tcx.lookup_stability(def_id) {
Some(s) => s.level.is_stable(),
None => true,
},
None => true,
};
if stable || tcx.sess.is_nightly_build() {
grouped.entry(param_name).or_insert(Vec::new()).push((
constraint,
def_id,
if stable { "" } else { "unstable " },
));
if !stable {
unstable_suggestion = true;
}
}
});

let mut applicability = Applicability::MachineApplicable;
Expand All @@ -290,16 +309,21 @@ pub fn suggest_constraining_type_params<'a>(
let Some(param) = param else { return false };

{
let mut sized_constraints = constraints.extract_if(|(_, def_id)| {
let mut sized_constraints = constraints.extract_if(|(_, def_id, _)| {
def_id.is_some_and(|def_id| tcx.is_lang_item(def_id, LangItem::Sized))
});
if let Some((_, def_id)) = sized_constraints.next() {
if let Some((_, def_id, _)) = sized_constraints.next() {
applicability = Applicability::MaybeIncorrect;

err.span_label(param.span, "this type parameter needs to be `Sized`");
suggest_changing_unsized_bound(generics, &mut suggestions, param, def_id);
}
}
let bound_message = if constraints.iter().any(|(_, def_id, _)| def_id.is_none()) {
SuggestChangingConstraintsMessage::RestrictBoundFurther
} else {
SuggestChangingConstraintsMessage::RestrictTypeFurther { ty: param_name }
};

// in the scenario like impl has stricter requirements than trait,
// we should not suggest restrict bound on the impl, here we double check
Expand All @@ -312,15 +336,54 @@ pub fn suggest_constraining_type_params<'a>(
.collect();

constraints
.retain(|(_, def_id)| def_id.map_or(true, |def| !bound_trait_defs.contains(&def)));
.retain(|(_, def_id, _)| def_id.map_or(true, |def| !bound_trait_defs.contains(&def)));

if constraints.is_empty() {
continue;
}

let mut constraint = constraints.iter().map(|&(c, _)| c).collect::<Vec<_>>();
let mut constraint = constraints.iter().map(|&(c, _, _)| c).collect::<Vec<_>>();
constraint.sort();
constraint.dedup();
let all_known = constraints.iter().all(|&(_, def_id, _)| def_id.is_some());
let all_stable = constraints.iter().all(|&(_, _, stable)| stable.is_empty());
let all_unstable = constraints.iter().all(|&(_, _, stable)| !stable.is_empty());
let post = if all_stable || all_unstable {
// Don't redundantly say "trait `X`, trait `Y`", instead "traits `X` and `Y`"
let mut trait_names = constraints
.iter()
.map(|&(c, def_id, _)| match def_id {
None => format!("`{c}`"),
Some(def_id) => format!("`{}`", tcx.item_name(def_id)),
})
.collect::<Vec<_>>();
trait_names.sort();
trait_names.dedup();
let n = trait_names.len();
let stable = if all_stable { "" } else { "unstable " };
let trait_ = if all_known { format!("trait{}", pluralize!(n)) } else { String::new() };
format!("{stable}{trait_}{}", match &trait_names[..] {
[t] => format!(" {t}"),
[ts @ .., last] => format!(" {} and {last}", ts.join(", ")),
[] => return false,
},)
} else {
// We're more explicit when there's a mix of stable and unstable traits.
let mut trait_names = constraints
.iter()
.map(|&(c, def_id, stable)| match def_id {
None => format!("`{c}`"),
Some(def_id) => format!("{stable}trait `{}`", tcx.item_name(def_id)),
})
.collect::<Vec<_>>();
trait_names.sort();
trait_names.dedup();
match &trait_names[..] {
[t] => t.to_string(),
[ts @ .., last] => format!("{} and {last}", ts.join(", ")),
[] => return false,
}
};
let constraint = constraint.join(" + ");
let mut suggest_restrict = |span, bound_list_non_empty, open_paren_sp| {
let suggestion = if span_to_replace.is_some() {
Expand All @@ -333,13 +396,11 @@ pub fn suggest_constraining_type_params<'a>(
format!(" {constraint}")
};

use SuggestChangingConstraintsMessage::RestrictBoundFurther;

if let Some(open_paren_sp) = open_paren_sp {
suggestions.push((open_paren_sp, "(".to_string(), RestrictBoundFurther));
suggestions.push((span, format!("){suggestion}"), RestrictBoundFurther));
suggestions.push((open_paren_sp, post.clone(), "(".to_string(), bound_message));
suggestions.push((span, post.clone(), format!("){suggestion}"), bound_message));
} else {
suggestions.push((span, suggestion, RestrictBoundFurther));
suggestions.push((span, post.clone(), suggestion, bound_message));
}
};

Expand Down Expand Up @@ -397,7 +458,8 @@ pub fn suggest_constraining_type_params<'a>(
// - insert: `, X: Bar`
suggestions.push((
generics.tail_span_for_predicate_suggestion(),
constraints.iter().fold(String::new(), |mut string, &(constraint, _)| {
post,
constraints.iter().fold(String::new(), |mut string, &(constraint, _, _)| {
write!(string, ", {param_name}: {constraint}").unwrap();
string
}),
Expand Down Expand Up @@ -426,6 +488,7 @@ pub fn suggest_constraining_type_params<'a>(
// default (`<T=Foo>`), so we suggest adding `where T: Bar`.
suggestions.push((
generics.tail_span_for_predicate_suggestion(),
post,
format!("{where_prefix} {param_name}: {constraint}"),
SuggestChangingConstraintsMessage::RestrictTypeFurther { ty: param_name },
));
Expand All @@ -439,6 +502,7 @@ pub fn suggest_constraining_type_params<'a>(
if let Some(colon_span) = param.colon_span {
suggestions.push((
colon_span.shrink_to_hi(),
post,
format!(" {constraint}"),
SuggestChangingConstraintsMessage::RestrictType { ty: param_name },
));
Expand All @@ -451,6 +515,7 @@ pub fn suggest_constraining_type_params<'a>(
// - help: consider restricting this type parameter with `T: Foo`
suggestions.push((
param.span.shrink_to_hi(),
post,
format!(": {constraint}"),
SuggestChangingConstraintsMessage::RestrictType { ty: param_name },
));
Expand All @@ -459,39 +524,46 @@ pub fn suggest_constraining_type_params<'a>(
// FIXME: remove the suggestions that are from derive, as the span is not correct
suggestions = suggestions
.into_iter()
.filter(|(span, _, _)| !span.in_derive_expansion())
.filter(|(span, _, _, _)| !span.in_derive_expansion())
.collect::<Vec<_>>();

let suggested = !suggestions.is_empty();
if suggestions.len() == 1 {
let (span, suggestion, msg) = suggestions.pop().unwrap();
let (span, post, suggestion, msg) = suggestions.pop().unwrap();
let msg = match msg {
SuggestChangingConstraintsMessage::RestrictBoundFurther => {
Cow::from("consider further restricting this bound")
format!("consider further restricting this bound")
}
SuggestChangingConstraintsMessage::RestrictTypeFurther { ty }
| SuggestChangingConstraintsMessage::RestrictType { ty }
if ty.starts_with("impl ") =>
{
format!("consider restricting opaque type `{ty}` with {post}")
}
SuggestChangingConstraintsMessage::RestrictType { ty } => {
Cow::from(format!("consider restricting type parameter `{ty}`"))
format!("consider restricting type parameter `{ty}` with {post}")
}
SuggestChangingConstraintsMessage::RestrictTypeFurther { ty } => {
Cow::from(format!("consider further restricting type parameter `{ty}`"))
format!("consider further restricting type parameter `{ty}` with {post}")
}
SuggestChangingConstraintsMessage::RemoveMaybeUnsized => {
Cow::from("consider removing the `?Sized` bound to make the type parameter `Sized`")
format!("consider removing the `?Sized` bound to make the type parameter `Sized`")
}
SuggestChangingConstraintsMessage::ReplaceMaybeUnsizedWithSized => {
Cow::from("consider replacing `?Sized` with `Sized`")
format!("consider replacing `?Sized` with `Sized`")
}
};

err.span_suggestion_verbose(span, msg, suggestion, applicability);
} else if suggestions.len() > 1 {
let post = if unstable_suggestion { " (some of them are unstable traits)" } else { "" };
err.multipart_suggestion_verbose(
"consider restricting type parameters",
suggestions.into_iter().map(|(span, suggestion, _)| (span, suggestion)).collect(),
format!("consider restricting type parameters{post}"),
suggestions.into_iter().map(|(span, _, suggestion, _)| (span, suggestion)).collect(),
applicability,
);
}

true
suggested
}

/// Collect al types that have an implicit `'static` obligation that we could suggest `'_` for.
Expand Down
4 changes: 4 additions & 0 deletions tests/run-make/missing-unstable-trait-bound/missing-bound.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
pub fn baz<T>(t: std::ops::Range<T>) {
for _ in t {}
}
fn main() {}
12 changes: 12 additions & 0 deletions tests/run-make/missing-unstable-trait-bound/missing-bound.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0277]: the trait bound `T: Step` is not satisfied
--> missing-bound.rs:2:14
|
2 | for _ in t {}
| ^ the trait `Step` is not implemented for `T`
|
= note: required for `std::ops::Range<T>` to implement `Iterator`
= note: required for `std::ops::Range<T>` to implement `IntoIterator`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0277`.
24 changes: 24 additions & 0 deletions tests/run-make/missing-unstable-trait-bound/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
//@ only-linux
//@ ignore-wasm32
//@ ignore-wasm64
// ignore-tidy-linelength

// Ensure that on stable we don't suggest restricting with an unsafe trait and we continue
// mentioning the rest of the obligation chain.

use run_make_support::{diff, rust_lib_name, rustc};

fn main() {
let out = rustc()
.env("RUSTC_BOOTSTRAP", "-1")
.input("missing-bound.rs")
.run_fail()
.assert_stderr_not_contains("help: consider restricting type parameter `T`")
.assert_stderr_contains(
r#"
= note: required for `std::ops::Range<T>` to implement `Iterator`
= note: required for `std::ops::Range<T>` to implement `IntoIterator`"#,
)
.stderr_utf8();
diff().expected_file("missing-bound.stderr").actual_text("(stable rustc)", &out).run()
}
4 changes: 2 additions & 2 deletions tests/rustdoc-ui/issues/issue-96287.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0220]: associated type `Assoc` not found for `V`
LL | pub type Foo<V> = impl Trait<V::Assoc>;
| ^^^^^ there is an associated type `Assoc` in the trait `TraitWithAssoc`
|
help: consider restricting type parameter `V`
help: consider restricting type parameter `V` with trait `TraitWithAssoc`
|
LL | pub type Foo<V: TraitWithAssoc> = impl Trait<V::Assoc>;
| ++++++++++++++++
Expand All @@ -16,7 +16,7 @@ LL | pub type Foo<V> = impl Trait<V::Assoc>;
| ^^^^^ there is an associated type `Assoc` in the trait `TraitWithAssoc`
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: consider restricting type parameter `V`
help: consider restricting type parameter `V` with trait `TraitWithAssoc`
|
LL | pub type Foo<V: TraitWithAssoc> = impl Trait<V::Assoc>;
| ++++++++++++++++
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0277]: the trait bound `C: Bar<5>` is not satisfied
LL | pub struct Structure<C: Tec> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Bar<5>` is not implemented for `C`
|
help: consider further restricting this bound
help: consider further restricting type parameter `C` with trait `Bar`
|
LL | pub struct Structure<C: Tec + Bar<5>> {
| ++++++++
Expand All @@ -15,7 +15,7 @@ error[E0277]: the trait bound `C: Bar<5>` is not satisfied
LL | _field: C::BarType,
| ^^^^^^^^^^ the trait `Bar<5>` is not implemented for `C`
|
help: consider further restricting this bound
help: consider further restricting type parameter `C` with trait `Bar`
|
LL | pub struct Structure<C: Tec + Bar<5>> {
| ++++++++
Expand Down
Loading

0 comments on commit f415c07

Please sign in to comment.