Skip to content

Commit

Permalink
Rollup merge of #66253 - ohadravid:improve-errors-after-re-rebalance-…
Browse files Browse the repository at this point in the history
…coherence, r=estebank

Improve errors after re rebalance coherence

Following #65247, I noticed that some error messages should be updated to reflect the changes of `re_rebalance_coherence` (also there was a [note](https://rust-lang.github.io/rfcs/2451-re-rebalancing-coherence.html#teaching-users) in the RFC about it).

First, error message `E0210` was updated to match the RFC, and I also tried to improve a little the error when the "order" of types is problematic.

For code like this:
```
#![feature(re_rebalance_coherence)] // Now stable

struct Wrap<T>(T);

impl<T> From<Wrap<T>> for T {
    fn from(x: Wrap<T>) -> T {
        x.0
    }
}
```

The old error was:
```
error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
 --> src/lib.rs:5:6
  |
5 | impl<T> From<Wrap<T>> for T {
  |      ^ type parameter `T` must be used as the type parameter for some local type
  |
  = note: only traits defined in the current crate can be implemented for a type parameter

```

and the new error is:
```
error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Wrap<T>`)
  --> main.rs:66:6
   |
66 | impl<T> From<Wrap<T>> for T {
   |      ^ type parameter `T` must be covered by another type when it appears before the first local type (`Wrap<T>`)
   |
   = note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local, and no uncovered type parameters appear before that first local type
   = note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last
```

I tried to point at the uncovered `T`, but couldn't get something which was reliable (but I'll be happy to try if someone points me in the right direction).

r? @estebank
cc @nikomatsakis

Fixes #65247
  • Loading branch information
JohnTitor authored Nov 14, 2019
2 parents 3f07f1c + 2db744c commit 79e2afc
Show file tree
Hide file tree
Showing 25 changed files with 122 additions and 49 deletions.
12 changes: 10 additions & 2 deletions src/librustc/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ pub fn trait_ref_is_local_or_fundamental<'tcx>(

pub enum OrphanCheckErr<'tcx> {
NonLocalInputType(Vec<(Ty<'tcx>, bool /* Is this the first input type? */)>),
UncoveredTy(Ty<'tcx>),
UncoveredTy(Ty<'tcx>, Option<Ty<'tcx>>),
}

/// Checks the coherence orphan rules. `impl_def_id` should be the
Expand Down Expand Up @@ -402,7 +402,15 @@ fn orphan_check_trait_ref<'tcx>(
return Ok(());
} else if let ty::Param(_) = input_ty.kind {
debug!("orphan_check_trait_ref: uncovered ty: `{:?}`", input_ty);
return Err(OrphanCheckErr::UncoveredTy(input_ty))
let local_type = trait_ref
.input_types()
.flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate))
.filter(|ty| ty_is_non_local_constructor(tcx, ty, in_crate).is_none())
.next();

debug!("orphan_check_trait_ref: uncovered ty local_type: `{:?}`", local_type);

return Err(OrphanCheckErr::UncoveredTy(input_ty, local_type))
}
if let Some(non_local_tys) = non_local_tys {
for input_ty in non_local_tys {
Expand Down
64 changes: 46 additions & 18 deletions src/librustc_typeck/coherence/orphan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,30 +77,58 @@ impl ItemLikeVisitor<'v> for OrphanChecker<'tcx> {
err.emit();
return;
}
Err(traits::OrphanCheckErr::UncoveredTy(param_ty)) => {
Err(traits::OrphanCheckErr::UncoveredTy(param_ty, local_type)) => {
let mut sp = sp;
for param in &generics.params {
if param.name.ident().to_string() == param_ty.to_string() {
sp = param.span;
}
}
let mut err = struct_span_err!(
self.tcx.sess,
sp,
E0210,
"type parameter `{}` must be used as the type parameter for some local \
type (e.g., `MyStruct<{}>`)",
param_ty,
param_ty
);
err.span_label(sp, format!(
"type parameter `{}` must be used as the type parameter for some local \
type",
param_ty,
));
err.note("only traits defined in the current crate can be implemented for a \
type parameter");
err.emit();

match local_type {
Some(local_type) => {
struct_span_err!(
self.tcx.sess,
sp,
E0210,
"type parameter `{}` must be covered by another type \
when it appears before the first local type (`{}`)",
param_ty,
local_type
).span_label(sp, format!(
"type parameter `{}` must be covered by another type \
when it appears before the first local type (`{}`)",
param_ty,
local_type
)).note("implementing a foreign trait is only possible if at \
least one of the types for which is it implemented is local, \
and no uncovered type parameters appear before that first \
local type"
).note("in this case, 'before' refers to the following order: \
`impl<..> ForeignTrait<T1, ..., Tn> for T0`, \
where `T0` is the first and `Tn` is the last"
).emit();
}
None => {
struct_span_err!(
self.tcx.sess,
sp,
E0210,
"type parameter `{}` must be used as the type parameter for some \
local type (e.g., `MyStruct<{}>`)",
param_ty,
param_ty
).span_label(sp, format!(
"type parameter `{}` must be used as the type parameter for some \
local type",
param_ty,
)).note("implementing a foreign trait is only possible if at \
least one of the types for which is it implemented is local"
).note("only traits defined in the current crate can be \
implemented for a type parameter"
).emit();
}
};
return;
}
}
Expand Down
23 changes: 17 additions & 6 deletions src/librustc_typeck/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2114,8 +2114,13 @@ E0210: r##"
This error indicates a violation of one of Rust's orphan rules for trait
implementations. The rule concerns the use of type parameters in an
implementation of a foreign trait (a trait defined in another crate), and
states that type parameters must be "covered" by a local type. To understand
what this means, it is perhaps easiest to consider a few examples.
states that type parameters must be "covered" by a local type.
When implementing a foreign trait for a foreign type,
the trait must have one or more type parameters.
A type local to your crate must appear before any use of any type parameters.
To understand what this means, it is perhaps easier to consider a few examples.
If `ForeignTrait` is a trait defined in some external crate `foo`, then the
following trait `impl` is an error:
Expand Down Expand Up @@ -2173,12 +2178,18 @@ impl<P1, ..., Pm> ForeignTrait<T1, ..., Tn> for T0 { ... }
where `P1, ..., Pm` are the type parameters of the `impl` and `T0, ..., Tn`
are types. One of the types `T0, ..., Tn` must be a local type (this is another
orphan rule, see the explanation for E0117). Let `i` be the smallest integer
such that `Ti` is a local type. Then no type parameter can appear in any of the
`Tj` for `j < i`.
orphan rule, see the explanation for E0117).
For information on the design of the orphan rules, see [RFC 1023].
Both of the following must be true:
1. At least one of the types `T0..=Tn` must be a local type.
Let `Ti` be the first such type.
2. No uncovered type parameters `P1..=Pm` may appear in `T0..Ti`
(excluding `Ti`).
For information on the design of the orphan rules,
see [RFC 2451] and [RFC 1023].
[RFC 2451]: https://rust-lang.github.io/rfcs/2451-re-rebalancing-coherence.html
[RFC 1023]: https://github.com/rust-lang/rfcs/blob/master/text/1023-rebalancing-coherence.md
"##,

Expand Down
1 change: 1 addition & 0 deletions src/test/ui/coherence/coherence-all-remote.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ error[E0210]: type parameter `T` must be used as the type parameter for some loc
LL | impl<T> Remote1<T> for isize { }
| ^ type parameter `T` must be used as the type parameter for some local type
|
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local
= note: only traits defined in the current crate can be implemented for a type parameter

error: aborting due to previous error
Expand Down
7 changes: 4 additions & 3 deletions src/test/ui/coherence/coherence-bigint-param.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`BigInt`)
--> $DIR/coherence-bigint-param.rs:8:6
|
LL | impl<T> Remote1<BigInt> for T { }
| ^ type parameter `T` must be used as the type parameter for some local type
| ^ type parameter `T` must be covered by another type when it appears before the first local type (`BigInt`)
|
= note: only traits defined in the current crate can be implemented for a type parameter
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local, and no uncovered type parameters appear before that first local type
= note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last

error: aborting due to previous error

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ error[E0210]: type parameter `A` must be used as the type parameter for some loc
LL | impl<A> Foo for A {
| ^ type parameter `A` must be used as the type parameter for some local type
|
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local
= note: only traits defined in the current crate can be implemented for a type parameter

error: aborting due to 2 previous errors
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/coherence/coherence-lone-type-parameter.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ error[E0210]: type parameter `T` must be used as the type parameter for some loc
LL | impl<T> Remote for T { }
| ^ type parameter `T` must be used as the type parameter for some local type
|
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local
= note: only traits defined in the current crate can be implemented for a type parameter

error: aborting due to previous error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ error[E0210]: type parameter `T` must be used as the type parameter for some loc
LL | impl<T> Remote for Box<T> {
| ^ type parameter `T` must be used as the type parameter for some local type
|
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local
= note: only traits defined in the current crate can be implemented for a type parameter

error: aborting due to previous error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ error[E0210]: type parameter `T` must be used as the type parameter for some loc
LL | impl<T> Remote1<u32> for Box<T> {
| ^ type parameter `T` must be used as the type parameter for some local type
|
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local
= note: only traits defined in the current crate can be implemented for a type parameter

error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
Expand All @@ -12,6 +13,7 @@ error[E0210]: type parameter `T` must be used as the type parameter for some loc
LL | impl<'a, T> Remote1<u32> for &'a T {
| ^ type parameter `T` must be used as the type parameter for some local type
|
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local
= note: only traits defined in the current crate can be implemented for a type parameter

error: aborting due to 2 previous errors
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ error[E0210]: type parameter `T` must be used as the type parameter for some loc
LL | impl<T> Remote1<u32> for T {
| ^ type parameter `T` must be used as the type parameter for some local type
|
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local
= note: only traits defined in the current crate can be implemented for a type parameter

error: aborting due to previous error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ error[E0210]: type parameter `T` must be used as the type parameter for some loc
LL | impl<T> Remote1<Box<T>> for u32 {
| ^ type parameter `T` must be used as the type parameter for some local type
|
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local
= note: only traits defined in the current crate can be implemented for a type parameter

error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
Expand All @@ -12,6 +13,7 @@ error[E0210]: type parameter `T` must be used as the type parameter for some loc
LL | impl<'a, T> Remote1<&'a T> for u32 {
| ^ type parameter `T` must be used as the type parameter for some local type
|
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local
= note: only traits defined in the current crate can be implemented for a type parameter

error: aborting due to 2 previous errors
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ error[E0210]: type parameter `T` must be used as the type parameter for some loc
LL | impl<'a, T> Remote1<Box<T>> for &'a T {
| ^ type parameter `T` must be used as the type parameter for some local type
|
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local
= note: only traits defined in the current crate can be implemented for a type parameter

error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
Expand All @@ -12,6 +13,7 @@ error[E0210]: type parameter `T` must be used as the type parameter for some loc
LL | impl<'a, T> Remote1<&'a T> for Box<T> {
| ^ type parameter `T` must be used as the type parameter for some local type
|
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local
= note: only traits defined in the current crate can be implemented for a type parameter

error: aborting due to 2 previous errors
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ error[E0210]: type parameter `T` must be used as the type parameter for some loc
LL | impl<T> Remote1<Box<T>> for T {
| ^ type parameter `T` must be used as the type parameter for some local type
|
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local
= note: only traits defined in the current crate can be implemented for a type parameter

error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
Expand All @@ -12,6 +13,7 @@ error[E0210]: type parameter `T` must be used as the type parameter for some loc
LL | impl<'a, T> Remote1<&'a T> for T {
| ^ type parameter `T` must be used as the type parameter for some local type
|
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local
= note: only traits defined in the current crate can be implemented for a type parameter

error: aborting due to 2 previous errors
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ use std::rc::Rc;
struct Local;

impl<T> Remote2<Box<T>, Local> for u32 {
//~^ ERROR type parameter `T` must be used as the type parameter for some local type
//~^ ERROR type parameter `T` must be covered by another type
}

impl<'a, T> Remote2<&'a T, Local> for u32 {
//~^ ERROR type parameter `T` must be used as the type parameter for some local type
//~^ ERROR type parameter `T` must be covered by another type
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
--> $DIR/impl[t]-foreign[fundamental[t]_local]-for-foreign.rs:10:6
|
LL | impl<T> Remote2<Box<T>, Local> for u32 {
| ^ type parameter `T` must be used as the type parameter for some local type
| ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
|
= note: only traits defined in the current crate can be implemented for a type parameter
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local, and no uncovered type parameters appear before that first local type
= note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last

error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
--> $DIR/impl[t]-foreign[fundamental[t]_local]-for-foreign.rs:14:10
|
LL | impl<'a, T> Remote2<&'a T, Local> for u32 {
| ^ type parameter `T` must be used as the type parameter for some local type
| ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
|
= note: only traits defined in the current crate can be implemented for a type parameter
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local, and no uncovered type parameters appear before that first local type
= note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last

error: aborting due to 2 previous errors

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ use std::rc::Rc;
struct Local;

impl<T> Remote1<Local> for Box<T> {
//~^ ERROR type parameter `T` must be used as the type parameter for some local type
//~^ ERROR type parameter `T` must be covered by another type
}

impl<T> Remote1<Local> for &T {
//~^ ERROR type parameter `T` must be used as the type parameter for some local type
//~^ ERROR type parameter `T` must be covered by another type
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
--> $DIR/impl[t]-foreign[local]-for-fundamental[t].rs:10:6
|
LL | impl<T> Remote1<Local> for Box<T> {
| ^ type parameter `T` must be used as the type parameter for some local type
| ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
|
= note: only traits defined in the current crate can be implemented for a type parameter
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local, and no uncovered type parameters appear before that first local type
= note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last

error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
--> $DIR/impl[t]-foreign[local]-for-fundamental[t].rs:14:6
|
LL | impl<T> Remote1<Local> for &T {
| ^ type parameter `T` must be used as the type parameter for some local type
| ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
|
= note: only traits defined in the current crate can be implemented for a type parameter
= note: implementing a foreign trait is only possible if at least one of the types for which is it implemented is local, and no uncovered type parameters appear before that first local type
= note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last

error: aborting due to 2 previous errors

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/coherence/impl[t]-foreign[local]-for-t.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::rc::Rc;
struct Local;

impl<T> Remote1<Local> for T {
//~^ ERROR type parameter `T` must be used as the type parameter for some local type
//~^ ERROR type parameter `T` must be covered by another type
}

fn main() {}
Loading

0 comments on commit 79e2afc

Please sign in to comment.