Skip to content

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Sep 1, 2020

This is quite a bad hack to fix #75486. There might be a better way to check if the self type depends on generic parameters, but I wasn't able to come up with one.

r? @varkor cc @petrochenkov

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 1, 2020
@bors

This comment has been minimized.

Comment on lines +114 to 116
/// The `bool` indicates if this constant may reference generic parameters
/// and is used to only allow generic parameters to be used in trivial constant expressions.
ConstantItemRibKind(bool),
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally this would be a new enum, but it is great you added docs to it.

Comment on lines 2618 to 2626
if self.session.features_untracked().min_const_generics {
// HACK(min_const_generics): We currently only allow `N` or `{ N }`.
if !trivial {
// HACK(min_const_generics): If we encounter `Self` in an anonymous constant
// we can't easily tell if it's generic at this stage, so we instead remember
// this and then enforce the self type to be concrete later on.
if let Res::SelfTy(trait_def, Some((impl_def, _))) = res {
res = Res::SelfTy(trait_def, Some((impl_def, true)));
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think of this @petrochenkov?

It seems sketchy as hell, but likely "correct", and being gated behing min_const_generics I am inclined at allowing as is.

@lcnr lcnr force-pushed the const-Self branch 2 times, most recently from 8271d7c to c71f90e Compare September 8, 2020 09:40
@lcnr
Copy link
Contributor Author

lcnr commented Sep 8, 2020

✔️ review

@varkor
Copy link
Contributor

varkor commented Sep 12, 2020

I'm going to approve: if @petrochenkov has any comments later, we can address them in a follow up.

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 12, 2020

📌 Commit f32593b5d5693d11c582240cf14ada7bb06201aa 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 Sep 12, 2020
@bors
Copy link
Collaborator

bors commented Sep 12, 2020

⌛ Testing commit f32593b5d5693d11c582240cf14ada7bb06201aa with merge b40adb3fedaabc8291fcc5a429b101336bf2ed5f...

@bors
Copy link
Collaborator

bors commented Sep 12, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 12, 2020
@lcnr
Copy link
Contributor Author

lcnr commented Sep 13, 2020

blessed new tests

@bors r=varkor

@bors
Copy link
Collaborator

bors commented Sep 13, 2020

📌 Commit 90dd798 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 Sep 13, 2020
@bors
Copy link
Collaborator

bors commented Sep 14, 2020

⌛ Testing commit 90dd798 with merge 56d8a93...

@bors
Copy link
Collaborator

bors commented Sep 14, 2020

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 14, 2020
@bors bors merged commit 56d8a93 into rust-lang:master Sep 14, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 14, 2020
@lcnr lcnr deleted the const-Self branch September 14, 2020 06:09
@lcnr lcnr added F-const_generics `#![feature(const_generics)]` F-min_const_generics and removed F-const_generics `#![feature(const_generics)]` labels Nov 14, 2020
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.

min_const_generics prevents references to concrete Self param
6 participants