Remove label/lifetime shadowing warnings

[cjgillot]

This PR removes some pre-1.0 shadowing warnings for labels and lifetimes.

The current behaviour of the compiler is to warn

• labels that shadow unrelated labels in the same function --> removed

'a: loop {}
'a: loop {} // STOP WARNING

• labels that shadow enclosing labels --> kept, but only if shadowing is hygienic

'a: loop {
  'a: loop {} // KEEP WARNING
}

• labels that shadow lifetime --> removed

fn foo<'a>() {
  'a: loop {} // STOP WARNING
}

• lifetimes that shadow labels --> removed

'a: loop {
  let b = Box::new(|x: &i8| *x) as Box<dyn for <'a> Fn(&'a i8) -> i8>; // STOP WARNING
}

• lifetimes that shadow lifetimes --> kept

fn foo<'a>() {
  let b = Box::new(|x: &i8| *x) as Box<dyn for <'a> Fn(&'a i8) -> i8>; // KEEP WARNING
}

Closes #31745.

---

From @petrochenkov in #95781 (comment)

I think we should remove these silly checks entirely.
They were introduced long time ago in case some new language features appear and require this space.
Now we have another mechanism for such language changes - editions, and if "lifetimes in expressions" or something like that needs to be introduced it could be introduced as an edition change.
However, there was no plans to introduce anything like for years, so it's unlikely that even the edition mechanism will be necessary.

r? rust-lang/lang

[rust-highfive]

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

[petrochenkov]

r? @petrochenkov

[petrochenkov]

As I see shadowing outer lifetimes/labels with inner lifetimes/labels is still prohibited in this PR.
This is also a mismatch with regular names, where shadowing is allowed, like type parameters (correspond to lifetimes) and local variables (correspond to labels), and I think these errors can also be removed, but it's a separate question.

[petrochenkov]

It would be better if this PR only removed the warnings from compiler/rustc_resolve/src/late/lifetimes.rs, without moving them to AST first.

[petrochenkov]

@rust-lang/lang
This PR removes warnings for collisions between unrelated labels in the same function

fn check<'a>() {
    'a: loop {}
    'a: loop {} // warning: label name `'a` shadows a label name that is already in scope
}

and between lifetimes and labels

fn check<'a>() {
    'a: loop {} // warning: label name `'a` shadows a lifetime name that is already in scope
}

These warnings were introduced as a part of pre-1.0 future-proofing in #24162.
With introduction of editions, and with no plans in sight to introduce features that would benefit from this future proofing, such warnings no longer make sense, especially given that they have hygiene issues that were never addressed (#24278).

Lifetimes and labels are syntactically different from regular paths, but from name resolution point of view they are equivalent to type parameters and local variables (they live in two separate namespaces, type/lifetime and value/label correspondingly, and have same hygiene rules).
So it would be reasonable for them to have same restrictions on shadowing and other name collisions, that means no restrictions.

[joshtriplett]

I agree. Labels shouldn't be in scope outside the block they label, since we don't have goto. If we ever need a feature that would require this, we can introduce it using an edition.

[joshtriplett]

T-lang, do we have consensus to make label scopes limited to their block, and eliminate spurious shadowing warnings?

@rfcbot merge

[rfcbot]

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

• @cramertj
• @joshtriplett
• @nikomatsakis
• @pnkfelix
• @scottmcm

Concerns:

• doc-pr resolved by Remove label/lifetime shadowing warnings #96296 (comment)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), 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.

[scottmcm]

This PR removes warnings for collisions between unrelated labels in the same function

Can you clarify what "unrelated" means here?

I'm 💯% in agreement with removing warnings like this one:

    'a: loop { break };
    'a: loop { break }; // warning: label name `'a` shadows a label name that is already in scope

Because there's no shadowing going on there at all, since this is an error:

2 |     'a: loop { break };
3 |     loop { break 'a };
  |                  ^^ undeclared label `'a`

But because hygiene hasn't happened for labels, it seems to me like it would still be good to have a warning for a situation like this

macro_rules! foo {
    ($e:expr) => {
        'a: loop {
            break 'a $e;
        }  
    };
}

'a: loop { 
    foo!(break 'a);

where it does something different from what would happen with bindings.

[petrochenkov]

@scottmcm

Can you clarify what "unrelated" means here?

Given two label definitions, if none of them refers to another when resolved as use.

'a: loop {}
'a: loop {} // unrelated

'a: loop {
    'a: loop {} // shadowing
}

The shadowing case restrictions should also be removed, just maybe in a separate PR.
Label resolution is otherwise equivalent to local variable resolution, and they do not restrict shadowing.

But because hygiene hasn't happened for labels

Labels are hygienic, they are resolved identically to local variables.
Only this specific warning didn't consider hygiene, because it was implemented separately from the main label resolution pass.

[petrochenkov]

r=me after squashing commits.
@rustbot author

[cjgillot]

@bors r=petrochenkov

[bors]

📌 Commit 2aa9c70 has been approved by petrochenkov

[bors]

⌛ Testing commit 2aa9c70 with merge 3a90bed...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 3a90bed to master...

[rust-timer]

Finished benchmarking commit (3a90bed): comparison url.

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

• Primary benchmarks: 😿 relevant regression found
• Secondary benchmarks: no relevant changes found

	mean1	max	count2
Regressions 😿 (primary)	2.3%	2.3%	1
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	2.3%	2.3%	1

Cycles

Results

• Primary benchmarks: 🎉 relevant improvement found
• Secondary benchmarks: 😿 relevant regression found

	mean1	max	count2
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	4.7%	4.7%	1
Improvements 🎉 (primary)	-2.2%	-2.2%	1
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	-2.2%	-2.2%	1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

1. the arithmetic mean of the percent change ↩ ↩²

2. number of relevant changes ↩ ↩²