Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
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 { "trait" } else { "" };
format!("{stable}{trait_}{} {}", pluralize!(n), match &trait_names[..] {
[t] => t.to_string(),
[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 with {post}")
}
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
Loading