Preventing lifetime shadowing


#1

I have this proto-RFC to disallow lifetime shadowing. But now it occurs to me this might be better done with a lint. I am curious to get people’s opinions on the matter.

UPDATE: I have now converted the “proto-RFC” into a real rfc: https://github.com/rust-lang/rfcs/pull/459


#2

I’m okay with it being an error, and if a use case for shadowed parameters is found then it could be relaxed. (especially if it’s easier to implement it this way.)


#3

+1 to this RFC, it feels consistent to make it an error, since we already forbid shadowing types/traits at the module level. And with associated items, traits and impls will look more like modules.

Slightly on topic, should we also forbid/lint name clashes between generic input parameters (types/lifetimes) and associated output types/lifetimes? We don’t yet know how troublesome this may be in practice, since AI is still WIP, but it’s something to think about.


#4

I’m okay with either, but I would prefer having an error since it’s better for backwards compatibility – we can change it back if we want, but not if we lint it.

Also, the compiler suggestions should be fixed so that they don’t suggest a lifetime which may shadow an existing one (see https://github.com/rust-lang/rust/issues/18824)


#5

Very much agreed that lifetime shadowing should be disallowed. There’s basically never been a time where I wanted it.


#6

+1, slight preference for error over lint.


#7

Given how many times I’ve seen people on IRC or Reddit asking for help because they made this mistake… yeah. Definite +1.


#8

I really like this, and would be very fine with it being an error as well. Lifetime parameters idiomatically have one-letter names, and the have a wide-scope, so this would certainly be a win in my book.

On a related note, I’ve seen many people trip up with non-lifetime type parameter shadowing as well (the often seen “Found T but expected T” error).


#9

The RFC forbids both lifetime and type shadowing.


#10

That’s what I get for only giving it a quick skim. It even gives the exact error. Anyway, just turns it into even more of a Yay from me.


#11

Disallow it. Disallow it with fire. We can always make this more lenient in the future (though I can’t see why we’d ever want to).


#12

OK, based on this feedback I opened the RFC: https://github.com/rust-lang/rfcs/pull/459

Please comment there.