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

Name resolution has changed between 1.7.0 and 1.8.0 #33458

Closed
matklad opened this issue May 6, 2016 · 10 comments
Closed

Name resolution has changed between 1.7.0 and 1.8.0 #33458

matklad opened this issue May 6, 2016 · 10 comments
Labels
T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@matklad
Copy link
Member

matklad commented May 6, 2016

The following program:

fn main() {
    let x: i32 = 92;
    {
        const x: i32  = 62;
        println!("{}", x);
    }
}

prints 92 with rustc 1.7.0 and 62 with rustc 1.8.0. Is this intentional? I have not found a mention in the release notes. Also the reference could be a bit more clear on the scoping rules :)

@jonas-schievink
Copy link
Contributor

cc @jseyfried @nrc

FWIW I think 62 makes more sense, intuitively speaking, but the subtle change is a bit terrifying

@nagisa nagisa added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels May 6, 2016
@nagisa
Copy link
Member

nagisa commented May 6, 2016

Nominating for T-lang discussion.

@jseyfried
Copy link
Contributor

This was intentional, see #31105.

@nagisa
Copy link
Member

nagisa commented May 7, 2016

This likely means that the change wasn’t communicated well enough?

@aturon
Copy link
Member

aturon commented May 12, 2016

Yes, for future reference, anything marked [breaking-change] should also have the relnotes tag applied.

@nikomatsakis
Copy link
Contributor

We discussed this in the @rust-lang/lang meeting. The conclusion was that the change was legit (a bug fix), but there was a process failure in that we failed to tag the breaking change with "relnotes". D'oh, but I think this change had only very minor impact in practice?

@matklad
Copy link
Member Author

matklad commented May 13, 2016

I think this change had only very minor impact in practice?

I also think so. I sometimes stumble upon such dark corners of the language, but it is only because we try to reimplement compiler fronted in IntelliJ Rust.

We probably should close the issue, thanks for the clarification!

@jseyfried
Copy link
Contributor

jseyfried commented May 14, 2016

I agree that this change had very minor impact in practice -- constants and local variables can only have the same name by violating naming conventions, and consts in deep blocks are already uncommon.

We have landed other [breaking-change] bug fixes with similarly low likelihoods of breakage (#32923, #32134, #32006, #31908, #31824, #30866, #30295, #30294, #32141, #31461) -- should these all have been tagged relnotes as well?

@shahn
Copy link
Contributor

shahn commented Jun 12, 2016

I think yes, they should be in the release notes. The release notes might help someone figuring out what's going on when they see some code breaking (maybe a breaking change had unintended/unforeseen consequences)

@matklad
Copy link
Member Author

matklad commented Oct 4, 2016

I think this is the intended behavior and the issue should be closed.

@matklad matklad closed this as completed Oct 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants