Stream: t-compiler/help

Topic: elided lifetime parameter names and async handling?


mark-i-m (Aug 21 2019 at 18:58, on Zulip):

Hi @nikomatsakis

I'm having a bit of trouble in https://github.com/rust-lang/rust/pull/58281. It seems that for some async fns, elided lifetimes produce regions that are actually called '_ now? To the best of my understanding, this is due to the changes in https://github.com/rust-lang/rust/pull/63501

The problem is that now, we don't properly generate a new lifetime for emitting diagnostics, so with my changes, we now have error messages like

help: consider adding the following bound: `'_: '_`

Based on my limited understanding, it seems like sometimes, we should be generating ParamName::Fresh rather than ParamName::Plain when we lower async fns. Any thoughts?

mark-i-m (Aug 26 2019 at 00:27, on Zulip):

not sure if @Matthew Jasper or @pnkfelix would be able to help too...

nikomatsakis (Aug 29 2019 at 15:36, on Zulip):

@mark-i-m hmm

nikomatsakis (Aug 29 2019 at 15:36, on Zulip):

I think I made some changes in this regard, actually

nikomatsakis (Aug 29 2019 at 15:37, on Zulip):

In particular, using ParamName::Fresh to generate elided lifetimes

nikomatsakis (Aug 29 2019 at 15:38, on Zulip):

That said: the problem of "anonymous lifetimes" is one of the reasons the NLL errors were introducing names for lifetimes and things, right? I guess I'd have to read a bit more into the code. But in the meantime, I'd say, try rebasing and see if things are any better? If not, ping me again and we can dig a bit more in :)

mark-i-m (Aug 29 2019 at 18:57, on Zulip):

@nikomatsakis I rebased, but am still getting the same '_ lifetimes.

mark-i-m (Aug 29 2019 at 18:58, on Zulip):

the problem of "anonymous lifetimes" is one of the reasons the NLL errors were introducing names for lifetimes and things, right?

yes, that's my understanding, which is why I think somewhere we have a bug that fails to recognize anon lifetimes...

nikomatsakis (Sep 04 2019 at 19:41, on Zulip):

Oh, I was confusing this and https://github.com/rust-lang/rust/pull/63678/

mark-i-m (Sep 10 2019 at 23:30, on Zulip):

@nikomatsakis Do you have any tips on how to debug the source of the lifetime names? I would rather not read the entire librustc_mir looking for them :P

nikomatsakis (Sep 12 2019 at 14:04, on Zulip):

@mark-i-m yeah, sorry, let me schedule some time to look into this. i've added an hour tomorrow, though I might bump it up to today. In the meantime I'll get a build going. I'll post you a response after that!

mark-i-m (Sep 12 2019 at 18:24, on Zulip):

btw, it looks like this only happens in async tests, so perhaps it is related to inferred member lifetimes?

nikomatsakis (Sep 13 2019 at 19:37, on Zulip):

@mark-i-m what's an example test where this occurs?

nikomatsakis (Sep 13 2019 at 19:38, on Zulip):

I guess async-await/issues/issue-63388-1.rs

mark-i-m (Sep 13 2019 at 21:05, on Zulip):

Sorry I'm not at my computer atm, but the nll compare mode tests on m branch do this

mark-i-m (Sep 13 2019 at 22:04, on Zulip):

Here is the full list of tests:

mark-i-m (Sep 13 2019 at 22:04, on Zulip):
$ git status
On branch synthesis
Your branch is up to date with 'origin/synthesis'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

    modified:   src/test/ui/async-await/issues/issue-63388-1.nll.stderr
    modified:   src/test/ui/c-variadic/variadic-ffi-4.nll.stderr
    modified:   src/test/ui/self/arbitrary_self_types_pin_lifetime_impl_trait-async.nll.stderr
    modified:   src/test/ui/self/arbitrary_self_types_pin_lifetime_mismatch-async.nll.stderr
    modified:   src/test/ui/self/elision/lt-ref-self-async.nll.stderr
    modified:   src/test/ui/self/elision/ref-mut-self-async.nll.stderr
    modified:   src/test/ui/self/elision/ref-mut-struct-async.nll.stderr
    modified:   src/test/ui/self/elision/ref-self-async.nll.stderr
    modified:   src/test/ui/self/elision/ref-struct-async.nll.stderr
mark-i-m (Oct 03 2019 at 13:55, on Zulip):

Hi @nikomatsakis do you have any suggestions on how to debug this

mark-i-m (Oct 03 2019 at 13:55, on Zulip):

I'm particular i want to trace the origins of a region name

mark-i-m (Nov 21 2019 at 17:39, on Zulip):

Some more updates here

mark-i-m (Nov 21 2019 at 17:42, on Zulip):

It looks like the regions with name '_ are considered NamedFreeRegions by the code in region_name.rs. I suspect that this is coming from ty::ReFree in give_name_from_error_region, and will keep tracing it back...

mark-i-m (Nov 21 2019 at 19:40, on Zulip):

Specifically, it is coming from a ty::BoundRegion::BrNamed where the name is _

mark-i-m (Nov 21 2019 at 19:42, on Zulip):

@nikomatsakis Any thoughts on where to go from here? It seems like there are a billion places a BrNamed could be coming from...

mark-i-m (Nov 21 2019 at 19:42, on Zulip):

Do you know which one is mostly to be caused by an implicit lifetime on an async function return type?

mark-i-m (Nov 21 2019 at 19:45, on Zulip):

Or alternately, perhaps a call to EarlyBoundRegion::has_name was missed somewhere when converting to a BoundRegion?

mark-i-m (Nov 21 2019 at 20:02, on Zulip):

Hmm... possibly this is the culprit:

mark-i-m (Nov 21 2019 at 20:02, on Zulip):
[DEBUG rustc_mir::borrow_check::nll::universal_regions] defining_ty (pre-replacement): for<'a, '_> fn(&'a Xyz, &dyn Foo) -> impl std::future::Future {Xyz::do_sth}
mark-i-m (Nov 21 2019 at 20:03, on Zulip):

It seems that the '_ is created as a BrNamed rather BrAnon at some point... Or am I misunderstanding?

Matthew Jasper (Nov 21 2019 at 20:05, on Zulip):

You should be able to see that with -Zverbose. But that sounds correct.

mark-i-m (Nov 21 2019 at 20:08, on Zulip):

@Matthew Jasper where would I see it? Would it be before MIR creation?

Matthew Jasper (Nov 21 2019 at 20:08, on Zulip):

I meant see the variant name in the logs.

mark-i-m (Nov 21 2019 at 20:08, on Zulip):

Oh yes, it is BrNamed

mark-i-m (Nov 21 2019 at 20:09, on Zulip):

I guess my question is should it be BrAnon? And where would that be constructed in the first place?

mark-i-m (Nov 21 2019 at 20:11, on Zulip):

@Matthew Jasper In particular, I see the following in the logs:

[DEBUG rustc::ty::sty] type_flags(ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(15), '_))) = HAS_RE_LATE_BOUND
mark-i-m (Nov 21 2019 at 20:11, on Zulip):

How can I find out where this was created?

Matthew Jasper (Nov 21 2019 at 20:12, on Zulip):

librustc/ty/mod.rs:853

Matthew Jasper (Nov 21 2019 at 20:15, on Zulip):

I don't think that we can create an BrAnon there correctly: I don't think that the index of the EarlyBoundRegion is compatible with the index in BrAnon.

mark-i-m (Nov 21 2019 at 20:17, on Zulip):

Hmm... I'm not very knowledgeable about the indexing system... what exactly is the difference?

mark-i-m (Nov 21 2019 at 20:17, on Zulip):

Or really why are they different?

Matthew Jasper (Nov 21 2019 at 20:19, on Zulip):

The early bound index is the position in ty::Generics.parms, the late bound index is some unique index for the corresponding Binder.

mark-i-m (Nov 21 2019 at 20:19, on Zulip):

Also, do you know where to_bound_region is called from? rg doesn't seem to find anything for some reason...

mark-i-m (Nov 21 2019 at 20:19, on Zulip):

Hmm... I see

Matthew Jasper (Nov 21 2019 at 20:26, on Zulip):

Apparently it isn't. It's actually in librustc/middle/resolve_lifetime.rs:96, but the idea's the same.

Matthew Jasper (Nov 21 2019 at 20:26, on Zulip):

The generation of the '_-named region, that is.

mark-i-m (Nov 21 2019 at 23:34, on Zulip):

So if I understand correctly, using the BrNamed in this case is the correct thing to do?

mark-i-m (Nov 21 2019 at 23:34, on Zulip):

Would the best fix then be to check the name better when finding the name to print in error messages in rustc_mir::borrow_check::nll::region_infer::error_reporting::region_name?

Matthew Jasper (Nov 21 2019 at 23:39, on Zulip):

Checking the name seems fine.

mark-i-m (Nov 21 2019 at 23:48, on Zulip):

Ok, thanks for the help... I will play around with this and maybe put up a PR at some point

Last update: Sep 27 2020 at 14:45UTC