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

Closures in enum discriminants error when they shouldn't #50689

Closed
leoyvens opened this issue May 12, 2018 · 3 comments · Fixed by #50851
Closed

Closures in enum discriminants error when they shouldn't #50689

leoyvens opened this issue May 12, 2018 · 3 comments · Fixed by #50851
Assignees
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) A-typesystem Area: The type system P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@leoyvens
Copy link
Contributor

leoyvens commented May 12, 2018

This used to work (regression from 1.14 stable) but now it doesn't:

enum Functions {
    Square = (|x:i32| { }, 42).1,
}

In #50623 the error less bad and some FIXME(#50689) are left in the code with hints, but still doesn't work. Issue #50688 seems related.

@estebank estebank added A-typesystem Area: The type system regression-from-stable-to-stable Performance or correctness regression from one stable version to another. A-const-eval Area: Constant evaluation (MIR interpretation) labels May 14, 2018
@nikomatsakis nikomatsakis added P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-nominated labels May 17, 2018
@nikomatsakis
Copy link
Contributor

Doesn't seem mega urgent. Kind of a gray area for promotion. Marking as medium but nominating for discussion with @eddyb

@eddyb
Copy link
Member

eddyb commented May 17, 2018

The old const-eval got changed at some point to eagerly evaluate constants, but miri should be fine.
Looks like we ended up with some typeck bugs in the meanwhile.

@nikomatsakis nikomatsakis added P-high High priority and removed I-nominated P-medium Medium priority labels May 17, 2018
@nikomatsakis
Copy link
Contributor

Something seems wacky. Assigning to eddyb to stay on top of ;) marking as P-high for now but that might be an over-reaction.

bors added a commit that referenced this issue May 20, 2018
rustc: introduce {ast,hir}::AnonConst to consolidate so-called "embedded constants".

Previously, constants in array lengths and enum variant discriminants were "merely an expression", and had no separate ID for, e.g. type-checking or const-eval, instead reusing the expression's.

That complicated code working with bodies, because such constants were the only special case where the "owner" of the body wasn't the HIR parent, but rather the same node as the body itself.
Also, if the body happened to be a closure, we had no way to allocate a `DefId` for both the constant *and* the closure, leading to *several* bugs (mostly ICEs where type errors were expected).

This PR rectifies the situation by adding another (`{ast,hir}::AnonConst`) node around every such constant. Also, const generics are expected to rely on the new `AnonConst` nodes, as well (cc @varkor).
* fixes #48838
* fixes #50600
* fixes #50688
* fixes #50689
* obsoletes #50623

r? @nikomatsakis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) A-typesystem Area: The type system P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants