bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #94375 - WaffleLapkin:copy-suggestion, r=estebank

Browse source 
Adt copy suggestions

Previously we've only suggested adding `Copy` bounds when the type being moved/copied is a type parameter (generic). With this PR we also suggest adding bounds when a type
- Can be copy
- All predicates that need to be satisfied for that are based on type params

i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest anything for `Option<String>`.

An example:
```rust
fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) {
    (t, t)
}
```
New error (current compiler doesn't provide `help`:):
```text
error[E0382]: use of moved value: `t`
 --> t.rs:2:9
  |
1 | fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) {
  |                 - move occurs because `t` has type `Option<T>`, which does not implement the `Copy` trait
2 |     (t, t)
  |      -  ^ value used here after move
  |      |
  |      value moved here
  |
help: consider restricting type parameter `T`
  |
1 | fn duplicate<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) {
  |               ++++++
```

Fixes #93623
r? ``````````@estebank``````````
``````````@rustbot`````````` label +A-diagnostics +A-suggestion-diagnostics +C-enhancement

----

I'm not at all sure if this is the right implementation for this kind of suggestion, but it seems to work :')
This commit is contained in:
Dylan DPC 2022-03-03 01:09:11 +01:00 • committed by GitHub
commit 7537b2036a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 608 additions and 164 deletions
Download patch file Download diff file Expand all files Collapse all files

63
compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file

@ -5,16 +5,21 @@ use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_hir::{AsyncGeneratorKind, GeneratorKind};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_infer::traits::ObligationCause;
use rustc_middle::mir::{
self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, ConstraintCategory,
FakeReadCause, LocalDecl, LocalInfo, LocalKind, Location, Operand, Place, PlaceRef,
ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, VarBindingForm,
};
use rustc_middle::ty::{self, suggest_constraining_type_param, Ty};
use rustc_middle::ty::{
self, suggest_constraining_type_param, suggest_constraining_type_params, PredicateKind, Ty,
};
use rustc_mir_dataflow::move_paths::{InitKind, MoveOutIndex, MovePathIndex};
use rustc_span::symbol::sym;
use rustc_span::{BytePos, MultiSpan, Span, DUMMY_SP};
use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::traits::TraitEngineExt as _;
use crate::borrow_set::TwoPhaseActivation;
use crate::borrowck_errors;
@ -423,7 +428,63 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
None,
);
}
} else {
// Try to find predicates on *generic params* that would allow copying `ty`
let tcx = self.infcx.tcx;
let generics = tcx.generics_of(self.mir_def_id());
if let Some(hir_generics) = tcx
.typeck_root_def_id(self.mir_def_id().to_def_id())
.as_local()
.and_then(|def_id| tcx.hir().get_generics(def_id))
{
let predicates: Result<Vec<_>, _> = tcx.infer_ctxt().enter(|infcx| {
let mut fulfill_cx =
<dyn rustc_infer::traits::TraitEngine<'_>>::new(infcx.tcx);
let copy_did = infcx.tcx.lang_items().copy_trait().unwrap();
let cause = ObligationCause::new(
span,
self.mir_hir_id(),
rustc_infer::traits::ObligationCauseCode::MiscObligation,
);
fulfill_cx.register_bound(&infcx, self.param_env, ty, copy_did, cause);
let errors = fulfill_cx.select_where_possible(&infcx);
// Only emit suggestion if all required predicates are on generic
errors
.into_iter()
.map(|err| match err.obligation.predicate.kind().skip_binder() {
PredicateKind::Trait(predicate) => {
match predicate.self_ty().kind() {
ty::Param(param_ty) => Ok((
generics.type_param(param_ty, tcx),
predicate
.trait_ref
.print_only_trait_path()
.to_string(),
)),
_ => Err(()),
}
}
_ => Err(()),
})
.collect()
});
if let Ok(predicates) = predicates {
suggest_constraining_type_params(
tcx,
hir_generics,
&mut err,
predicates.iter().map(|(param, constraint)| {
(param.name.as_str(), &**constraint, None)
}),
);
}
}
}
let span = if let Some(local) = place.as_local() {
let decl = &self.body.local_decls[local];
Some(decl.source_info.span)

1
compiler/rustc_middle/src/lib.rs
View file

@ -56,6 +56,7 @@
#![feature(nonzero_ops)]
#![feature(unwrap_infallible)]
#![feature(decl_macro)]
#![feature(drain_filter)]
#![recursion_limit = "512"]
#![allow(rustc::potential_query_instability)]

168
compiler/rustc_middle/src/ty/diagnostics.rs
View file

@ -7,6 +7,7 @@ use crate::ty::{
ProjectionTy, Term, Ty, TyCtxt, TypeAndMut,
};
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::{Applicability, Diagnostic};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
@ -157,9 +158,17 @@ pub fn suggest_arbitrary_trait_bound(
true
}
#[derive(Debug)]
enum SuggestChangingConstraintsMessage<'a> {
RestrictBoundFurther,
RestrictType { ty: &'a str },
RestrictTypeFurther { ty: &'a str },
RemovingQSized,
}
fn suggest_removing_unsized_bound(
generics: &hir::Generics<'_>,
err: &mut Diagnostic,
suggestions: &mut Vec<(Span, String, SuggestChangingConstraintsMessage<'_>)>,
param_name: &str,
param: &hir::GenericParam<'_>,
def_id: Option<DefId>,
@ -221,13 +230,12 @@ fn suggest_removing_unsized_bound(
// ^^^^^^^^^
(_, pos, _, _) => bounds[pos - 1].span().shrink_to_hi().to(bound.span()),
};
err.span_suggestion_verbose(
suggestions.push((
sp,
"consider removing the `?Sized` bound to make the \
type parameter `Sized`",
String::new(),
Applicability::MaybeIncorrect,
);
SuggestChangingConstraintsMessage::RemovingQSized,
));
}
}
_ => {}
@ -249,13 +257,12 @@ fn suggest_removing_unsized_bound(
// ^^^^^^^^^
(_, pos) => param.bounds[pos - 1].span().shrink_to_hi().to(bound.span()),
};
err.span_suggestion_verbose(
suggestions.push((
sp,
"consider removing the `?Sized` bound to make the type parameter \
`Sized`",
String::new(),
Applicability::MaybeIncorrect,
);
SuggestChangingConstraintsMessage::RemovingQSized,
));
}
_ => {}
}
@ -271,30 +278,64 @@ pub fn suggest_constraining_type_param(
constraint: &str,
def_id: Option<DefId>,
) -> bool {
suggest_constraining_type_params(
tcx,
generics,
err,
[(param_name, constraint, def_id)].into_iter(),
)
}
/// Suggest restricting a type param with a new bound.
pub fn suggest_constraining_type_params<'a>(
tcx: TyCtxt<'_>,
generics: &hir::Generics<'_>,
err: &mut Diagnostic,
param_names_and_constraints: impl Iterator<Item = (&'a str, &'a str, Option<DefId>)>,
) -> bool {
let mut grouped = FxHashMap::default();
param_names_and_constraints.for_each(|(param_name, constraint, def_id)| {
grouped.entry(param_name).or_insert(Vec::new()).push((constraint, def_id))
});
let mut applicability = Applicability::MachineApplicable;
let mut suggestions = Vec::new();
for (param_name, mut constraints) in grouped {
let param = generics.params.iter().find(|p| p.name.ident().as_str() == param_name);
let Some(param) = param else { return false };
let Some(param) = param else {
return false;
};
{
let mut sized_constraints =
constraints.drain_filter(|(_, def_id)| *def_id == tcx.lang_items().sized_trait());
if let Some((constraint, def_id)) = sized_constraints.next() {
applicability = Applicability::MaybeIncorrect;
const MSG_RESTRICT_BOUND_FURTHER: &str = "consider further restricting this bound";
let msg_restrict_type = format!("consider restricting type parameter `{}`", param_name);
let msg_restrict_type_further =
format!("consider further restricting type parameter `{}`", param_name);
if def_id == tcx.lang_items().sized_trait() {
// Type parameters are already `Sized` by default.
err.span_label(param.span, &format!("this type parameter needs to be `{}`", constraint));
suggest_removing_unsized_bound(generics, err, param_name, param, def_id);
return true;
}
let mut suggest_restrict = |span| {
err.span_suggestion_verbose(
span,
MSG_RESTRICT_BOUND_FURTHER,
format!(" + {}", constraint),
Applicability::MachineApplicable,
err.span_label(
param.span,
&format!("this type parameter needs to be `{}`", constraint),
);
suggest_removing_unsized_bound(
generics,
&mut suggestions,
param_name,
param,
def_id,
);
}
}
if constraints.is_empty() {
continue;
}
let constraint = constraints.iter().map(|&(c, _)| c).collect::<Vec<_>>().join(" + ");
let mut suggest_restrict = |span| {
suggestions.push((
span,
format!(" + {}", constraint),
SuggestChangingConstraintsMessage::RestrictBoundFurther,
))
};
if param_name.starts_with("impl ") {
@ -314,7 +355,7 @@ pub fn suggest_constraining_type_param(
// replace with: `impl Foo + Bar`
suggest_restrict(param.span.shrink_to_hi());
return true;
continue;
}
if generics.where_clause.predicates.is_empty()
@ -342,15 +383,12 @@ pub fn suggest_constraining_type_param(
//
// fn foo<T>(t: T) { ... }
// - help: consider restricting this type parameter with `T: Foo`
err.span_suggestion_verbose(
suggestions.push((
param.span.shrink_to_hi(),
&msg_restrict_type,
format!(": {}", constraint),
Applicability::MachineApplicable,
);
SuggestChangingConstraintsMessage::RestrictType { ty: param_name },
));
}
true
} else {
// This part is a bit tricky, because using the `where` clause user can
// provide zero, one or many bounds for the same type parameter, so we
@ -408,12 +446,11 @@ pub fn suggest_constraining_type_param(
{
// Suggest a bound, but there is no existing `where` clause *and* the type param has a
// default (`<T=Foo>`), so we suggest adding `where T: Bar`.
err.span_suggestion_verbose(
suggestions.push((
generics.where_clause.tail_span_for_suggestion(),
&msg_restrict_type_further,
format!(" where {}: {}", param_name, constraint),
Applicability::MachineApplicable,
);
SuggestChangingConstraintsMessage::RestrictTypeFurther { ty: param_name },
));
} else {
let mut param_spans = Vec::new();
@ -437,18 +474,53 @@ pub fn suggest_constraining_type_param(
match param_spans[..] {
[&param_span] => suggest_restrict(param_span.shrink_to_hi()),
_ => {
err.span_suggestion_verbose(
suggestions.push((
generics.where_clause.tail_span_for_suggestion(),
&msg_restrict_type_further,
format!(", {}: {}", param_name, constraint),
Applicability::MachineApplicable,
);
constraints
.iter()
.map(|&(constraint, _)| format!(", {}: {}", param_name, constraint))
.collect::<String>(),
SuggestChangingConstraintsMessage::RestrictTypeFurther {
ty: param_name,
},
));
}
}
}
}
}
true
if suggestions.len() == 1 {
let (span, suggestion, msg) = suggestions.pop().unwrap();
let s;
let msg = match msg {
SuggestChangingConstraintsMessage::RestrictBoundFurther => {
"consider further restricting this bound"
}
SuggestChangingConstraintsMessage::RestrictType { ty } => {
s = format!("consider restricting type parameter `{}`", ty);
&s
}
SuggestChangingConstraintsMessage::RestrictTypeFurther { ty } => {
s = format!("consider further restricting type parameter `{}`", ty);
&s
}
SuggestChangingConstraintsMessage::RemovingQSized => {
"consider removing the `?Sized` bound to make the type parameter `Sized`"
}
};
err.span_suggestion_verbose(span, msg, suggestion, applicability);
} else {
err.multipart_suggestion_verbose(
"consider restricting type parameters",
suggestions.into_iter().map(|(span, suggestion, _)| (span, suggestion)).collect(),
applicability,
);
}
true
}
/// Collect al types that have an implicit `'static` obligation that we could suggest `'_` for.

6
src/test/ui/moves/use_of_moved_value_clone_suggestions.rs Normal file
View file

@ -0,0 +1,6 @@
// `Rc` is not ever `Copy`, we should not suggest adding `T: Copy` constraint
fn duplicate_rc<T>(t: std::rc::Rc<T>) -> (std::rc::Rc<T>, std::rc::Rc<T>) {
(t, t) //~ use of moved value: `t`
}
fn main() {}

13
src/test/ui/moves/use_of_moved_value_clone_suggestions.stderr Normal file
View file

@ -0,0 +1,13 @@
error[E0382]: use of moved value: `t`
--> $DIR/use_of_moved_value_clone_suggestions.rs:3:9
|
LL | fn duplicate_rc<T>(t: std::rc::Rc<T>) -> (std::rc::Rc<T>, std::rc::Rc<T>) {
| - move occurs because `t` has type `Rc<T>`, which does not implement the `Copy` trait
LL | (t, t)
| - ^ value used here after move
| |
| value moved here
error: aborting due to previous error
For more information about this error, try `rustc --explain E0382`.

72
src/test/ui/moves/use_of_moved_value_copy_suggestions.fixed Normal file
View file

@ -0,0 +1,72 @@
// run-rustfix
#![allow(dead_code)]
fn duplicate_t<T: Copy>(t: T) -> (T, T) {
//~^ HELP consider restricting type parameter `T`
(t, t) //~ use of moved value: `t`
}
fn duplicate_opt<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) {
//~^ HELP consider restricting type parameter `T`
(t, t) //~ use of moved value: `t`
}
fn duplicate_tup1<T: Copy>(t: (T,)) -> ((T,), (T,)) {
//~^ HELP consider restricting type parameter `T`
(t, t) //~ use of moved value: `t`
}
fn duplicate_tup2<A: Copy, B: Copy>(t: (A, B)) -> ((A, B), (A, B)) {
//~^ HELP consider restricting type parameters
(t, t) //~ use of moved value: `t`
}
fn duplicate_custom<T: Trait + Copy>(t: S<T>) -> (S<T>, S<T>) {
//~^ HELP consider restricting type parameter `T`
(t, t) //~ use of moved value: `t`
}
struct S<T>(T);
trait Trait {}
impl<T: Trait + Clone> Clone for S<T> {
fn clone(&self) -> Self {
Self(self.0.clone())
}
}
impl<T: Trait + Copy> Copy for S<T> {}
trait A {}
trait B {}
// Test where bounds are added with different bound placements
fn duplicate_custom_1<T: Trait + Copy>(t: S<T>) -> (S<T>, S<T>) where {
//~^ HELP consider restricting type parameter `T`
(t, t) //~ use of moved value: `t`
}
fn duplicate_custom_2<T>(t: S<T>) -> (S<T>, S<T>)
where
T: A + Trait + Copy,
//~^ HELP consider further restricting this bound
{
(t, t) //~ use of moved value: `t`
}
fn duplicate_custom_3<T>(t: S<T>) -> (S<T>, S<T>)
where
T: A,
T: B, T: Trait, T: Copy
//~^ HELP consider further restricting type parameter `T`
{
(t, t) //~ use of moved value: `t`
}
fn duplicate_custom_4<T: A>(t: S<T>) -> (S<T>, S<T>)
where
T: B + Trait + Copy,
//~^ HELP consider further restricting this bound
{
(t, t) //~ use of moved value: `t`
}
fn main() {}

72
src/test/ui/moves/use_of_moved_value_copy_suggestions.rs Normal file
View file

@ -0,0 +1,72 @@
// run-rustfix
#![allow(dead_code)]
fn duplicate_t<T>(t: T) -> (T, T) {
//~^ HELP consider restricting type parameter `T`
(t, t) //~ use of moved value: `t`
}
fn duplicate_opt<T>(t: Option<T>) -> (Option<T>, Option<T>) {
//~^ HELP consider restricting type parameter `T`
(t, t) //~ use of moved value: `t`
}
fn duplicate_tup1<T>(t: (T,)) -> ((T,), (T,)) {
//~^ HELP consider restricting type parameter `T`
(t, t) //~ use of moved value: `t`
}
fn duplicate_tup2<A, B>(t: (A, B)) -> ((A, B), (A, B)) {
//~^ HELP consider restricting type parameters
(t, t) //~ use of moved value: `t`
}
fn duplicate_custom<T>(t: S<T>) -> (S<T>, S<T>) {
//~^ HELP consider restricting type parameter `T`
(t, t) //~ use of moved value: `t`
}
struct S<T>(T);
trait Trait {}
impl<T: Trait + Clone> Clone for S<T> {
fn clone(&self) -> Self {
Self(self.0.clone())
}
}
impl<T: Trait + Copy> Copy for S<T> {}
trait A {}
trait B {}
// Test where bounds are added with different bound placements
fn duplicate_custom_1<T>(t: S<T>) -> (S<T>, S<T>) where {
//~^ HELP consider restricting type parameter `T`
(t, t) //~ use of moved value: `t`
}
fn duplicate_custom_2<T>(t: S<T>) -> (S<T>, S<T>)
where
T: A,
//~^ HELP consider further restricting this bound
{
(t, t) //~ use of moved value: `t`
}
fn duplicate_custom_3<T>(t: S<T>) -> (S<T>, S<T>)
where
T: A,
T: B,
//~^ HELP consider further restricting type parameter `T`
{
(t, t) //~ use of moved value: `t`
}
fn duplicate_custom_4<T: A>(t: S<T>) -> (S<T>, S<T>)
where
T: B,
//~^ HELP consider further restricting this bound
{
(t, t) //~ use of moved value: `t`
}
fn main() {}

147
src/test/ui/moves/use_of_moved_value_copy_suggestions.stderr Normal file
View file

@ -0,0 +1,147 @@
error[E0382]: use of moved value: `t`
--> $DIR/use_of_moved_value_copy_suggestions.rs:6:9
|
LL | fn duplicate_t<T>(t: T) -> (T, T) {
| - move occurs because `t` has type `T`, which does not implement the `Copy` trait
LL |
LL | (t, t)
| - ^ value used here after move
| |
| value moved here
|
help: consider restricting type parameter `T`
|
LL | fn duplicate_t<T: Copy>(t: T) -> (T, T) {
| ++++++
error[E0382]: use of moved value: `t`
--> $DIR/use_of_moved_value_copy_suggestions.rs:11:9
|
LL | fn duplicate_opt<T>(t: Option<T>) -> (Option<T>, Option<T>) {
| - move occurs because `t` has type `Option<T>`, which does not implement the `Copy` trait
LL |
LL | (t, t)
| - ^ value used here after move
| |
| value moved here
|
help: consider restricting type parameter `T`
|
LL | fn duplicate_opt<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) {
| ++++++
error[E0382]: use of moved value: `t`
--> $DIR/use_of_moved_value_copy_suggestions.rs:16:9
|
LL | fn duplicate_tup1<T>(t: (T,)) -> ((T,), (T,)) {
| - move occurs because `t` has type `(T,)`, which does not implement the `Copy` trait
LL |
LL | (t, t)
| - ^ value used here after move
| |
| value moved here
|
help: consider restricting type parameter `T`
|
LL | fn duplicate_tup1<T: Copy>(t: (T,)) -> ((T,), (T,)) {
| ++++++
error[E0382]: use of moved value: `t`
--> $DIR/use_of_moved_value_copy_suggestions.rs:21:9
|
LL | fn duplicate_tup2<A, B>(t: (A, B)) -> ((A, B), (A, B)) {
| - move occurs because `t` has type `(A, B)`, which does not implement the `Copy` trait
LL |
LL | (t, t)
| - ^ value used here after move
| |
| value moved here
|
help: consider restricting type parameters
|
LL | fn duplicate_tup2<A: Copy, B: Copy>(t: (A, B)) -> ((A, B), (A, B)) {
| ++++++ ++++++
error[E0382]: use of moved value: `t`
--> $DIR/use_of_moved_value_copy_suggestions.rs:26:9
|
LL | fn duplicate_custom<T>(t: S<T>) -> (S<T>, S<T>) {
| - move occurs because `t` has type `S<T>`, which does not implement the `Copy` trait
LL |
LL | (t, t)
| - ^ value used here after move
| |
| value moved here
|
help: consider restricting type parameter `T`
|
LL | fn duplicate_custom<T: Trait + Copy>(t: S<T>) -> (S<T>, S<T>) {
| ++++++++++++++
error[E0382]: use of moved value: `t`
--> $DIR/use_of_moved_value_copy_suggestions.rs:44:9
|
LL | fn duplicate_custom_1<T>(t: S<T>) -> (S<T>, S<T>) where {
| - move occurs because `t` has type `S<T>`, which does not implement the `Copy` trait
LL |
LL | (t, t)
| - ^ value used here after move
| |
| value moved here
|
help: consider restricting type parameter `T`
|
LL | fn duplicate_custom_1<T: Trait + Copy>(t: S<T>) -> (S<T>, S<T>) where {
| ++++++++++++++
error[E0382]: use of moved value: `t`
--> $DIR/use_of_moved_value_copy_suggestions.rs:52:9
|
LL | fn duplicate_custom_2<T>(t: S<T>) -> (S<T>, S<T>)
| - move occurs because `t` has type `S<T>`, which does not implement the `Copy` trait
...
LL | (t, t)
| - ^ value used here after move
| |
| value moved here
|
help: consider further restricting this bound
|
LL | T: A + Trait + Copy,
| ++++++++++++++
error[E0382]: use of moved value: `t`
--> $DIR/use_of_moved_value_copy_suggestions.rs:61:9
|
LL | fn duplicate_custom_3<T>(t: S<T>) -> (S<T>, S<T>)
| - move occurs because `t` has type `S<T>`, which does not implement the `Copy` trait
...
LL | (t, t)
| - ^ value used here after move
| |
| value moved here
|
help: consider further restricting type parameter `T`
|
LL | T: B, T: Trait, T: Copy
| ~~~~~~~~~~~~~~~~~~~
error[E0382]: use of moved value: `t`
--> $DIR/use_of_moved_value_copy_suggestions.rs:69:9
|
LL | fn duplicate_custom_4<T: A>(t: S<T>) -> (S<T>, S<T>)
| - move occurs because `t` has type `S<T>`, which does not implement the `Copy` trait
...
LL | (t, t)
| - ^ value used here after move
| |
| value moved here
|
help: consider further restricting this bound
|
LL | T: B + Trait + Copy,
| ++++++++++++++
error: aborting due to 9 previous errors
For more information about this error, try `rustc --explain E0382`.