resolve: Fix #23880, a scoping bug #31105
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
|
r? @nrc (or @nikomatsakis) |
|
your example code that's broken by your changes could be a compile-fail test |
| // option. This file may not be copied, modified, or distributed | ||
| // except according to those terms. | ||
|
|
||
| #![allow(unused)] |
There was a problem hiding this comment.
Could you add a comment describing what is being tested please?
|
@oli-obk true, but the run-pass test is testing the same thing |
|
@nrc I added a comment describing the test and improved the commit message. |
|
Could you add [breaking-change] to the commit message please? |
|
r+ with the change to the commit message |
jseyfried
force-pushed
the
fix_lexical_scoping
branch
from
1c07493 to
5821219
Compare
|
I updated the commit message. |
|
@jseyfried sorry, I should have been clearer. The subject line can stay the same, but you should add |
jseyfried
force-pushed
the
fix_lexical_scoping
branch
from
5821219 to
e1dd02c
Compare
This fixes a bug in which items in a block are shadowed by local variables and type parameters that are in scope.
It is a [breaking-change]. For example, the following code breaks:
```rust
fn foo() {
let mut f = 1;
{
fn f() {}
f += 1; // This will now resolve to the function instead of the local variable
}
}
```
Any breakage can be fixed by renaming the item that is no longer shadowed.
jseyfried
force-pushed
the
fix_lexical_scoping
branch
from
e1dd02c to
faf0852
Compare
|
@nrc Ok. I'm a little unclear on what should go in the merge commit message for a PR (in general) and what should go in the individual commit message(s). Sampling past PRs, including ones with breaking changes, it looks pretty common to have a detailed merge commit message and a simple one-liner for the individual commit(s). Is this bad practice? |
|
@jseyfried thanks, I am a little confused too - I didn't know bors uses the PR message as the merge commit message, so it would have been fine, I guess. I personally prefer having the breaking-change message in the commit which makes the change so that it is easy to find the relevant code, but I think official policy is just that breaking-change has to be in a commit message. |
|
@bors: r+ |
|
📌 Commit faf0852 has been approved by |
This fixes #23880, a scoping bug in which items in a block are shadowed by local variables and type parameters that are in scope. After this PR, an item in a block will shadow any local variables or type parameters above the item in the scope hierarchy. Items in a block will continue to be shadowed by local variables in the same block (even if the item is defined after the local variable). This is a [breaking-change]. For example, the following code breaks: ```rust fn foo() { let mut f = 1; { fn f() {} f += 1; // This will resolve to the function instead of the local variable } }
This fixes a bug (#31845) introduced in #31105 in which lexical scopes contain items from all anonymous module ancestors, even if the path to the anonymous module includes a normal module: ```rust fn f() { fn g() {} mod foo { fn h() { g(); // This erroneously resolves on nightly } } } ``` This is a [breaking-change] on nightly but not on stable or beta. r? @nikomatsakis
This fixes #23880, a scoping bug in which items in a block are shadowed by local variables and type parameters that are in scope.
After this PR, an item in a block will shadow any local variables or type parameters above the item in the scope hierarchy. Items in a block will continue to be shadowed by local variables in the same block (even if the item is defined after the local variable).
This is a [breaking-change]. For example, the following code breaks: