Skip to content

Implement proper stability check for const impl Trait, fall back to unstable const when undeclared - #97177

Merged
bors merged 7 commits into
rust-lang:masterfrom
oli-obk:const-stability
May 22, 2022
Merged

Implement proper stability check for const impl Trait, fall back to unstable const when undeclared#97177
bors merged 7 commits into
rust-lang:masterfrom
oli-obk:const-stability

Conversation

@oli-obk

@oli-obkoli-obk commented May 19, 2022

Copy link
Copy Markdown
Contributor

Continuation of #93960

@jhpratt it looks to me like the test was simply not testing for the failure you were looking for? Your checks actually do the right thing for const traits?

jhprattand others added 7 commits May 19, 2022 12:21
@jhpratt@oli-obk
Fall back to being const-unstable when undeclared
f53fc41
@jhpratt@oli-obk
Add and use stability helper methods
a9dd4cf
This avoids an ambiguity (when reading) where `.level.is_stable()` is
not immediately clear whether it is general stability or const
stability.
@jhpratt@oli-obk
Proper const stability check, default to unstable
f0620c9
Rather than deferring to const eval for checking if a trait is const, we
now check up-front. This allows the error to be emitted earlier, notably
at the same time as other stability checks.
Also included in this commit is a change of the default const stability
level to UNstable. Previously, an item that was `const` but did not
explicitly state it was unstable was implicitly stable.
@jhpratt@oli-obk
Move check to existing pass
5ff3311
This alters the diagnostics a bit, as the trait method is still stable.
The only thing this check does is ensure that compilation fails if a
trait implementation is declared const-stable.
@oli-obk
Rebase fallout
cf5a803
@oli-obk
Stable const things need a stability attribute
aa9eae5
@oli-obk
Fix up tests
f9c4f2b
@rustbotrustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels May 19, 2022
@rust-highfive

rust-highfive commented May 19, 2022

Copy link
Copy Markdown
Contributor

Some changes occurred in clean/types.rs.

cc @camelid

@rust-highfive

rust-highfive commented May 19, 2022

Copy link
Copy Markdown
Contributor

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 19, 2022
@jhpratt

jhpratt commented May 19, 2022

Copy link
Copy Markdown
Member

Well, that's good to know at least! It's been a little bit since I've looked at this change, but I'll look over your additions soon.

davidtwco
davidtwco approved these changes May 20, 2022
View reviewed changes

@davidtwcodavidtwco left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

r=me unless jhpratt has feedback

@jhpratt

jhpratt commented May 20, 2022

Copy link
Copy Markdown
Member

LGTM.

@bors r=davidtwco

@bors

bors commented May 20, 2022

Copy link
Copy Markdown
Collaborator

@jhpratt: 🔑 Insufficient privileges: Not in reviewers

@davidtwco

davidtwco commented May 20, 2022

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented May 20, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit f9c4f2b has been approved by davidtwco

@borsbors 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 May 20, 2022
@bors

bors commented May 22, 2022

Copy link
Copy Markdown
Collaborator

⌛ Testing commit f9c4f2b with merge acfd327...

@bors

bors commented May 22, 2022

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing acfd327 to master...

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label May 22, 2022
@bors
bors merged commit acfd327 into rust-lang:masterMay 22, 2022
@rustbotrustbot added this to the 1.63.0 milestone May 22, 2022
@borsbors mentioned this pull request May 22, 2022
Merged
@rust-timer

rust-timer commented May 22, 2022

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (acfd327): comparison url.

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results
  • Primary benchmarks: 🎉 relevant improvement found
  • Secondary benchmarks: 😿 relevant regression found
Regressions 😿
(primary)		Regressions 😿
(secondary)		Improvements 🎉
(primary)		Improvements 🎉
(secondary)		All 😿 🎉
(primary)
count101101
mean2N/A2.4%-1.3%N/A-1.3%
maxN/A2.4%-1.3%N/A-1.3%

Cycles

This benchmark run did not return any relevant results for this metric.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. number of relevant changes↩

  2. the arithmetic mean of the percent change↩

@dtolnaydtolnay mentioned this pull request Nov 17, 2024
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidtwcodavidtwcodavidtwco approved these changes
@jhprattjhprattjhpratt approved these changes

Assignees

@davidtwcodavidtwco

Labels

merged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.T-rustdocRelevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.63.0

Development

Successfully merging this pull request may close these issues.

7 participants

@oli-obk@rust-highfive@jhpratt@bors@davidtwco@rust-timer@rustbot