Remove special handling of obsolete impl Trait for .. syntax
#121072
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
|
Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt |
|
Best reviewed one commit at a time. |
nnethercote
changed the title
impl Trait for .. syntax errors. ErrorGuaranteed to ast::TyKind::Err.
Noratrieb
reviewed
Feb 14, 2024
| @@ -588,15 +588,8 @@ impl<'a> Parser<'a> { | |||
| let has_for = self.eat_keyword(kw::For); | |||
| let missing_for_span = self.prev_token.span.between(self.token.span); | |||
|
|
|||
| let ty_second = if self.token == token::DotDot { | |||
| // We need to report this error after `cfg` expansion for compatibility reasons | |||
There was a problem hiding this comment.
This suggests that we intend the following (historically wrongly gated 😭) to pass:
#[cfg(dont)]
impl Trait for .. {}given that this should have emitted a future compat warning (from a quick test it doesn't seem to do that though????, I might be doing it wrong though)
but this is a breaking change (a probably justified one, but one)
maybe this should just use some dummy node in the meantime to not block this
This comment has been minimized.
This comment has been minimized.
This was referenced Feb 14, 2024
|
This is related to #65860. |
|
@bors try |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 14, 2024
Add an `ErrorGuaranteed` to `ast::TyKind::Err`. This makes it more like `hir::TyKind::Err`, and avoids a `span_delayed_bug` call in `LoweringContext::lower_ty_direct`. r? `@oli-obk`
|
☀️ Try build successful - checks-actions |
|
@craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
craterbot
added
S-waiting-on-crater
|
I don't want to hold up the |
nnethercote
changed the title
ErrorGuaranteed to ast::TyKind::Err.impl Trait for .. syntax
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
|
craterbot
added
S-waiting-on-review
|
The crater run has 77 failures, which boil down to three crates. crossbeamMost of the failures are from this crate, versions 0.2.9, 0.2.10, 0.3.0, which are all over six years old. Current version is 0.8.4. dimensionedWe see a few failures with version 0.6.0, which is seven years old. Current version is 0.8.0, which is two years old. The crate has 220,000 downloads for all time. packedWe see a couple of failures with version 0.4.2, which is the latest version, but over seven years old. The crate has 21,000 downloads for all time. |
nnethercote
force-pushed
the
TyKind-Err-guar
branch
from
3a97622 to
3a7d5f3
Compare
rustbot
added
A-testsuite
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #121400) made this pull request unmergeable. Please resolve the merge conflicts. |
The ancient (pre-1.0) RFC 19 suggested using `impl Trait for ..` syntax
for default traits. That was later changed to `auto trait Trait {}`
syntax. The parser has special treatment for the `..` syntax, suggesting
the `auto` syntax.
Given that default traits have not be stabilized and the `..` syntax is
so old, the special case seems unnecessary, and it gets in the way of
adding `ErrorGuaranteed` to `TyKind::Err`. This commit removes it and
the tests.
nnethercote
force-pushed
the
TyKind-Err-guar
branch
from
3a7d5f3 to
40f1be0
Compare
|
The failing line from #[cfg(feature = "nightly")]
unsafe impl ZerosValid for .. {} |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
After some discussion with @tmandry I have decided that removing this is more trouble than it is worth, given that it's a pretty tiny chunk of code in the compiler. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The ancient (pre-1.0) RFC 19 suggested using
impl Trait for ..syntax for default traits. That was later changed toauto trait Trait {}syntax. The parser has special treatment for the..syntax, suggesting theautosyntax.Given that default traits have not be stabilized and the
..syntax is so old, the special case seems unnecessary, and it gets in the way of addingErrorGuaranteedtoTyKind::Err. This commit removes it and the tests.r? @oli-obk