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 may be mishandling _ within type-ascription expression form #57731

Closed
pnkfelix opened this issue Jan 18, 2019 · 5 comments
Closed

NLL may be mishandling _ within type-ascription expression form #57731

pnkfelix opened this issue Jan 18, 2019 · 5 comments
Assignees
Labels
A-NLL Area: Non Lexical Lifetimes (NLL) NLL-sound Working towards the "invalid code does not compile" goal

Comments

@pnkfelix
Copy link
Member

pnkfelix commented Jan 18, 2019

Adapted from examples given on #55748. Note that observing this bug requires use of feature(type_ascription) and therefore is not high priority.

consider the following code (play):

#![feature(nll, type_ascription)]

#![allow(dead_code, unused_mut)]
type PairCoupledTypes<T> = (T, T);

#[cfg(this_is_just_here_for_comparison_it_behaves_same_with_and_without_nll)]
fn coupled_types_rhs<'a>(_x: &'a u32, s: &'static u32) -> &'static u32 {
    let ((y, _z),) = ((s, &_x),): (PairCoupledTypes<&u32>,);
    y //~ ERROR unsatisfied lifetime constraints
}

fn coupled_wilds_rhs<'a>(_x: &'a u32, s: &'static u32) -> &'static u32 {
    let ((y, _z),) = ((s, &_x),): (PairCoupledTypes<_>,);
    y //~ ERROR unsatisfied lifetime constraints
}

fn main() {}

Right now, as written, this compiles without error. (Note that fn coupled_types_rhs is not included; that function will error with and without NLL.)

If you remove the nll in the feature declaration, the code for fn coupled_wilds_rhs is rejected.

This is an indication that either:

  1. NLL has a bug in how it is handling _ in the type-ascription expression form EXPR: TYPE, or
  2. there exists some argument that fn coupled_wilds_rhs should be accepted (and that NLL is correct in accepting it), but I do not see how such an argument exists that would not also imply that we should be seeing the same behavior in fn coupled_types_rhs.
@matthewjasper
Copy link
Contributor

I had a brief look, the annotation is dropped before we get to NLL. Probably here:

if ty.has_free_regions() || ty.has_projections() {
let c_ty = self.infcx.canonicalize_response(&UserTypeAnnotation::Ty(ty));
debug!("to_ty_saving_user_provided_ty: c_ty={:?}", c_ty);
self.tables.borrow_mut().user_provided_types_mut().insert(ast_ty.hir_id, c_ty);
}

@matthewjasper matthewjasper added the NLL-sound Working towards the "invalid code does not compile" goal label Jan 19, 2019
@matthewjasper
Copy link
Contributor

This also applies to casts, so it is visble on stable:

fn coupled_wilds_rhs<'a>(_x: &'a u32, s: &'static u32) -> &'static u32 {
    let ((y, _z),) = ((s, _x),): (PairCoupledTypes<_>,);
    y //~ ERROR unsatisfied lifetime constraints
}

@pnkfelix
Copy link
Member Author

so it is visble on stable

type ascription is feature-gated though, no?

@pnkfelix
Copy link
Member Author

(or did you mean to use as, as in ((s, _x),) as (PairCoupledTypes<_>,), in your example?)

@matthewjasper
Copy link
Contributor

Yes, it should be as in the example.

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) NLL-sound Working towards the "invalid code does not compile" goal
Projects
None yet
Development

No branches or pull requests

2 participants