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

Teach parser to understand fake anonymous enum syntax #106960

Merged
merged 4 commits into from
Jan 26, 2023

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Jan 17, 2023

Parse Ty | OtherTy in function argument and return types.
Parse type ascription in top level patterns.

Minimally address #100741.

@rustbot
Copy link
Collaborator

rustbot commented Jan 17, 2023

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 17, 2023
@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Jan 17, 2023

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

@fmease
Copy link
Member

fmease commented Jan 17, 2023

You should probably add a self.may_recover() somewhere (#103534). The following code no longer compiles in your patch as far as I can tell (I could be wrong though):

macro_rules! check {
    ($Z:ty) => { compile_error!("triggered"); };
    ($X:ty | $Y:ty) => { };
}

check! { i32 | u8 } // passes on stable & nightly

@estebank
Copy link
Contributor Author

@fmease that code continues to compile successfully (because the new parsing only triggers explicitly on type params and return types and nothing else). Added it as a test, as well as another test for the macro in those positions to confirm that even then the new parsing doesn't trigger in the macro context.

Copy link
Contributor

@cjgillot cjgillot left a comment

Choose a reason for hiding this comment

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

The improvements to type ascription in patterns are great.

@@ -2096,6 +2096,9 @@ pub enum TyKind {
Err,
/// Placeholder for a `va_list`.
CVarArgs,
/// Placeholder for "anonymous enums", which don't exist, but keeping their
/// information around lets us produce better diagnostics.
AnonEnum(Vec<P<Ty>>),
Copy link
Contributor

Choose a reason for hiding this comment

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

While I generally encourage to add information to the AST for diagnostics, adding a variant that does not exist in the language seems a bit too far.
IIUC, this is only needed for pretty-printing. I couldn't see where it appears in ui tests. Why isn't the usual TyKind::Err sufficient?

Copy link
Contributor Author

@estebank estebank Jan 23, 2023

Choose a reason for hiding this comment

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

I can avoid including this for the purposes of this PR, but I have a very early attempt at real support for anonymous enums 1) to have more info for more accurate suggestions in later stages, particularly to deduplicate them (thinking of declaring the same enum in multiple return types) in subsequent work, and 2) having an in-tree proof of concept of the actual feature to convince myself of whether a very restricted version of this would be possible. I can safely remove this from this PR.

Edit: took it out.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe TyKind::Err could contain an enum with parsed invalid syntax, like

enum TyError {
  AnonEnum(Vec<P<Ty>>),
  Other,
}

@cjgillot cjgillot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 21, 2023
@estebank estebank force-pushed the parse-anon-enums branch 2 times, most recently from 0a8fa43 to cfff2eb Compare January 23, 2023 14:32
@estebank estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 23, 2023
@rust-log-analyzer

This comment was marked as resolved.

@cjgillot
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jan 23, 2023

📌 Commit 020cca8 has been approved by cjgillot

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 23, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 23, 2023
Teach parser to understand fake anonymous enum syntax

Parse `Ty | OtherTy` in function argument and return types.
Parse type ascription in top level patterns.

Minimally address rust-lang#100741.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 25, 2023
Teach parser to understand fake anonymous enum syntax

Parse `Ty | OtherTy` in function argument and return types.
Parse type ascription in top level patterns.

Minimally address rust-lang#100741.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2023
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#106407 (Improve proc macro attribute diagnostics)
 - rust-lang#106960 (Teach parser to understand fake anonymous enum syntax)
 - rust-lang#107085 (Custom MIR: Support binary and unary operations)
 - rust-lang#107086 (Print PID holding bootstrap build lock on Linux)
 - rust-lang#107175 (Fix escaping inference var ICE in `point_at_expr_source_of_inferred_type`)
 - rust-lang#107204 (suggest qualifying bare associated constants)
 - rust-lang#107248 (abi: add AddressSpace field to Primitive::Pointer )
 - rust-lang#107272 (Implement ObjectSafe and WF in the new solver)
 - rust-lang#107285 (Implement `Generator` and `Future` in the new solver)
 - rust-lang#107286 (ICE in new solver if we see an inference variable)
 - rust-lang#107313 (Add Style Team Triagebot config)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ba928ba into rust-lang:master Jan 26, 2023
@rustbot rustbot added this to the 1.69.0 milestone Jan 26, 2023
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Feb 2, 2023
…ambiguous, r=estebank

Revert "Teach parser to understand fake anonymous enum syntax" and related commits

anonymous enum types are currently ambiguous in positions like:

* `|` operator: `a as fn() -> B | C`
* closure args: `|_: as fn() -> A | B`

I first tried to thread around `RecoverAnonEnum` into all these positions, but the resulting complexity in the compiler is IMO not worth it, or at least worth a bit more thinking time. In the mean time, let's revert this syntax for now, so we can go back to the drawing board.

Fixes rust-lang#107461

cc: `@estebank` `@cjgillot` rust-lang#106960

---
### Squashed revert commits:

Revert "review comment: Remove AST AnonTy"

This reverts commit 020cca8.

Revert "Ensure macros are not affected"

This reverts commit 12d18e4.

Revert "Emit fewer errors on patterns with possible type ascription"

This reverts commit c847a01.

Revert "Teach parser to understand fake anonymous enum syntax"

This reverts commit 2d82420.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 2, 2023
…biguous, r=estebank

Revert "Teach parser to understand fake anonymous enum syntax" and related commits

anonymous enum types are currently ambiguous in positions like:

* `|` operator: `a as fn() -> B | C`
* closure args: `|_: as fn() -> A | B`

I first tried to thread around `RecoverAnonEnum` into all these positions, but the resulting complexity in the compiler is IMO not worth it, or at least worth a bit more thinking time. In the mean time, let's revert this syntax for now, so we can go back to the drawing board.

Fixes rust-lang#107461

cc: `@estebank` `@cjgillot` rust-lang#106960

---
### Squashed revert commits:

Revert "review comment: Remove AST AnonTy"

This reverts commit 020cca8.

Revert "Ensure macros are not affected"

This reverts commit 12d18e4.

Revert "Emit fewer errors on patterns with possible type ascription"

This reverts commit c847a01.

Revert "Teach parser to understand fake anonymous enum syntax"

This reverts commit 2d82420.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 3, 2023
…ler-errors

Parse and recover from type ascription in patterns

Reintroduce part of rust-lang#106960, which was reverted in rust-lang#107478.

r? `@compiler-errors`
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Feb 9, 2023
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#106407 (Improve proc macro attribute diagnostics)
 - rust-lang#106960 (Teach parser to understand fake anonymous enum syntax)
 - rust-lang#107085 (Custom MIR: Support binary and unary operations)
 - rust-lang#107086 (Print PID holding bootstrap build lock on Linux)
 - rust-lang#107175 (Fix escaping inference var ICE in `point_at_expr_source_of_inferred_type`)
 - rust-lang#107204 (suggest qualifying bare associated constants)
 - rust-lang#107248 (abi: add AddressSpace field to Primitive::Pointer )
 - rust-lang#107272 (Implement ObjectSafe and WF in the new solver)
 - rust-lang#107285 (Implement `Generator` and `Future` in the new solver)
 - rust-lang#107286 (ICE in new solver if we see an inference variable)
 - rust-lang#107313 (Add Style Team Triagebot config)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
calebcartwright pushed a commit to calebcartwright/rust that referenced this pull request Jun 20, 2023
Teach parser to understand fake anonymous enum syntax

Parse `Ty | OtherTy` in function argument and return types.
Parse type ascription in top level patterns.

Minimally address rust-lang#100741.
@estebank estebank deleted the parse-anon-enums branch November 9, 2023 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants