Skip to content

Improve error message for impl Self #95041

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

Closed
Closed
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
18 changes: 18 additions & 0 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,9 @@ struct DiagnosticMetadata<'ast> {
/// Used to detect possible `if let` written without `let` and to provide structured suggestion.
in_if_condition: Option<&'ast Expr>,

/// When processing a impl self type and encountering a `Self`, suggest not using it.
currently_processing_impl_self_ty: bool,

/// If we are currently in a trait object definition. Used to point at the bounds when
/// encountering a struct or enum.
current_trait_object: Option<&'ast [ast::GenericBound]>,
Expand Down Expand Up @@ -1318,7 +1321,12 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
visit::walk_trait_ref(this, trait_ref);
}
// Resolve the self type.
let prev = replace(
&mut this.diagnostic_metadata.currently_processing_impl_self_ty,
true,
);
this.visit_ty(self_type);
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if we visit self_type outside of the call to with_self_rib(res,?

Copy link
Member Author

Choose a reason for hiding this comment

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

If we do, visit_ty just visits self_ty unless currently_processing_impl_self_ty is true. It never causes a Self is only available in impls, traits, and type definitions error.

In reference to #93971 (review), should I improve this error to report it right away?

this.diagnostic_metadata.currently_processing_impl_self_ty = prev;
// Resolve the generic parameters.
this.visit_generics(generics);
// Resolve the items within the impl.
Expand Down Expand Up @@ -2038,6 +2046,16 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
Ok(Some(partial_res)) if partial_res.unresolved_segments() == 0 => {
if source.is_expected(partial_res.base_res()) || partial_res.base_res() == Res::Err
{
if self.diagnostic_metadata.currently_processing_impl_self_ty {
if let Res::SelfTy { .. } = partial_res.base_res() {
if path.len() == 1 && path[0].ident.name == kw::SelfUpper {
self.r.session.span_err(
span,
"`Self` is only available in impls, traits, and type definitions",
);
}
}
}
partial_res
} else {
report_errors(self, Some(partial_res.base_res()))
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/resolve/issue-23305.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ pub trait ToNbt<T> {

impl dyn ToNbt<Self> {}
//~^ ERROR cycle detected
//~| ERROR `Self` is only available in impls, traits, and type definitions

fn main() {}
8 changes: 7 additions & 1 deletion src/test/ui/resolve/issue-23305.stderr
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
error: `Self` is only available in impls, traits, and type definitions
--> $DIR/issue-23305.rs:5:16
|
LL | impl dyn ToNbt<Self> {}
| ^^^^

error[E0391]: cycle detected when computing type of `<impl at $DIR/issue-23305.rs:5:1: 5:24>`
--> $DIR/issue-23305.rs:5:16
|
Expand All @@ -11,6 +17,6 @@ note: cycle used when collecting item types in top-level module
LL | pub trait ToNbt<T> {
| ^^^^^^^^^^^^^^^^^^

error: aborting due to previous error
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0391`.
4 changes: 4 additions & 0 deletions src/test/ui/resolve/resolve-self-in-impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@ impl Tr for S where S<Self>: Copy {} // OK
impl Tr for S where Self::A: Copy {} // OK

impl Tr for Self {} //~ ERROR cycle detected
//~^ ERROR `Self` is only available in impls, traits, and type definitions
Copy link
Member

Choose a reason for hiding this comment

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

We should emit a single diagnostic for these, not multiple. I could see

Self is only available in impls, traits, and type definitions

being a “help”, but I think it being a singular error makes more sense.

Copy link
Member Author

@TaKO8Ki TaKO8Ki Mar 22, 2022

Choose a reason for hiding this comment

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

Ok. I'll fix it.

impl Tr for S<Self> {} //~ ERROR cycle detected
//~^ ERROR `Self` is only available in impls, traits, and type definitions
impl Self {} //~ ERROR cycle detected
//~^ ERROR `Self` is only available in impls, traits, and type definitions
impl S<Self> {} //~ ERROR cycle detected
//~^ ERROR `Self` is only available in impls, traits, and type definitions
impl Tr<Self::A> for S {} //~ ERROR cycle detected

fn main() {}
50 changes: 37 additions & 13 deletions src/test/ui/resolve/resolve-self-in-impl.stderr
Original file line number Diff line number Diff line change
@@ -1,3 +1,27 @@
error: `Self` is only available in impls, traits, and type definitions
--> $DIR/resolve-self-in-impl.rs:14:13
|
LL | impl Tr for Self {}
| ^^^^

error: `Self` is only available in impls, traits, and type definitions
--> $DIR/resolve-self-in-impl.rs:16:15
|
LL | impl Tr for S<Self> {}
| ^^^^

error: `Self` is only available in impls, traits, and type definitions
--> $DIR/resolve-self-in-impl.rs:18:6
|
LL | impl Self {}
| ^^^^

error: `Self` is only available in impls, traits, and type definitions
--> $DIR/resolve-self-in-impl.rs:20:8
|
LL | impl S<Self> {}
| ^^^^

error[E0391]: cycle detected when computing type of `<impl at $DIR/resolve-self-in-impl.rs:14:1: 14:20>`
--> $DIR/resolve-self-in-impl.rs:14:13
|
Expand All @@ -17,13 +41,13 @@ LL | |
LL | | fn main() {}
| |____________^

error[E0391]: cycle detected when computing type of `<impl at $DIR/resolve-self-in-impl.rs:15:1: 15:23>`
--> $DIR/resolve-self-in-impl.rs:15:15
error[E0391]: cycle detected when computing type of `<impl at $DIR/resolve-self-in-impl.rs:16:1: 16:23>`
--> $DIR/resolve-self-in-impl.rs:16:15
|
LL | impl Tr for S<Self> {}
| ^^^^
|
= note: ...which immediately requires computing type of `<impl at $DIR/resolve-self-in-impl.rs:15:1: 15:23>` again
= note: ...which immediately requires computing type of `<impl at $DIR/resolve-self-in-impl.rs:16:1: 16:23>` again
note: cycle used when collecting item types in top-level module
--> $DIR/resolve-self-in-impl.rs:1:1
|
Expand All @@ -36,13 +60,13 @@ LL | |
LL | | fn main() {}
| |____________^

error[E0391]: cycle detected when computing type of `<impl at $DIR/resolve-self-in-impl.rs:16:1: 16:13>`
--> $DIR/resolve-self-in-impl.rs:16:6
error[E0391]: cycle detected when computing type of `<impl at $DIR/resolve-self-in-impl.rs:18:1: 18:13>`
--> $DIR/resolve-self-in-impl.rs:18:6
|
LL | impl Self {}
| ^^^^
|
= note: ...which immediately requires computing type of `<impl at $DIR/resolve-self-in-impl.rs:16:1: 16:13>` again
= note: ...which immediately requires computing type of `<impl at $DIR/resolve-self-in-impl.rs:18:1: 18:13>` again
note: cycle used when collecting item types in top-level module
--> $DIR/resolve-self-in-impl.rs:1:1
|
Expand All @@ -55,13 +79,13 @@ LL | |
LL | | fn main() {}
| |____________^

error[E0391]: cycle detected when computing type of `<impl at $DIR/resolve-self-in-impl.rs:17:1: 17:16>`
--> $DIR/resolve-self-in-impl.rs:17:8
error[E0391]: cycle detected when computing type of `<impl at $DIR/resolve-self-in-impl.rs:20:1: 20:16>`
--> $DIR/resolve-self-in-impl.rs:20:8
|
LL | impl S<Self> {}
| ^^^^
|
= note: ...which immediately requires computing type of `<impl at $DIR/resolve-self-in-impl.rs:17:1: 17:16>` again
= note: ...which immediately requires computing type of `<impl at $DIR/resolve-self-in-impl.rs:20:1: 20:16>` again
note: cycle used when collecting item types in top-level module
--> $DIR/resolve-self-in-impl.rs:1:1
|
Expand All @@ -74,13 +98,13 @@ LL | |
LL | | fn main() {}
| |____________^

error[E0391]: cycle detected when computing trait implemented by `<impl at $DIR/resolve-self-in-impl.rs:18:1: 18:26>`
--> $DIR/resolve-self-in-impl.rs:18:1
error[E0391]: cycle detected when computing trait implemented by `<impl at $DIR/resolve-self-in-impl.rs:22:1: 22:26>`
--> $DIR/resolve-self-in-impl.rs:22:1
|
LL | impl Tr<Self::A> for S {}
| ^^^^^^^^^^^^^^^^^^^^^^
|
= note: ...which immediately requires computing trait implemented by `<impl at $DIR/resolve-self-in-impl.rs:18:1: 18:26>` again
= note: ...which immediately requires computing trait implemented by `<impl at $DIR/resolve-self-in-impl.rs:22:1: 22:26>` again
note: cycle used when collecting item types in top-level module
--> $DIR/resolve-self-in-impl.rs:1:1
|
Expand All @@ -93,6 +117,6 @@ LL | |
LL | | fn main() {}
| |____________^

error: aborting due to 5 previous errors
error: aborting due to 9 previous errors

For more information about this error, try `rustc --explain E0391`.