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 doesn't respect user types in closure signatures #54692

Closed
matthewjasper opened this issue Sep 30, 2018 · 7 comments
Closed

NLL doesn't respect user types in closure signatures #54692

matthewjasper opened this issue Sep 30, 2018 · 7 comments
Assignees
Labels
A-NLL Area: Non-lexical lifetimes (NLL) E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. NLL-sound Working towards the "invalid code does not compile" goal

Comments

@matthewjasper
Copy link
Contributor

For example, the following code compiles

#![feature(nll)]
fn foo<'a>() {
    |x: &'a i32| -> &'static i32 {
        return &x;
    };
}

cc #47184

@matthewjasper matthewjasper added A-NLL Area: Non-lexical lifetimes (NLL) NLL-sound Working towards the "invalid code does not compile" goal labels Sep 30, 2018
@nikomatsakis nikomatsakis added this to the Edition 2018 RC 2 milestone Oct 2, 2018
@nikomatsakis
Copy link
Contributor

Tagging as RC2

@nikomatsakis
Copy link
Contributor

So, the closure region requirements are as follows:

> rustc ~/tmp/issue-54692.rs   -Zverbose
note: External requirements
 --> /home/nmatsakis/tmp/issue-54692.rs:6:5
  |
6 | /     |x: &'a i32| -> &'static i32 {
7 | |         return &x;
8 | |     };
  | |_____^
  |
  = note: defining type: DefId(0/1:10 ~ issue_54692[317d]::foo[0]::{{closure}}[0]) with closure substs [
              i8,
              extern "rust-call" fn((&'_#1r i32,)) -> &'_#2r i32
          ]
  = note: late-bound region is '_#3r
  = note: number of external vids: 4
  = note: where '_#1r: '_#2r

@nikomatsakis
Copy link
Contributor

You can see the '_#1r: '_#2r indicating that — indeed — the argument must outlive the return type. But we have lost the connection to 'a and 'static, indeed.

@nikomatsakis
Copy link
Contributor

The user-given signature of the closure is computed here, during the HIR typeck:

// Get the signature S that the user gave.
//
// (See comment on `sig_of_closure_with_expectation` for the
// meaning of these letters.)
let supplied_sig = self.supplied_sig_of_closure(decl);

(In general, reconciling the expected signature and the user-given signature is a royal pain, as is described in detail in that file.)

I imagine what we have to do is probably to record the "supplied closure signature" somewhere, probably in a new table similar to user_substs.

The question is where it should be accessed from. I think it should probably be the closure's job to enforce these relations (if you read that code above, you might see why: in general, the user's signatures are written from the "POV" of the closure body, rather than its creator).

This implies that MIR may want to grow a user_fn_sig field, set to None for everything but closures. The type-checker can then unify those signatures with the types of the arguments.

@nikomatsakis nikomatsakis added the E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. label Oct 2, 2018
@nikomatsakis
Copy link
Contributor

Hmm this may be blocking me in other areas.

@nikomatsakis nikomatsakis assigned mikhail-m1 and unassigned blitzerr Oct 4, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Oct 9, 2018

adding self @spastorino to the assignee list to try to ensure this gets addressed soon

@pnkfelix pnkfelix assigned pnkfelix and spastorino and unassigned pnkfelix Oct 9, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Oct 9, 2018

Got a report from @mikhail-m1 that they are actively working on this

Unassigning @spastorino (since @mikhail-m1 is taking lead here)

bors added a commit that referenced this issue Oct 23, 2018
…=MatthewJasper

enforce user annotations in closure signatures

Not *quite* ready yet but I'm opening anyway. Still have to finish running tests locally.

Fixes #54692
Fixes #54124

r? @matthewjasper
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) E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. NLL-sound Working towards the "invalid code does not compile" goal
Projects
None yet
Development

No branches or pull requests

6 participants