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

In some limited cases, suggest where bounds for non-type params #82194

Merged
merged 1 commit into from
Feb 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions compiler/rustc_middle/src/ty/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,36 @@ impl<'tcx> TyS<'tcx> {
}
}

pub fn suggest_arbitrary_trait_bound(
generics: &hir::Generics<'_>,
err: &mut DiagnosticBuilder<'_>,
param_name: &str,
constraint: &str,
) -> bool {
let param = generics.params.iter().find(|p| p.name.ident().as_str() == param_name);
match (param, param_name) {
(Some(_), "Self") => return false,
_ => {}
}
// Suggest a where clause bound for a non-type paremeter.
let (action, prefix) = if generics.where_clause.predicates.is_empty() {
("introducing a", " where ")
} else {
("extending the", ", ")
};
err.span_suggestion_verbose(
generics.where_clause.tail_span_for_suggestion(),
&format!(
"consider {} `where` bound, but there might be an alternative better way to express \
this requirement",
action,
),
format!("{}{}: {}", prefix, param_name, constraint),
Applicability::MaybeIncorrect,
);
true
}

/// Suggest restricting a type param with a new bound.
pub fn suggest_constraining_type_param(
tcx: TyCtxt<'_>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ use rustc_hir::intravisit::Visitor;
use rustc_hir::lang_items::LangItem;
use rustc_hir::{AsyncGeneratorKind, GeneratorKind, Node};
use rustc_middle::ty::{
self, suggest_constraining_type_param, AdtKind, DefIdTree, Infer, InferTy, ToPredicate, Ty,
TyCtxt, TypeFoldable, WithConstness,
self, suggest_arbitrary_trait_bound, suggest_constraining_type_param, AdtKind, DefIdTree,
Infer, InferTy, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness,
};
use rustc_middle::ty::{TypeAndMut, TypeckResults};
use rustc_span::symbol::{kw, sym, Ident, Symbol};
Expand Down Expand Up @@ -334,7 +334,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
let (param_ty, projection) = match self_ty.kind() {
ty::Param(_) => (true, None),
ty::Projection(projection) => (false, Some(projection)),
_ => return,
_ => (false, None),
};

// FIXME: Add check for trait bound that is already present, particularly `?Sized` so we
Expand Down Expand Up @@ -453,6 +453,26 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}
}

hir::Node::Item(hir::Item {
kind:
hir::ItemKind::Struct(_, generics)
| hir::ItemKind::Enum(_, generics)
| hir::ItemKind::Union(_, generics)
| hir::ItemKind::Trait(_, _, generics, ..)
| hir::ItemKind::Impl(hir::Impl { generics, .. })
| hir::ItemKind::Fn(_, generics, _)
| hir::ItemKind::TyAlias(_, generics)
| hir::ItemKind::TraitAlias(generics, _)
| hir::ItemKind::OpaqueTy(hir::OpaqueTy { generics, .. }),
..
}) if !param_ty => {
// Missing generic type parameter bound.
let param_name = self_ty.to_string();
let constraint = trait_ref.print_only_trait_path().to_string();
if suggest_arbitrary_trait_bound(generics, &mut err, &param_name, &constraint) {
return;
}
}
hir::Node::Crate(..) => return,

_ => {}
Expand Down
4 changes: 4 additions & 0 deletions src/test/ui/partialeq_help.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ LL | a == b;
| ^^ no implementation for `&T == T`
|
= help: the trait `PartialEq<T>` is not implemented for `&T`
help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement
|
LL | fn foo<T: PartialEq>(a: &T, b: T) where &T: PartialEq<T> {
| ^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ LL | default type U = &'static B;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no implementation for `&'static B == B`
|
= help: the trait `PartialEq<B>` is not implemented for `&'static B`
help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement
|
LL | impl<B: 'static, T> X<B> for T where &'static B: PartialEq<B> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error; 1 warning emitted

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/suggestions/suggest-change-mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use std::io::{BufRead, BufReader, Read, Write};

fn issue_81421<T: Read + Write>(mut stream: T) {
fn issue_81421<T: Read + Write>(mut stream: T) { //~ HELP consider introducing a `where` bound
let initial_message = format!("Hello world");
let mut buffer: Vec<u8> = Vec::new();
let bytes_written = stream.write_all(initial_message.as_bytes());
Expand Down
4 changes: 4 additions & 0 deletions src/test/ui/suggestions/suggest-change-mut.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ help: consider removing the leading `&`-reference
|
LL | let mut stream_reader = BufReader::new(stream);
| --
help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement
|
LL | fn issue_81421<T: Read + Write>(mut stream: T) where &T: std::io::Read {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this syntax even allowed? where &T: .. will not compile due to missing lifetime parameters unfortunately

error[E0637]: `&` without an explicit lifetime name cannot be used here
 --> src/lib.rs:2:23
  |
2 | fn foo<T>(x: T) where &T: std::io::Read { }
  |                       ^ explicit lifetime name needed here

Copy link
Contributor Author

@estebank estebank Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We (I?) allow for limited "mistakes" in the suggested code as long as the compiler gives you further suggestions that lead you to working code. We can gate this suggestion in particular or/and extend E0637 to suggest where a lifetime can be added (we already have suggestions to extend foo's type param list and to add a for lifetime in other places).

Indeed writing where for<'a> &'a T: Read compiles successfully.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #82312 to track that potential work.

| ^^^^^^^^^^^^^^^^^^^^^^^
help: consider changing this borrow's mutability
|
LL | let mut stream_reader = BufReader::new(&mut stream);
Expand Down
8 changes: 8 additions & 0 deletions src/test/ui/traits/suggest-where-clause.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ LL | <u64 as From<T>>::from;
| ^^^^^^^^^^^^^^^^^^^^^^ the trait `From<T>` is not implemented for `u64`
|
= note: required by `from`
help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement
|
LL | fn check<T: Iterator, U: ?Sized>() where u64: From<T> {
| ^^^^^^^^^^^^^^^^^^

error[E0277]: the trait bound `u64: From<<T as Iterator>::Item>` is not satisfied
--> $DIR/suggest-where-clause.rs:18:5
Expand All @@ -43,6 +47,10 @@ LL | <u64 as From<<T as Iterator>::Item>>::from;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `From<<T as Iterator>::Item>` is not implemented for `u64`
|
= note: required by `from`
help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement
|
LL | fn check<T: Iterator, U: ?Sized>() where u64: From<<T as Iterator>::Item> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0277]: the trait bound `Misc<_>: From<T>` is not satisfied
--> $DIR/suggest-where-clause.rs:23:5
Expand Down