Stream: t-compiler/wg-nll

Topic: #54692 closure signatures


nikomatsakis (Oct 04 2018 at 18:32, on Zulip):

@mikhail-m1 are you going to take a look at this?

mikhail-m1 (Oct 04 2018 at 18:42, on Zulip):

@nikomatsakis yes, sounds interesting

nikomatsakis (Oct 04 2018 at 18:48, on Zulip):

@mikhail-m1 I think it's blocking something else I'm working on. I can try to leave a few more tips if you like; I forget how much I wrote before

mikhail-m1 (Oct 04 2018 at 18:50, on Zulip):

additional information is always helpful, but it looks enough at least for beginning

mikhail-m1 (Oct 05 2018 at 15:00, on Zulip):

@nikomatsakis I looked around, expected steps:
1. Store closure signatures in the tables in a new field
2. in MIR HAIR builder use it for unification during closure processing
I need some time to understand how it works because previously I didn't have a chance to touch this part of code

nikomatsakis (Oct 05 2018 at 15:27, on Zulip):

@mikhail-m1 that sounds correct, yes, let me know if there are particular things I should explain

mikhail-m1 (Oct 05 2018 at 16:23, on Zulip):

comments about the second step will be helpful because I need to check how this is implemented for functions

nikomatsakis (Oct 05 2018 at 16:35, on Zulip):

ok so actually step 2 is a bit confused

nikomatsakis (Oct 05 2018 at 16:36, on Zulip):

the MIR HAIR builder would simply store the user types somewhere in the Mir data structure (or elsewhere)

nikomatsakis (Oct 05 2018 at 16:36, on Zulip):

and there is a step 3: extend NLL type checker to enforce them

nikomatsakis (Oct 05 2018 at 16:36, on Zulip):

I will try to write up how it works now for some cases to show what I mean

mikhail-m1 (Oct 11 2018 at 13:17, on Zulip):

Hi @nikomatsakis , I stuck a little with choosing a right type for storing closure signatures in the tables, I have three options now:

after a signature is added in typeck/check/close.rs, it's inserted to tcx tables in writeback.rs, and I'am going to use it in unversal_region.rs UniversalRegionsBuilder::build inside replace_bound_regions_with_nll_infer_vars same way as for a common function.

I'm not going to touch HAIR or MIR generation, but may be I missed something.

But now I cannot find how to change 'UniversalRegionsBuilder::defining_ty to returns closure Ty`. It's not clear how to convert described above types. Or may be all approach for second part is wrong...

nikomatsakis (Oct 11 2018 at 18:24, on Zulip):

Hey @mikhail-m1

nikomatsakis (Oct 11 2018 at 18:38, on Zulip):

hmm so I think that we want to be preserved a PolyFnSig — specifically the supplied_sig returned here

nikomatsakis (Oct 11 2018 at 18:39, on Zulip):

we would canonicalize it, presumably, to make a Canonical<PolyFnSig>

nikomatsakis (Oct 11 2018 at 18:39, on Zulip):

I would think we want to store that into the MIR definition, something like user_supplied_sig: Option<Canonical<PolyFnSig>>>

nikomatsakis (Oct 11 2018 at 18:40, on Zulip):

then the NLL type-checker would read it from there

nikomatsakis (Oct 11 2018 at 18:40, on Zulip):

but this is mostly because I want to try and make the MIR relatively self-contained; I suppose you could fetch it with a query instead

mikhail-m1 (Oct 15 2018 at 13:59, on Zulip):

@nikomatsakis sorry but I cannot find a right way how to use a stored closure signature, I am tried to use it here https://github.com/rust-lang/rust/blob/2a5de33193b4a96af0af7faff2a8b6d6f1b5989f/src/librustc_mir/borrow_check/nll/universal_regions.rs#L584 but the code expects a closure signature with arguments as a tuple, and I cannot find a right way how to convert it in my case or may be I just should make an other branch.

nikomatsakis (Oct 15 2018 at 16:06, on Zulip):

hmm

nikomatsakis (Oct 15 2018 at 16:06, on Zulip):

do you have a branch containing what you did so far @mikhail-m1?

mikhail-m1 (Oct 15 2018 at 16:18, on Zulip):

yep, but it's contains planty of ugly debug! calls because i didn't plan to share it, please ignore it - https://github.com/mikhail-m1/rust/commit/627ed91b1d75b7ee8e4850930b9339b21c08fc73

nikomatsakis (Oct 16 2018 at 13:45, on Zulip):

I'll take a look today

mikhail-m1 (Oct 16 2018 at 14:12, on Zulip):

thanks, I've amended the commit https://github.com/mikhail-m1/rust/commit/52847dfac566a4ffbb830aad92448f0791ef3a8c but it doesn't work

nikomatsakis (Oct 16 2018 at 21:50, on Zulip):

@mikhail-m1 so I had planned to do a bit of coding on this but I didn't get to it — I think though your problem is that you shouldn't (I don't thnk) be editing universal_regions.rs at all

nikomatsakis (Oct 16 2018 at 21:50, on Zulip):

rather, I expect you to extract the user-provided fn sig somewhere in this function

nikomatsakis (Oct 16 2018 at 21:50, on Zulip):

and then to "instantiate" the canonical signature into variables and do the unification

nikomatsakis (Oct 16 2018 at 21:51, on Zulip):

though that last step will be easier once https://github.com/rust-lang/rust/pull/55093 lands

pnkfelix (Oct 16 2018 at 21:51, on Zulip):

Do you think anyone would object to me raising the priority on that PR?

pnkfelix (Oct 16 2018 at 21:52, on Zulip):

I guess everyone has work they're stressed about landing in time for RC2

mikhail-m1 (Oct 16 2018 at 21:54, on Zulip):

Thanks, I will try it tomorrow. I can merge the PR to my branch.

nikomatsakis (Oct 16 2018 at 21:59, on Zulip):

Do you think anyone would object to me raising the priority on that PR?

I already did :)

mikhail-m1 (Oct 17 2018 at 22:14, on Zulip):

@nikomatsakis it's interesting after rebase to #55093 by code just starts work. But I am trying to move it to equate_inputs_and_outputs from univeral_regions but I have the same problem, I don't know how to convert PolyFnSig to Ty.

nikomatsakis (Oct 18 2018 at 09:38, on Zulip):

@mikhail-m1 so what we have to do is not to convert the polyfnsig into a type per se, but rather to (a) liberate the bound regions, yielding a FnSig ("liberate" meaning to replace them with suitable free regions) and then (b) relate the type of each argument from the inputs field to the corresponding type of the MIR argument

nikomatsakis (Oct 18 2018 at 09:38, on Zulip):

jfyi I may have a bit of time today and can take a look at a branch if you want (and/or hack on this)

nikomatsakis (Oct 18 2018 at 19:56, on Zulip):

(this is listed as an RC2 blocker, but I wonder @pnkfelix if we should demote it..?)

nikomatsakis (Oct 18 2018 at 19:57, on Zulip):

it will want to be backported but doesn't seem like it will be so terrible

pnkfelix (Oct 18 2018 at 22:24, on Zulip):

Demotion .... seems okay ....

mikhail-m1 (Oct 18 2018 at 22:25, on Zulip):

sorry, but looks like it's better to reassign this issue to someone else, I tried to find a way how to fix it but I didn't have enough time and next week will be the same. Sorry again for a taken time

nikomatsakis (Oct 19 2018 at 10:09, on Zulip):

@mikhail-m1 np at all! I'll take a look

nikomatsakis (Oct 20 2018 at 17:19, on Zulip):

posted https://github.com/rust-lang/rust/pull/55229

Last update: Nov 21 2019 at 14:55UTC