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

correctly deal with unsorted generic parameters #74676

Merged
merged 5 commits into from
Jul 24, 2020

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Jul 23, 2020

We now stop sorting generic params and instead correctly handle unsorted params in the rest of the compiler.

We still restrict const params to come after type params though, so this PR does not change anything which
is visible to users.

This might slightly influence perf, so let's prevent any unintentional rollups. @bors rollup=never

r? @varkor

@lcnr lcnr added the F-const_generics `#![feature(const_generics)]` label Jul 23, 2020
@Mark-Simulacrum
Copy link
Member

@bors rollup=never

I believe bors doesn't listen to commands in the descriptions of PRs.

@lcnr
Copy link
Contributor Author

lcnr commented Jul 23, 2020

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Jul 23, 2020

⌛ Trying commit 9910f15 with merge ba1527a7e6cf83998280da8ed6fb2d5c16c77dda...

@lcnr lcnr added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 23, 2020
@bors
Copy link
Contributor

bors commented Jul 23, 2020

☀️ Try build successful - checks-actions, checks-azure
Build commit: ba1527a7e6cf83998280da8ed6fb2d5c16c77dda (ba1527a7e6cf83998280da8ed6fb2d5c16c77dda)

@rust-timer
Copy link
Collaborator

Queued ba1527a7e6cf83998280da8ed6fb2d5c16c77dda with parent 2bbfa02, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (ba1527a7e6cf83998280da8ed6fb2d5c16c77dda): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never

@varkor
Copy link
Member

varkor commented Jul 24, 2020

Thanks for figuring out what was going on here — it's been a longstanding annoyance!

@bors r+

@bors
Copy link
Contributor

bors commented Jul 24, 2020

📌 Commit 5f1eea9 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 Jul 24, 2020
@bors
Copy link
Contributor

bors commented Jul 24, 2020

⌛ Testing commit 5f1eea9 with merge cfb6114...

@bors
Copy link
Contributor

bors commented Jul 24, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: varkor
Pushing cfb6114 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 24, 2020
@bors bors merged commit cfb6114 into rust-lang:master Jul 24, 2020
@lcnr lcnr deleted the generics-no-sort branch July 24, 2020 16:01
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 10, 2020
Remove restriction on type parameters preceding consts w/ feature const-generics

Removed the restriction on type parameters preceding const parameters when the feature const-generics is enabled.

Builds on rust-lang#74676, which deals with unsorted generic parameters. This just lifts the check in lowering the AST to HIR that permits consts and types to be reordered with respect to each other. Lifetimes still must precede both

This change is not intended for min-const-generics, and is gated behind the `#![feature(const_generics)]`.

One thing is that it also permits type parameters without a default to come after consts, which I expected to not work, and was hoping to get more guidance on whether that should be permitted or how to prevent it otherwise.

I did not go through the RFC process for this pull request because there was prior work to get this feature added. In the previous PR that was cited, work was done to enable this change.

r? @lcnr
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-const_generics `#![feature(const_generics)]` 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.

6 participants