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

nll: respect user type annotations with constants in patterns #55511

Closed
nikomatsakis opened this issue Oct 30, 2018 · 5 comments
Closed

nll: respect user type annotations with constants in patterns #55511

nikomatsakis opened this issue Oct 30, 2018 · 5 comments
Assignees
Labels
A-NLL Area: Non-lexical lifetimes (NLL) NLL-sound Working towards the "invalid code does not compile" goal P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

Breaking off from #47184 -- we need to respect user type annotations in patterns. Here are some examples:

Compiles without NLL (playground):

trait Foo<'a> {
    const C: &'a u32;
}

impl<'a, T> Foo<'a> for T {
    const C: &'a u32 = &22;
}

fn foo() {
    let a = 22;
    match &a {
        <() as Foo<'static>>::C => { }
        &_ => { }
    }
}

fn main() {
}

Errors without NLL (playground):

use std::cell::Cell;

trait Foo<'a> {
    const C: Option<Cell<&'a u32>>;
}

impl<'a, T> Foo<'a> for T {
    const C: Option<Cell<&'a u32>> = None;
}

fn foo() {
    let a = 22;
    let b = Some(Cell::new(&a));
    match b {
        <() as Foo<'static>>::C => { }
        _ => { }
    }
}

fn main() {
}
@nikomatsakis nikomatsakis added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-NLL Area: Non-lexical lifetimes (NLL) labels Oct 30, 2018
@nikomatsakis nikomatsakis added this to the Rust 2018 Release milestone Oct 30, 2018
@pnkfelix pnkfelix added the NLL-sound Working towards the "invalid code does not compile" goal label Nov 6, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Nov 6, 2018

triage: Either this is something that we're planning to implement and backport to the beta channel... or its something that is not that high priority.

Given that this is an NLL-sound issue, I'm going to assume that it is indeed high priority and belongs on the Release milestone.

Thus, tagging as P-high

@pnkfelix pnkfelix added the P-high High priority label Nov 6, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Nov 7, 2018

Just a quick note: this variant of the description's second example is rejected by NLL (play):

#![feature(nll)]

use std::cell::Cell;

trait Foo<'a> {
    const C: Option<Cell<&'a u32>>;
}

impl<'a, T> Foo<'a> for T {
    const C: Option<Cell<&'a u32>> = None;
}

fn foo<'a>(r: &'a i32) {
    let b = Some(Cell::new(r));
    match b {
        <() as Foo<'static>>::C => { }
        _ => { }
    }
}

fn main() {
}

From looking at the MIR generated for the description's second example, perhaps the problem stems from this ?

    let mut _4: &'static u32;
...
        StorageLive(_1);                 // bb0[0]: scope 0 at ../../should-cfail.rs:12:9: 12:10
        _1 = const 22u32;                // bb0[1]: scope 0 at ../../should-cfail.rs:12:13: 12:15
...
        _4 = &'static _1;                // bb0[6]: scope 1 at ../../should-cfail.rs:13:28: 13:30

(I got the 'static annotation in the _4 = &'static _1; via -Z identify-regions. Though I'm not 100% sure that debug flag reflects the NLL state. Anyway, it might be important to make that case work.)

@pnkfelix pnkfelix self-assigned this Nov 8, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Nov 8, 2018

triage: assigning to self

@pnkfelix
Copy link
Member

didn't get a chance to look into this further today. Unassigning self (I'll be back from leave in ~2 weeks, but I want to make sure other people know that this is available to hack on in the meantime).

@pnkfelix pnkfelix removed their assignment Nov 12, 2018
@nikomatsakis nikomatsakis removed this from the Rust 2018 Release milestone Nov 13, 2018
@nikomatsakis
Copy link
Contributor Author

Removing from milestone — sufficiently obscure that it need not be a release blocker

@nikomatsakis nikomatsakis added P-medium Medium priority and removed P-high High priority labels Nov 13, 2018
@davidtwco davidtwco self-assigned this Nov 24, 2018
bors added a commit that referenced this issue Jan 1, 2019
NLL: User type annotations refactor, associated constant patterns and ref bindings.

Fixes #55511 and Fixes #55401. Contributes to #54943.

This PR performs a large refactoring on user type annotations, checks user type annotations for associated constants in patterns and that user type annotations for `ref` bindings are respected.

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) NLL-sound Working towards the "invalid code does not compile" goal P-medium Medium 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

3 participants