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

ICE with Const Generic expression inside array declaration #60619

Closed
sfleischman105 opened this issue May 7, 2019 · 19 comments
Closed

ICE with Const Generic expression inside array declaration #60619

sfleischman105 opened this issue May 7, 2019 · 19 comments
Labels
A-const-generics Area: const generics (parameters and arguments) A-lazy-normalization Area: lazy normalization (tracking issue: #60471) C-bug Category: This is a bug. F-const_generics `#![feature(const_generics)]` glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@sfleischman105
Copy link

Found an ICE with the following code:

pub struct Foo<const LEN: usize> {
    buf: [u8; {LEN * 2}]
}

Results in "no errors encountered even though delay_span_bug issued" on current nightly.

Playground link.

The ICE still happens regardless of the LEN * 2 being in a block, in parenthesis, or even as a standalone expression. Interestingly, refactoring out the array into a seperate struct solves the issue.

cc @varkor @yodaldevoid

@Centril Centril added A-const-generics Area: const generics (parameters and arguments) I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-nominated labels May 7, 2019
@Centril
Copy link
Contributor

Centril commented May 7, 2019

Recommended: P-medium due to unstable incomplete feature.

@varkor varkor removed the I-nominated label May 7, 2019
@varkor
Copy link
Member

varkor commented May 7, 2019

Thanks for the report.

(This only ICEs with the feature gate, so there's no need to raise the priority on this.)

@hellow554
Copy link
Contributor

hellow554 commented May 8, 2019

(This only ICEs with the feature gate, so there's no need to raise the priority on this.)

Not on beta though, but I guess #60277 will land in beta soon-ish so it shouldn't ICE there anymore?

@yodaldevoid
Copy link
Contributor

I'm currently looking into this. That said, if someone else wants to work on this, let me know.

@yodaldevoid
Copy link
Contributor

This can also be reproduced with the following code:

pub struct Foo<const LEN: usize> {
    buf: [u8; {LEN}]
}

Due to this, I think that this is related to the const parameter being in braces in the array length.

@varkor
Copy link
Member

varkor commented May 11, 2019

If that's the case, it might be related to #60717.

@HadrienG2
Copy link

@varkor Are you sure about that PR/issue number ? To my untrained eye, the linked ancient PR seems largely unrelated to the issue at hand.

@varkor
Copy link
Member

varkor commented May 11, 2019

Are you sure about that PR/issue number ?

Thanks, I had missed off the final character 😄

@yodaldevoid
Copy link
Contributor

Updating to latest master which includes #60717 fixes this for my minified case, but there seems to be an additional issue here as the original case still fails.

@varkor
Copy link
Member

varkor commented May 11, 2019

The original issue will continue to fail because the expression { P } (for a const parameter P) is special-cased to be treated as ConstValue::Param, whereas { P * 2 } (and any other more complicated expression) is going to be treated as ConstValue::Unevaluated. So I suppose #60717 is really just hiding the issue in the reduced test case: there shouldn't be an issue using an arbitrary expression as an array length.

@yodaldevoid
Copy link
Contributor

Ah, I see, that makes sense. it sounds like #60717 was a bit of a hack, or rather that our AST conversion code is a bit of a hack as it stands now.

It seems like we need to either drill into ConstValue::Unevaluated until we are sure there exists no const param anywhere in it, or we need to plug into the const_eval mechanism and find const params when it decides a constant is "too generic" (as it does for this test case). I'm not sure the viability of the first solution just on a gut level, and I'm not sure of the second solution as that seems too far into the compiler pipeline to start changing our minds about how we parsed the AST. I could easily be wrong on both points or have missed another solution entirely.

@varkor
Copy link
Member

varkor commented May 11, 2019

or rather that our AST conversion code is a bit of a hack as it stands now

Yes, I think this is the case. We're being very strict about parameters: at the moment, we only allow quantification directly over a parameter, rather than being in any more complex expression.

It seems like we need to either drill into ConstValue::Unevaluated until we are sure there exists no const param anywhere in it

At what point are you thinking this should happen? Ideally we want to permit parameters everywhere, but at the moment, I'm assuming we hit this path:

ConstValue::Param(_) => return err!(TooGeneric),

which means we haven't substituted the parameter early enough. I'm not sure when it's supposed to be being substituted, though.

@yodaldevoid
Copy link
Contributor

You are correct at where we are throwing that error.

@yodaldevoid
Copy link
Contributor

Ok, I've spent some more time looking into this as I've had time.

The multiplication with the const param (and hence the error) is attempted to be evaluated as part of well-formedness checking of the struct. The expression containing the const param attempted to be evaluated, found to be too generic as it hasn't been monomorphized (and never will in this code as it isn't used), and the const evaluator returns saying it is too generic. For raw const params everything is kosher as the what determines whether to try to evaluate a const only checks for Unevaluated:

if let ConstValue::Unevaluated(def_id, substs) = constant.val {

Now, because the TooGeneric error has been thrown, later on when errors are checked for it is found and an error is thrown here:

fcx.select_all_obligations_or_error();

This is because an obligation that the expression can be const eval'd got added when const evaluation failed, and we cannot prove that it can be evaluated.


To say that I am out of my comfort zone here is an understatement. That said, I think the problem here is that we are treating all TooGeneric errors as catestrophic, where in this case all we care about is that the interior parameter (interior to the operation, that is) is declared by the struct. I'm not sure how to go about solving this or if I am even diagnosing the root issue correctly here, so I would love to hear from those who are more knowledgeable on the subject.

I should have known this would not be an easy one to fix...

@varkor
Copy link
Member

varkor commented May 30, 2019

This is probably the same issue as #61368.

@varkor
Copy link
Member

varkor commented Jun 13, 2019

I spoke to @eddyb about these sorts of issues: it seems we need to wait for lazy normalisation before we can handle them, unfortunately.

@varkor varkor added the A-lazy-normalization Area: lazy normalization (tracking issue: #60471) label Jun 13, 2019
@eddyb
Copy link
Member

eddyb commented Jun 13, 2019

This is an instance of #43408, please use that as the canonical issue.

@Centril Centril added requires-nightly This issue requires a nightly compiler in some way. F-const_generics `#![feature(const_generics)]` labels Aug 6, 2019
@jonas-schievink jonas-schievink added the C-bug Category: This is a bug. label Aug 6, 2019
@rust-lang-glacier-bot rust-lang-glacier-bot added the glacier ICE tracked in rust-lang/glacier. label Oct 15, 2019
@jonas-schievink
Copy link
Contributor

The message is now:

error: internal compiler error: src/librustc/ty/subst.rs:650: const parameter `LEN/#0` (Const { ty: usize, val: Param(LEN/#0) }/0) out of range when substituting substs=[]

@eddyb
Copy link
Member

eddyb commented Nov 27, 2019

Closing as duplicate of #43408 (which should probably be updated to mention not just array lengths but any const expressions in types).

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) A-lazy-normalization Area: lazy normalization (tracking issue: #60471) C-bug Category: This is a bug. F-const_generics `#![feature(const_generics)]` glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants