Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Items with 'lexical' scope have poor scoping #23880

Closed
nrc opened this issue Mar 31, 2015 · 4 comments · Fixed by #31105
Closed

Items with 'lexical' scope have poor scoping #23880

nrc opened this issue Mar 31, 2015 · 4 comments · Fixed by #31105
Assignees
Labels
A-resolve Area: Name/path resolution done by `rustc_resolve` specifically P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nrc
Copy link
Member

nrc commented Mar 31, 2015

E.g.,

struct Foo;

impl Foo {
    fn bar(&self) {}

    fn foo(&self) {
        fn bar(x: &Foo) {}

        bar(self);
    }
}

fn main() {}

Is an error, but shouldn't be, because the bar method hides the locally declared bar function inside foo.

I think this is a bug, so it shouldn't be on any milestone, but nominating just in case.

@nrc nrc added I-wrong A-resolve Area: Name/path resolution done by `rustc_resolve` specifically labels Mar 31, 2015
@oli-obk
Copy link
Contributor

oli-obk commented Apr 1, 2015

It's not an error anymore: http://is.gd/CKvJn3

@nrc
Copy link
Member Author

nrc commented Apr 1, 2015

Ah curses, that isn't an error. Here's another go:

struct Foo<X> {
    x: Box<X>
}

impl<Bar> Foo<Bar> {
    fn foo(&self) {
        type Bar = i32;

        let _: Bar = 42;
    }
}

In this case the let statement should see the local type alias, but sees the type parameter.

@pnkfelix
Copy link
Member

pnkfelix commented Apr 2, 2015

classifying as "just a bug", P-high, not 1.0.

Actually leaving as P-backcompat-lang, to represent the fact that this can lead to programs being accpeted that shoudl be rejected. So we will keep that tag as is.

But still not 1.0 blocker.

@brson brson added P-high High priority and removed P-backcompat-lang labels Apr 29, 2015
@nrc nrc added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 2, 2015
@nikomatsakis
Copy link
Contributor

triage: P-medium

This has been sitting for a while and we don't expect immediate action, though hopefully a refactoring of resolve would lead to this being fixed.

@rust-highfive rust-highfive added P-medium Medium priority and removed P-high High priority labels Nov 12, 2015
jseyfried added a commit to jseyfried/rust that referenced this issue Jan 26, 2016
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.
bors added a commit that referenced this issue Jan 26, 2016
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
    }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-resolve Area: Name/path resolution done by `rustc_resolve` specifically P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants