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

Refer to "self type" instead of "receiver type" #64110

Merged
merged 7 commits into from
Sep 5, 2019
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
2 changes: 1 addition & 1 deletion src/librustc/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Generally, `Self: Sized` is used to indicate that the trait should not be used
as a trait object. If the trait comes from your own crate, consider removing
this restriction.

### Method references the `Self` type in its arguments or return type
### Method references the `Self` type in its parameters or return type

This happens when a trait has a method like the following:

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1627,7 +1627,7 @@ impl<'tcx> ObligationCause<'tcx> {
MainFunctionType => Error0580("main function has wrong type"),
StartFunctionType => Error0308("start function has wrong type"),
IntrinsicType => Error0308("intrinsic has wrong type"),
MethodReceiver => Error0308("mismatched method receiver"),
MethodReceiver => Error0308("mismatched `self` parameter type"),

// In the case where we have no more specific thing to
// say, also take a look at the error code, maybe we can
Expand Down
5 changes: 4 additions & 1 deletion src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1384,7 +1384,10 @@ impl<'tcx> TyCtxt<'tcx> {
let mut reported_violations = FxHashSet::default();
for violation in violations {
if reported_violations.insert(violation.clone()) {
err.note(&violation.error_msg());
match violation.span() {
Some(span) => err.span_label(span, violation.error_msg()),
None => err.note(&violation.error_msg()),
};
}
}
Some(err)
Expand Down
72 changes: 44 additions & 28 deletions src/librustc/traits/object_safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use std::borrow::Cow;
use std::iter::{self};
use syntax::ast::{self};
use syntax::symbol::InternedString;
use syntax_pos::Span;
use syntax_pos::{Span, DUMMY_SP};

#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub enum ObjectSafetyViolation {
Expand All @@ -32,10 +32,10 @@ pub enum ObjectSafetyViolation {
SupertraitSelf,

/// Method has something illegal.
Method(ast::Name, MethodViolationCode),
Method(ast::Name, MethodViolationCode, Span),

/// Associated const.
AssocConst(ast::Name),
AssocConst(ast::Name, Span),
}

impl ObjectSafetyViolation {
Expand All @@ -46,22 +46,35 @@ impl ObjectSafetyViolation {
ObjectSafetyViolation::SupertraitSelf =>
"the trait cannot use `Self` as a type parameter \
in the supertraits or where-clauses".into(),
ObjectSafetyViolation::Method(name, MethodViolationCode::StaticMethod) =>
format!("method `{}` has no receiver", name).into(),
ObjectSafetyViolation::Method(name, MethodViolationCode::ReferencesSelf) =>
format!("method `{}` references the `Self` type \
in its arguments or return type", name).into(),
ObjectSafetyViolation::Method(name,
MethodViolationCode::WhereClauseReferencesSelf(_)) =>
format!("method `{}` references the `Self` type in where clauses", name).into(),
ObjectSafetyViolation::Method(name, MethodViolationCode::Generic) =>
ObjectSafetyViolation::Method(name, MethodViolationCode::StaticMethod, _) =>
format!("associated function `{}` has no `self` parameter", name).into(),
ObjectSafetyViolation::Method(name, MethodViolationCode::ReferencesSelf, _) => format!(
"method `{}` references the `Self` type in its parameters or return type",
name,
).into(),
ObjectSafetyViolation::Method(
name,
MethodViolationCode::WhereClauseReferencesSelf,
_,
) => format!("method `{}` references the `Self` type in where clauses", name).into(),
Centril marked this conversation as resolved.
Show resolved Hide resolved
ObjectSafetyViolation::Method(name, MethodViolationCode::Generic, _) =>
format!("method `{}` has generic type parameters", name).into(),
ObjectSafetyViolation::Method(name, MethodViolationCode::UndispatchableReceiver) =>
format!("method `{}`'s receiver cannot be dispatched on", name).into(),
ObjectSafetyViolation::AssocConst(name) =>
ObjectSafetyViolation::Method(name, MethodViolationCode::UndispatchableReceiver, _) =>
format!("method `{}`'s `self` parameter cannot be dispatched on", name).into(),
ObjectSafetyViolation::AssocConst(name, _) =>
format!("the trait cannot contain associated consts like `{}`", name).into(),
}
}

pub fn span(&self) -> Option<Span> {
// When `span` comes from a separate crate, it'll be `DUMMY_SP`. Treat it as `None` so
// diagnostics use a `note` instead of a `span_label`.
match *self {
ObjectSafetyViolation::AssocConst(_, span) |
ObjectSafetyViolation::Method(_, _, span) if span != DUMMY_SP => Some(span),
estebank marked this conversation as resolved.
Show resolved Hide resolved
_ => None,
}
}
}

/// Reasons a method might not be object-safe.
Expand All @@ -74,7 +87,7 @@ pub enum MethodViolationCode {
ReferencesSelf,

/// e.g., `fn foo(&self) where Self: Clone`
WhereClauseReferencesSelf(Span),
WhereClauseReferencesSelf,

/// e.g., `fn foo<A>()`
Generic,
Expand All @@ -88,9 +101,10 @@ impl<'tcx> TyCtxt<'tcx> {
/// astconv -- currently, `Self` in supertraits. This is needed
/// because `object_safety_violations` can't be used during
/// type collection.
pub fn astconv_object_safety_violations(self, trait_def_id: DefId)
-> Vec<ObjectSafetyViolation>
{
pub fn astconv_object_safety_violations(
self,
trait_def_id: DefId,
) -> Vec<ObjectSafetyViolation> {
debug_assert!(self.generics_of(trait_def_id).has_self);
let violations = traits::supertrait_def_ids(self, trait_def_id)
.filter(|&def_id| self.predicates_reference_self(def_id, true))
Expand Down Expand Up @@ -128,7 +142,7 @@ impl<'tcx> TyCtxt<'tcx> {
}

match self.virtual_call_violation_for_method(trait_def_id, method) {
None | Some(MethodViolationCode::WhereClauseReferencesSelf(_)) => true,
None | Some(MethodViolationCode::WhereClauseReferencesSelf) => true,
Some(_) => false,
}
}
Expand All @@ -138,12 +152,15 @@ impl<'tcx> TyCtxt<'tcx> {
let mut violations: Vec<_> = self.associated_items(trait_def_id)
.filter(|item| item.kind == ty::AssocKind::Method)
.filter_map(|item|
self.object_safety_violation_for_method(trait_def_id, &item)
.map(|code| ObjectSafetyViolation::Method(item.ident.name, code))
self.object_safety_violation_for_method(trait_def_id, &item).map(|code| {
ObjectSafetyViolation::Method(item.ident.name, code, item.ident.span)
})
).filter(|violation| {
if let ObjectSafetyViolation::Method(_,
MethodViolationCode::WhereClauseReferencesSelf(span)) = violation
{
if let ObjectSafetyViolation::Method(
_,
MethodViolationCode::WhereClauseReferencesSelf,
span,
) = violation {
// Using `CRATE_NODE_ID` is wrong, but it's hard to get a more precise id.
// It's also hard to get a use site span, so we use the method definition span.
self.lint_node_note(
Expand All @@ -169,7 +186,7 @@ impl<'tcx> TyCtxt<'tcx> {

violations.extend(self.associated_items(trait_def_id)
.filter(|item| item.kind == ty::AssocKind::Const)
.map(|item| ObjectSafetyViolation::AssocConst(item.ident.name)));
.map(|item| ObjectSafetyViolation::AssocConst(item.ident.name, item.ident.span)));

debug!("object_safety_violations_for_trait(trait_def_id={:?}) = {:?}",
trait_def_id,
Expand Down Expand Up @@ -325,8 +342,7 @@ impl<'tcx> TyCtxt<'tcx> {
.visit_tys_shallow(|t| {
self.contains_illegal_self_type_reference(trait_def_id, t)
}) {
let span = self.def_span(method.def_id);
return Some(MethodViolationCode::WhereClauseReferencesSelf(span));
return Some(MethodViolationCode::WhereClauseReferencesSelf);
}

let receiver_ty = self.liberate_late_bound_regions(
Expand Down
36 changes: 18 additions & 18 deletions src/librustc_typeck/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -762,19 +762,19 @@ fn check_opaque_types<'fcx, 'tcx>(
substituted_predicates
}

const HELP_FOR_SELF_TYPE: &str =
"consider changing to `self`, `&self`, `&mut self`, `self: Box<Self>`, \
`self: Rc<Self>`, `self: Arc<Self>`, or `self: Pin<P>` (where P is one \
of the previous types except `Self`)";

fn check_method_receiver<'fcx, 'tcx>(
fcx: &FnCtxt<'fcx, 'tcx>,
method_sig: &hir::MethodSig,
method: &ty::AssocItem,
self_ty: Ty<'tcx>,
) {
const HELP_FOR_SELF_TYPE: &str =
"consider changing to `self`, `&self`, `&mut self`, `self: Box<Self>`, \
`self: Rc<Self>`, `self: Arc<Self>`, or `self: Pin<P>` (where P is one \
of the previous types except `Self`)";
// Check that the method has a valid receiver type, given the type `Self`.
debug!("check_method_receiver({:?}, self_ty={:?})",
method, self_ty);
debug!("check_method_receiver({:?}, self_ty={:?})", method, self_ty);

if !method.method_has_self_argument {
return;
Expand Down Expand Up @@ -805,12 +805,7 @@ fn check_method_receiver<'fcx, 'tcx>(
if fcx.tcx.features().arbitrary_self_types {
if !receiver_is_valid(fcx, span, receiver_ty, self_ty, true) {
// Report error; `arbitrary_self_types` was enabled.
fcx.tcx.sess.diagnostic().mut_span_err(
span, &format!("invalid method receiver type: {:?}", receiver_ty)
).note("type of `self` must be `Self` or a type that dereferences to it")
.help(HELP_FOR_SELF_TYPE)
.code(DiagnosticId::Error("E0307".into()))
.emit();
e0307(fcx, span, receiver_ty);
}
} else {
if !receiver_is_valid(fcx, span, receiver_ty, self_ty, false) {
Expand All @@ -830,17 +825,22 @@ fn check_method_receiver<'fcx, 'tcx>(
.emit();
} else {
// Report error; would not have worked with `arbitrary_self_types`.
fcx.tcx.sess.diagnostic().mut_span_err(
span, &format!("invalid method receiver type: {:?}", receiver_ty)
).note("type must be `Self` or a type that dereferences to it")
.help(HELP_FOR_SELF_TYPE)
.code(DiagnosticId::Error("E0307".into()))
.emit();
e0307(fcx, span, receiver_ty);
}
}
}
}

fn e0307(fcx: &FnCtxt<'fcx, 'tcx>, span: Span, receiver_ty: Ty<'_>) {
fcx.tcx.sess.diagnostic().mut_span_err(
span,
&format!("invalid `self` parameter type: {:?}", receiver_ty)
).note("type of `self` must be `Self` or a type that dereferences to it")
.help(HELP_FOR_SELF_TYPE)
.code(DiagnosticId::Error("E0307".into()))
.emit();
}

/// Returns whether `receiver_ty` would be considered a valid receiver type for `self_ty`. If
/// `arbitrary_self_types` is enabled, `receiver_ty` must transitively deref to `self_ty`, possibly
/// through a `*const/mut T` raw pointer. If the feature is not enabled, the requirements are more
Expand Down
84 changes: 82 additions & 2 deletions src/librustc_typeck/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ match string {
E0033: r##"
This error indicates that a pointer to a trait type cannot be implicitly
dereferenced by a pattern. Every trait defines a type, but because the
size of trait implementors isn't fixed, this type has no compile-time size.
size of trait implementers isn't fixed, this type has no compile-time size.
Therefore, all accesses to trait types must be through pointers. If you
encounter this error you should try to avoid dereferencing the pointer.

Expand Down Expand Up @@ -2425,6 +2425,87 @@ struct Bar<S, T> { x: Foo<S, T> }
```
"##,

E0307: r##"
This error indicates that the `self` parameter in a method has an invalid
"reciever type".

Methods take a special first parameter, of which there are three variants:
`self`, `&self`, and `&mut self`. These are syntactic sugar for
`self: Self`, `self: &Self`, and `self: &mut Self` respectively.

```
# struct Foo;
trait Trait {
fn foo(&self);
// ^^^^^ `self` here is a reference to the receiver object
}

impl Trait for Foo {
fn foo(&self) {}
// ^^^^^ the receiver type is `&Foo`
}
```

The type `Self` acts as an alias to the type of the current trait
implementer, or "receiver type". Besides the already mentioned `Self`,
`&Self` and `&mut Self` valid receiver types, the following are also valid:
`self: Box<Self>`, `self: Rc<Self>`, `self: Arc<Self>`, and `self: Pin<P>`
(where P is one of the previous types except `Self`). Note that `Self` can
also be the underlying implementing type, like `Foo` in the following
example:

```
# struct Foo;
# trait Trait {
# fn foo(&self);
# }
impl Trait for Foo {
fn foo(self: &Foo) {}
}
```

E0307 will be emitted by the compiler when using an invalid reciver type,
like in the following example:

```compile_fail,E0307
# struct Foo;
# struct Bar;
# trait Trait {
# fn foo(&self);
# }
impl Trait for Foo {
fn foo(self: &Bar) {}
}
```
estebank marked this conversation as resolved.
Show resolved Hide resolved

The nightly feature [Arbintrary self types][AST] extends the accepted
set of receiver types to also include any type that can dereference to
`Self`:

```
#![feature(arbitrary_self_types)]

struct Foo;
struct Bar;

// Because you can dereference `Bar` into `Foo`...
impl std::ops::Deref for Bar {
type Target = Foo;

fn deref(&self) -> &Foo {
&Foo
}
}

impl Foo {
fn foo(self: Bar) {}
// ^^^^^^^^^ ...it can be used as the receiver type
}
```

[AST]: https://doc.rust-lang.org/unstable-book/language-features/arbitrary-self-types.html
"##,

E0321: r##"
A cross-crate opt-out trait was implemented on something which wasn't a struct
or enum type. Erroneous code example:
Expand Down Expand Up @@ -4851,7 +4932,6 @@ register_diagnostics! {
// E0247,
// E0248, // value used as a type, now reported earlier during resolution as E0412
// E0249,
E0307, // invalid method `self` type
// E0319, // trait impls for defaulted traits allowed just for structs/enums
// E0372, // coherence not object safe
E0377, // the trait `CoerceUnsized` may only be implemented for a coercion
Expand Down
5 changes: 3 additions & 2 deletions src/test/ui/associated-const/associated-const-in-trait.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
error[E0038]: the trait `Trait` cannot be made into an object
--> $DIR/associated-const-in-trait.rs:9:6
|
LL | const N: usize;
| - the trait cannot contain associated consts like `N`
...
LL | impl dyn Trait {
| ^^^^^^^^^ the trait `Trait` cannot be made into an object
|
= note: the trait cannot contain associated consts like `N`

error: aborting due to previous error

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error[E0038]: the trait `NotObjectSafe` cannot be made into an object
--> $DIR/coherence-impl-trait-for-trait-object-safe.rs:11:6
|
LL | trait NotObjectSafe { fn eq(&self, other: Self); }
| -- method `eq` references the `Self` type in its parameters or return type
LL | impl NotObjectSafe for dyn NotObjectSafe { }
| ^^^^^^^^^^^^^ the trait `NotObjectSafe` cannot be made into an object
|
= note: method `eq` references the `Self` type in its arguments or return type

error: aborting due to previous error

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error[E0038]: the trait `NotObjectSafe` cannot be made into an object
--> $DIR/coherence-impl-trait-for-trait-object-safe.rs:11:6
|
LL | trait NotObjectSafe { fn eq(&self, other: Self); }
| -- method `eq` references the `Self` type in its parameters or return type
LL | impl NotObjectSafe for dyn NotObjectSafe { }
| ^^^^^^^^^^^^^ the trait `NotObjectSafe` cannot be made into an object
|
= note: method `eq` references the `Self` type in its arguments or return type

error: aborting due to previous error

Expand Down
5 changes: 3 additions & 2 deletions src/test/ui/did_you_mean/issue-40006.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ error[E0038]: the trait `X` cannot be made into an object
|
LL | impl dyn X {
| ^^^^^ the trait `X` cannot be made into an object
|
= note: method `xxx` has no receiver
...
LL | fn xxx() { ### }
| --- associated function `xxx` has no `self` parameter

error: aborting due to 9 previous errors

Expand Down
Loading