Skip to content

Make 'self lifetime illegal. #10897

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

Merged
merged 1 commit into from
Dec 11, 2013
Merged

Conversation

erik
Copy link
Contributor

@erik erik commented Dec 10, 2013

Also remove all instances of 'self within the codebase.

This fixes #10889.

To make reviewing easier the following files were modified with more than a dumb text replacement:

  • src/test/compile-fail/lifetime-no-keyword.rs
  • src/test/compile-fail/lifetime-obsoleted-self.rs
  • src/test/compile-fail/regions-free-region-ordering-incorrect.rs
  • src/libsyntax/parse/lexer.rs

@@ -765,8 +765,7 @@ fn next_token_inner(rdr: @mut StringReader) -> token::Token {
let tok = &token::IDENT(ident, false);

if token::is_any_keyword(tok)
&& !token::is_keyword(token::keywords::Static, tok)
&& !token::is_keyword(token::keywords::Self, tok) {
&& !token::is_keyword(token::keywords::Static, tok) {
Copy link
Member

Choose a reason for hiding this comment

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

Since 'self is currently so common, and lifetimes are sometimes touchy to get correct, I think it would be good to have an obsolete syntax warning here. That is, when tok is Self it prints something like obsolete syntax: invalid 'selflifetime; the restrictions have been removed, use any ident other thanself here..

Unfortunately our current mechanism for doing this (parse/obsolete.rs) requires a parser object, while this is too early. Others may have a better suggestion but maybe something like:

let mut msg = if token::is_keyword(token::keywords::Self, tok) {
    ~"obsolete syntax: ..."
} else {
    ~"invalid lifetime name"
};
fatal_span(rdr, start, rdr.last_pos, msg);

Copy link
Member

Choose a reason for hiding this comment

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

(I don't like my wording of that error message, btw; if someone could improve it, that would be nice.)

Copy link
Member

Choose a reason for hiding this comment

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

"lifetime name cannot be a reserved word" is fine.

@pcwalton
Copy link
Contributor

r? @nikomatsakis

Also remove all instances of 'self within the codebase.

This fixes rust-lang#10889.
bors added a commit that referenced this pull request Dec 11, 2013
Also remove all instances of 'self within the codebase.

This fixes #10889.

To make reviewing easier the following files were modified with more than a dumb text replacement:

- `src/test/compile-fail/lifetime-no-keyword.rs`
- `src/test/compile-fail/lifetime-obsoleted-self.rs`
- `src/test/compile-fail/regions-free-region-ordering-incorrect.rs`
- `src/libsyntax/parse/lexer.rs`
@bors bors closed this Dec 11, 2013
@bors bors merged commit 5731ca3 into rust-lang:master Dec 11, 2013
@erik erik deleted the remove-self-lifetime branch December 11, 2013 23:12
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 30, 2023
[`missing_fields_in_debug`]: don't ICE when self type is a generic param

Fixes rust-lang#10887

This PR fixes an ICE that happens when the implementor (self type) of a `Debug` impl is a generic parameter.
The lint calls `TyCtxt::type_of` with that self type, which ICEs when called with generic parameters, so this just adds a quick check before getting there to ignore them.

That can only happen inside of core itself (afaik) because the orphan rules forbid defining an impl such as `impl<T> Debug for T` outside of core, so I'm not sure how to add a test for this.
It seems like this impl in particular caused this: https://doc.rust-lang.org/stable/std/fmt/trait.Debug.html#impl-Debug-for-F

changelog: [`missing_fields_in_debug`]: don't ICE on blanket `Debug` impl in core
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 14, 2023
[`missing_fields_in_debug`]: make sure self type is an adt

Fixes rust-lang#11063, another ICE that can only happen in core.

This lint needs the `DefId` of the implementor to get its fields, but that ICEs if the implementor does not have a `DefId` (as is the case with primitive types, e.g. `impl Debug for bool`), which is where this ICE comes from.

This PR changes the check I added in rust-lang#10897 to be more... robust against `Debug` implementations we don't want to lint.
Instead of just checking if the self type is a type parameter and "special casing" one specific case we don't want to lint, we should probably rather just check that the self type is either a struct, an enum or a union and only then continue.
That prevents weird edge cases like this one that can only happen in core.

Again, I don't know if it's even possible to add a test case for this since one cannot implement `Debug` for primitive types outside of the crate that defined `Debug` (core).
I did make sure that this PR no longer ICEs on `impl<T> Debug for T` and `impl Debug for bool`.
Maybe writing such a test is possible with `#![no_core]` and then re-defining the `Debug` trait or something like that...?

changelog: [`missing_fields_in_debug`]: make sure self type is an adt (fixes an ICE in core)

r? `@Alexendoo` (reviewed the last PRs for this lint)
flip1995 pushed a commit to flip1995/rust that referenced this pull request Aug 17, 2023
[`missing_fields_in_debug`]: make sure self type is an adt

Fixes rust-lang#11063, another ICE that can only happen in core.

This lint needs the `DefId` of the implementor to get its fields, but that ICEs if the implementor does not have a `DefId` (as is the case with primitive types, e.g. `impl Debug for bool`), which is where this ICE comes from.

This PR changes the check I added in rust-lang#10897 to be more... robust against `Debug` implementations we don't want to lint.
Instead of just checking if the self type is a type parameter and "special casing" one specific case we don't want to lint, we should probably rather just check that the self type is either a struct, an enum or a union and only then continue.
That prevents weird edge cases like this one that can only happen in core.

Again, I don't know if it's even possible to add a test case for this since one cannot implement `Debug` for primitive types outside of the crate that defined `Debug` (core).
I did make sure that this PR no longer ICEs on `impl<T> Debug for T` and `impl Debug for bool`.
Maybe writing such a test is possible with `#![no_core]` and then re-defining the `Debug` trait or something like that...?

changelog: [`missing_fields_in_debug`]: make sure self type is an adt (fixes an ICE in core)

r? `@Alexendoo` (reviewed the last PRs for this lint)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove all uses of the 'self lifetime from the codebase and make it illegal to use self as a lifetime name
6 participants