Mention deduplicated lifetimes

[Manishearth]

We currently deduplicate input lifetimes in rustc. This is a deliberate choice, however
it does not match this RFC. This PR amends this RFC to match the impl.

The impl currently allows the following code to compile:

fn foo<'a>(x: (&'a u8, &'a u8)) -> &u8 {}

by assigning 'a to all output lifetimes.

We also have what I consider as a bug in the current implementation, since deduplication does not
work across inputs. So, currently, the following will not compile

fn foo<'a>(x: &'a u8, y: &'a u8) -> &u8 {}

according to neither the impl nor the RFC. This pull request also makes that legal.

(Given that this is technically introducing new elision semantics, it might make more sense to do this as a separate RFC. However, I contend that the fact that deduplication doesn't work across inputs to be a bug, so it should just be fixed and amended into the existing RFC. I can split this out into a separate RFC if folks wish -- either move the entire concept of deduplication to a new RFC, or just the proposal that deduplication work across inputs.)

See also: rust-lang/rust#47929

[eddyb]

cc @nikomatsakis

[Manishearth]

I've got a PR at rust-lang/rust#48122 implementing this fix

[nikomatsakis]

This seems like a fine clarification. I'm going to take the liberty of just merging it.

To make my opinion clear:

As part of the move towards in-band lifetimes, I think we should deprecate the pattern where an elided lifetime refers to a named lifetime. If you have a name, use it. This would be a lint (at least in the current epoch).

However, having the behavior of fn foo((&'a, &'a)) -> & be inconsistent with fn foo(&'a, &'a) -> & seems unnecessary to me.

[nikomatsakis]

@rfcbot fcp merge

Well, having second thoughts about just merging. I'll do an fcp merge instead. But I still think we might as well merge this.

[rfcbot]

Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged teams:

• @aturon
• @cramertj
• @eddyb
• @nikomatsakis
• @nrc
• @pnkfelix
• @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[aturon]

I think we should deprecate the pattern where an elided lifetime refers to a named lifetime. If you have a name, use it.

I agree with this sentiment, but also agree that it's better to do that via a lint; this RFC amendment is fine with me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[withoutboats]

@rfcbot concern confused-boats

I'm not certain what we're approving here. If we're just approving acknowledging that the implementation works this way I don't really care, but if we're extending this behavior as suggested by the link to rust-lang/rust#47929, I'm against that.

I think that if you name a lifetime, it should only be used in places where you use that name. We should never expand an elided lifetime to an explicit lifetime; the point of being explicit is linking all the places where that name is used.

Rather than make this behavior work in more cases and then linting against them, I think we should lint against the cases where it works today and keeping it an error everywhere it is already an error. I think its more appropriate to treat the currently accepted cases as a bug & lint against them that way than it would be to lint against them as a style issue or something.

[Manishearth]

Yeah, there are two separate components to this. One acknowledges that the implementation works this way, and the other extends it to be more consistent.

I think we should either extend it to be consistent, or lint about things as you say; keeping the status quo is not great.

[withoutboats]

I think we should either extend it to be consistent, or lint about things as you say; keeping the status quo is not great.

Definitely! "The return type will refer to a named lifetime only if that lifetime is only used in 1 input argument and no other input arguments have a lifetime" is not a sensible elision rule.

[nikomatsakis]

I think we should either extend it to be consistent, or lint about things as you say; keeping the status quo is not great.

I definitely think we want to lint. The question is basically whether to make this change as well.

[Manishearth]

Oh, agreed, I should have been clearer.

[nox]

I think we should deprecate the pattern where an elided lifetime refers to a named lifetime. If you have a name, use it.

I strongly agree with this. It always looked strange to me that an elided output lifetime wouldn't always correspond to some other elided input lifetime or trigger a compile error.

[withoutboats]

Not sure how to make progress here. I'd like to cancel the FCP and instead treat the current implementation as a bug we want to phase out, but I have no idea what the rest of the lang team things.

[rfcbot]

The final comment period is now complete.

[withoutboats]

Removing FCP label because my concern hasn't been resolved.

[Manishearth]

@withoutboats Can i get soft consensus from the lang team on what we want to do here? I can make the RFC either way.

[nox]

I don't think this is something that can just be considered a bug and shrug the behaviour change in stable without an epoch or something like that. At the very least this should only be considered by introducing an error lint and doing a crater run with that.

[Manishearth]

I don't think anyone is proposing that; we're proposing deprecation (could epoch-gate).

This pattern is already relied upon by users, we don't need a crater run to know that.

[nox]

@Manishearth My bad, it wasn't really clear what was meant by "treat the current implementation as a bug we want to phase out".

[Manishearth]

deprecation and eventual epoching I believe.

[nikomatsakis]

@withoutboats

I'd like to cancel the FCP and instead treat the current implementation as a bug we want to phase out, but I have no idea what the rest of the lang team things.

Can you clarify what you want? That we do not expand the set of cases that are accepted, and instead just focus on the lint?

cc rust-lang/rust#48686 btw, which is another instance of this lint

[Manishearth]

(Did we make any progress on figuring out what we want here?)

[nikomatsakis]

I was just wondering the same thing. I'm nominating to discuss in lang team meeting since I don't really know what our position is here.

[nikomatsakis]

OK, we discussed in the lang team meeting. The general feeling was that if @withoutboats strongly objects to accepting this, then it is also fine to instead just start linting (as part of in-band lifetimes transition) on those cases where elision is used to expand to a named lifetime, and not change the implementation to accept this.

Therefore, I now move to close. =)

@rfcbot fcp close

[nikomatsakis]

I wonder if rfcbot can handle that.

[Manishearth]

Should we be amending the RFC to reflect the state as it currently is? Perhaps as a parenthetical?

[nikomatsakis]

@Manishearth Personally, I'm not a big fan of amending RFCs, because I don't see them as the "source of truth". They snapshot a moment in time. Better would, I think, be to document elision in the reference manual -- and yes, describe the decision here and the reasoning behind it.

[Manishearth]

Hmm, fair. Will do.

[aturon]

RFC bot has gotten wedged, but I'm going to go ahead and close this as per final discussion.