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

figure out how to integrate constants and the MIR type checker #46702

Closed
nikomatsakis opened this issue Dec 13, 2017 · 9 comments
Closed

figure out how to integrate constants and the MIR type checker #46702

nikomatsakis opened this issue Dec 13, 2017 · 9 comments
Assignees
Labels
A-NLL Area: Non Lexical Lifetimes (NLL) C-enhancement Category: An issue proposing an enhancement or a PR with one. NLL-sound Working towards the "invalid code does not compile" goal P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Dec 13, 2017

Currently the MIR type checker has some kind of hacky code to figure out what sorts of well-formedness constraints are present on a constant. The values of constants now are "post normalization" and don't give us ready access to what we need. As the comment from the code says:

                // FIXME(#46702) -- We need some way to get the predicates
                // associated with the "pre-evaluated" form of the
                // constant. For example, consider that the constant
                // may have associated constant projections (`<Foo as
                // Trait<'a, 'b>>::SOME_CONST`) that impose
                // constraints on `'a` and `'b`. These constraints
                // would be lost if we just look at the normalized
                // value.
@nikomatsakis nikomatsakis added A-NLL Area: Non Lexical Lifetimes (NLL) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 13, 2017
@XAMPPRocky XAMPPRocky added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Mar 26, 2018
@nikomatsakis
Copy link
Contributor Author

I'm not really aware of any actual problems linked to this code, but I'll push it to the release milestone.

@nikomatsakis nikomatsakis added this to the Rust 2018 Release milestone Jul 3, 2018
@pnkfelix
Copy link
Member

visited for triage. @nikomatsakis believes this continues to be a potential issue of (potentially bad) interaction between miri integration and MIR/NLL. tagging as NLL-deferred

@pnkfelix
Copy link
Member

pnkfelix commented Nov 8, 2018

Visited for T-compiler triage. I cannot imagine us blocking the release on this, given that there are not known bugs linked directly to this code...

I-nominating for discussion at next WG-compiler-nll meeting. Removing from Release milestone.

@pnkfelix pnkfelix removed this from the Rust 2018 Release milestone Nov 8, 2018
@pnkfelix
Copy link
Member

unnominating. We'll get to this when we get to all the NLL-deferred issues, and right now its just clogging up T-compiler process.

@pnkfelix
Copy link
Member

Re-triaging for #56754. Assuming NLL-sound for now. Tagging P-high based on that assumption. And nominating for discussion at tonight's A-NLL meeting, with the intent that someone get assigned to investigate this issue. (If no one is assigned by thursday, then please unnominate since I do not want this clogging up T-compiler again).

@pnkfelix pnkfelix added I-nominated P-high High priority NLL-sound Working towards the "invalid code does not compile" goal and removed NLL-deferred labels Dec 19, 2018
@pnkfelix
Copy link
Member

discussed at A-NLL meeting. assigning to niko. removing I-nominated.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 20, 2018

I have recently run into this code. I'm not sure I'm doing the right thing though: https://github.com/rust-lang/rust/pull/56723/files#r241086396

@matthewjasper
Copy link
Contributor

It seems that the handling of type annotations takes care of the example in the comment. There are cases where types of constants are handled incorrectly:

// Compiles in both migrate mode and NLL mode.

fn main() {
    const FUN: fn(&'static ()) = |_| {};
    struct A;
    impl A {
        const MORE_FUN: fn(&'static ()) = |_| {};
    }

    let x = ();
    FUN(&x);
    A::MORE_FUN(&x);
}

@matthewjasper matthewjasper self-assigned this Jan 11, 2019
@pnkfelix
Copy link
Member

triage: no movement that I know of. But its assigned to @matthewjasper and I think that seems like a reasonable status for now. Only question is whether to keep this at P-high or downgrade to P-medium. Presumably the demo posted in the previous comment is probably justification for leaving at P-high.

bors added a commit that referenced this issue Jan 25, 2019
[NLL] Clean up handling of type annotations

* Renames (Canonical)?UserTypeAnnotation -> (Canonical)?UserType so that the name CanonicalUserTypeAnnotation is free.
* Keep the inferred type associated to user type annotations in the MIR, so that it can be compared against the annotated type, even when the annotated expression gets removed from the MIR. (#54943)
* Use the inferred type to allow infallible handling of user type projections (#57531)
* Uses revisions for the tests in #56993
* Check the types of `Unevaluated` constants with no annotations (#46702)
* Some drive-by cleanup

Closes #46702
Closes #54943
Closes #57531
Closes #57731
cc #56993 leaving this open to track the underlying issue: we are not running tests with full NLL enabled on CI at the moment

r? @nikomatsakis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NLL Area: Non Lexical Lifetimes (NLL) C-enhancement Category: An issue proposing an enhancement or a PR with one. NLL-sound Working towards the "invalid code does not compile" goal P-high High priority 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

5 participants