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

Introduce ~const #88328

Merged
merged 10 commits into from
Aug 28, 2021
Merged

Introduce ~const #88328

merged 10 commits into from
Aug 28, 2021

Conversation

fee1-dead
Copy link
Member

@fee1-dead fee1-dead commented Aug 25, 2021

  • Removed ?const and change uses of ?const
  • Added ~const to the AST. It is gated behind const_trait_impl.
  • Validate ~const in ast_validation.
  • Update UI Tests
  • Add enum BoundConstness (With variants NotConst and
    ConstIfConst allowing future extensions)
  • Adjust trait selection and pre-existing code to use BoundConstness.
  • Optional steps for this PR

r? @oli-obk

@rust-highfive
Copy link
Collaborator

Some changes occurred in src/tools/rustfmt.

cc @calebcartwright

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 25, 2021
@rust-log-analyzer

This comment has been minimized.

@jyn514
Copy link
Member

jyn514 commented Aug 26, 2021

Why are you changing this? ~ seems inconsistent with traits; I guess this isn't a trait, but I think at the very least T-lang should be involved somehow.

@jyn514 jyn514 added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Aug 26, 2021
@oli-obk oli-obk removed the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Aug 26, 2021
@oli-obk
Copy link
Contributor

oli-obk commented Aug 26, 2021

@rust-lang/wg-const-eval has permission to experiment with ?const, the change of the syntax to ~const (nothing finalized, just experimental syntax) is within that permission. The previous syntax system was buggy and exposed something to stable we wanted to reserve for an unstable feature. So we are scrapping that scheme and going with a new one, where ?const makes no sense anymore.

@oli-obk
Copy link
Contributor

oli-obk commented Aug 26, 2021

More details in https://rust-lang.zulipchat.com/#narrow/stream/146212-t-compiler.2Fconst-eval/topic/const_trait_impl's.20syntax

@rust-log-analyzer

This comment has been minimized.

 - [x] Removed `?const` and change uses of `?const`
 - [x] Added `~const` to the AST. It is gated behind const_trait_impl.
 - [x] Validate `~const` in ast_validation.
 - [ ] Add enum `BoundConstness` to the HIR. (With variants `NotConst` and
 `ConstIfConst` allowing future extensions)
 - [ ] Adjust trait selection and pre-existing code to use `BoundConstness`.
 - [ ] Optional steps (*for this PR, obviously*)
      - [ ] Fix rust-lang#88155
      - [ ] Do something with constness bounds in chalk
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@@ -609,6 +609,7 @@ impl<'a> Parser<'a> {
|| self.check_lifetime()
|| self.check(&token::Not) // Used for error reporting only.
|| self.check(&token::Question)
|| self.check(&token::Tilde)
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this can be a breaking change for some weird macro like

macro_rules! weird {
    ($x:ty) => { println!("ty"); };
    (dyn ~const Trait) => { println!("~"); };
}

fn main() {
    weird!(dyn Trait);
    weird!(dyn ?const Trait);
    weird!(dyn ~const Trait);
}

but... in hindsight, the same was true for ?const

Copy link
Member Author

Choose a reason for hiding this comment

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

It is gated behind const_trait_impl. Do we have a policy for AST changes like this?

Copy link
Contributor

Choose a reason for hiding this comment

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

the problem is that it works on stable and errors on your branch (or if the feature gate is stabilized, picks the first macro pattern instead of the second)

I think we can do this, but I don't really know the policy. I think we have some way of not breaking macros with syntax changes like this. Maybe @petrochenkov knows?

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@fee1-dead fee1-dead marked this pull request as ready for review August 27, 2021 13:05
@oli-obk
Copy link
Contributor

oli-obk commented Aug 27, 2021

@bors r+ rollup=never (for bisection in case we break sth)

@bors
Copy link
Contributor

bors commented Aug 27, 2021

📌 Commit 2d7dbf2 has been approved by oli-obk

@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 Aug 27, 2021
@bors
Copy link
Contributor

bors commented Aug 27, 2021

⌛ Testing commit 2d7dbf2 with merge ac50a53...

@bors
Copy link
Contributor

bors commented Aug 28, 2021

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing ac50a53 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 28, 2021
@bors bors merged commit ac50a53 into rust-lang:master Aug 28, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 28, 2021
@fee1-dead fee1-dead deleted the not-quite-const branch August 28, 2021 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

const trait checks allow calling non-const methods
7 participants