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

In which we start to parse const generics defaults #80547

Merged
merged 7 commits into from
Jan 1, 2021

Conversation

lqd
Copy link
Member

@lqd lqd commented Dec 31, 2020

As discussed in this zulip topic, this PR extracts the parsing parts from @JulianKnodt's PR #75384 for a better user-experience using the newly stabilized min_const_generics (albeit temporary) as shown in #80507: trying to use default values on const generics currently results in parse errors, as if the user didn't use the correct syntax (which is somewhat true but also misleading).

This PR extracts (and slightly modifies in a couple places) @JulianKnodt's parsing code (with attribution if I've done everything correctly), AST and HIR changes, and feature gate setup.

This feature is now marked as "incomplete" and thus will also print out the expected "const generics default values are unstable" error instead of a syntax error. Note that, as I've only extracted the parsing part, the actual feature will not work at all if enabled. There will be ICEs, and inference errors on the const generics default values themselves.

Fixes #80507.

Once this merges, I'll:

  • modify the const generics tracking issue to refer to the const_generics_defaults gate rather than the older temporary name it uses there.
  • create the GH F-const_generics_defaults label

r? @varkor

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 31, 2020
@varkor
Copy link
Member

varkor commented Dec 31, 2020

Thanks @lqd — and thanks for the work on the parsing and lowering changes, @JulianKnodt!

@bors r+

@bors
Copy link
Contributor

bors commented Dec 31, 2020

📌 Commit 583e7e4f13300946c88cf5393a585ec873cf38f9 has been approved by varkor

@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 Dec 31, 2020
@varkor varkor added the A-const-generics Area: const generics (parameters and arguments) label Dec 31, 2020
@varkor
Copy link
Member

varkor commented Dec 31, 2020

@bors r-

Something was brought something to my attention regarding accidental stabilisation of the syntax.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 31, 2020
@varkor
Copy link
Member

varkor commented Dec 31, 2020

The issue I had overlooked is that we're feature gating too liberally here, in that the new syntax will not be forbidden under #[cfg(FALSE)], so that it's still accepted by the parser, even though it's supposed to be unstable syntax. The feature gating should take place in feature_gate.rs rather than ast_validation.rs: we need to collect the spans corresponding to the default generic parameters so that we can emit correct errors.

There's an example in this PR: https://github.com/rust-lang/rust/pull/63545/files

@lqd: sorry for overlooking this. Would you be able to make the changes and add a new test for #[cfg(FALSE)]?

@lqd
Copy link
Member Author

lqd commented Dec 31, 2020

@varkor sure !

@varkor
Copy link
Member

varkor commented Dec 31, 2020

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Dec 31, 2020

📌 Commit d02ac5a4bf01151b68e25ccc10d43293c4990cd6 has been approved by varkor

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 31, 2020
@bors
Copy link
Contributor

bors commented Dec 31, 2020

☔ The latest upstream changes (presumably #80459) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 31, 2020
@lqd
Copy link
Member Author

lqd commented Dec 31, 2020

@bors r=varkor

@bors
Copy link
Contributor

bors commented Dec 31, 2020

📌 Commit 2a012649d6df03f1ea1ee078effe44842d79b29e has been approved by varkor

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 31, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 1, 2021
Rollup of 8 pull requests

Successful merges:

 - rust-lang#80323 (Update and improve `rustc_codegen_{llvm,ssa}` docs)
 - rust-lang#80368 (rustdoc: Render visibilities succinctly)
 - rust-lang#80514 (Fix broken ./x.py install)
 - rust-lang#80519 (Take type defaults into account in suggestions to reorder generic parameters)
 - rust-lang#80526 (Update LLVM)
 - rust-lang#80532 (remove unnecessary trailing semicolon from bootstrap)
 - rust-lang#80548 (FIx ICE on wf check for foreign fns)
 - rust-lang#80551 (support pattern as const parents in type_of)

Failed merges:

 - rust-lang#80547 (In which we start to parse const generics defaults)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Jan 1, 2021

☔ The latest upstream changes (presumably #80566) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 1, 2021
- Adds optional default values to const generic parameters in the AST
  and HIR
- Parses these optional default values
- Adds a `const_generics_defaults` feature gate
The `const_generics_defaults` now handles them, and they correctly parse, so we can update these tests expecting a parser error .
This is important to not accidentally stabilize the parsing of the syntax while it still is experimental and not formally accepted
@lqd
Copy link
Member Author

lqd commented Jan 1, 2021

@bors r=varkor

@bors
Copy link
Contributor

bors commented Jan 1, 2021

📌 Commit 942b7ce has been approved by varkor

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 1, 2021
@bors
Copy link
Contributor

bors commented Jan 1, 2021

⌛ Testing commit 942b7ce with merge a609fb4...

@bors
Copy link
Contributor

bors commented Jan 1, 2021

☀️ Test successful - checks-actions
Approved by: varkor
Pushing a609fb4 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 1, 2021
@bors bors merged commit a609fb4 into rust-lang:master Jan 1, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 1, 2021
@lqd lqd deleted the const_generics_defaults branch January 1, 2021 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-generics Area: const generics (parameters and arguments) merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide more helpful error message when trying to use const generic defaults
6 participants