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

dont provide fwd declared params to cg defaults #86580

Merged
merged 6 commits into from
Jul 24, 2021

Conversation

BoxyUwU
Copy link
Member

@BoxyUwU BoxyUwU commented Jun 23, 2021

Fixes #83938

#![feature(const_evaluatable_checked, const_generics, const_generics_defaults)]
#![allow(incomplete_features)]

pub struct Bar<const N: usize, const M: usize = { N + 1 }>;
pub fn foo<const N1: usize>() -> Bar<N1> { loop {} }

fn main() {}

This PR makes this code no longer ICE, it was ICE'ing previously because when building substs for Bar<N1> we would subst the anon ct: ConstKind::Unevaluated({N + 1}, substs: [N, M]) with substs of [N1]. the anon const has forward declared params supplied though so we end up trying to substitute the provided M param which causes the ICE.

This PR doesn't handle the predicates of the const so

trait Foo<const N: usize> { const Assoc: usize; }
pub struct Bar<const N: usize = { <()>::Assoc }> where (): Foo<N>;

Resolves to <() as Foo<N>>::Assoc which can allow for using fwd declared params indirectly.

trait Foo<const N: usize> {}
struct Bar<const N: usize = { 2 + 3 }> where (): Foo<N>;

This code also ICEs under this PR because instantiating the default's predicates causes an ICE as predicates_of contains predicates with fwd declared params

PR was briefly discussed in this zulip thread

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 23, 2021
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

while this does work for now, we probably want to implement these changes in generics_of instead to deal with the ParamEnv. We will need to do that to deal with unused substs in general.

I don't have a strong opinion on whether to merge this as is, so r=me if you think that it's desirable

@BoxyUwU
Copy link
Member Author

BoxyUwU commented Jul 9, 2021

(this PR needs the code to be wrapped in a if tcx.lazy_normalization() or wahtever so that we dont provide these generics without const_generics/const_evaluatable_checked feature gates)

edit: Done, and changes have been moved to generics_of and explicit_predicates_of

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@BoxyUwU
Copy link
Member Author

BoxyUwU commented Jul 12, 2021

r? @nikomatsakis

@rust-highfive rust-highfive assigned nikomatsakis and unassigned lcnr Jul 12, 2021
@rust-log-analyzer

This comment has been minimized.

compiler/rustc_typeck/src/collect.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/collect.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/outlives/mod.rs Outdated Show resolved Hide resolved
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jul 20, 2021

📌 Commit abfd44d has been approved by nikomatsakis

@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 20, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jul 21, 2021
dont provide fwd declared params to cg defaults

Fixes rust-lang#83938

```rust
#![feature(const_evaluatable_checked, const_generics, const_generics_defaults)]
#![allow(incomplete_features)]

pub struct Bar<const N: usize, const M: usize = { N + 1 }>;
pub fn foo<const N1: usize>() -> Bar<N1> { loop {} }

fn main() {}
```
This PR makes this code no longer ICE, it was ICE'ing previously because when building substs for `Bar<N1>` we would subst the anon ct: `ConstKind::Unevaluated({N + 1}, substs: [N, M])` with substs of `[N1]`. the anon const has forward declared params supplied though so we end up trying to substitute the provided `M` param which causes the ICE.

This PR doesn't handle the predicates of the const so
```rust
trait Foo<const N: usize> { const Assoc: usize; }
pub struct Bar<const N: usize = { <()>::Assoc }> where (): Foo<N>;
```
Resolves to `<() as Foo<N>>::Assoc` which can allow for using fwd declared params indirectly.

```rust
trait Foo<const N: usize> {}
struct Bar<const N: usize = { 2 + 3 }> where (): Foo<N>;
```
This code also ICEs under this PR because instantiating the default's predicates causes an ICE as predicates_of contains predicates with fwd declared params

PR was briefly discussed [in this zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/260443-project-const-generics/topic/evil.20preds.20in.20param.20env.20.2386580)
@bors
Copy link
Contributor

bors commented Jul 21, 2021

⌛ Testing commit abfd44d with merge deab484024a324406ff1c8984f240ec0334a65a3...

@BoxyUwU
Copy link
Member Author

BoxyUwU commented Jul 21, 2021

@bors retry

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

bors commented Jul 21, 2021

⌛ Testing commit abfd44d with merge 8e3359c9c54ca446b64a0aa00316f3f42fcda67a...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jul 21, 2021

💔 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 Jul 21, 2021
@BoxyUwU
Copy link
Member Author

BoxyUwU commented Jul 22, 2021

@bors retry

@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 22, 2021
@BoxyUwU
Copy link
Member Author

BoxyUwU commented Jul 24, 2021

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Jul 24, 2021

📌 Commit d1e5e72 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Jul 24, 2021

⌛ Testing commit d1e5e72 with merge d9aa287...

@bors
Copy link
Contributor

bors commented Jul 24, 2021

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing d9aa287 to master...

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.

ICE using const_generics_defaults
7 participants