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

Issue 10378 - Local imports hide local symbols #4915

Closed
wants to merge 4 commits into from

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Aug 21, 2015

https://issues.dlang.org/show_bug.cgi?id=10378

Reuse the symbol shadowing detection mechanism for WithStatement.

This PR is based on #4857, because the local import behavior change will hit issue 14858 in Phobos unittest.

@dlang-bot
Copy link
Contributor

Fix Bugzilla Description
10378 Prevent local imports from hiding local symbols

@MetaLang
Copy link
Member

I have revived this pull request here: #5353

@9rnsr
Copy link
Contributor Author

9rnsr commented Jan 17, 2016

Rebased.

@ibuclaw
Copy link
Member

ibuclaw commented Feb 4, 2016

Looks reasonable at a glance. I'll have a second look later.

@WalterBright
Copy link
Member

I want to spend some more time on this. At first glance, it seems like a lot of code just to add a check.

It's a special scope for the function local imports.
By the rewriting, a symbol shadowing by local imports will become equivalent with the shadowing by WithStatement.
@WalterBright
Copy link
Member

This PR deals with shadowing of local symbols within a function. It does not deal with shadowing when using an import in, say, a class declaration. For example:

int writeln() { return 3; }

struct S {
    import std.stdio;
    void bar() { assert(writeln() == 3); }
}

void main() {
    S s;
    s.bar();
}

fails to compile.

@WalterBright
Copy link
Member

I'm working on fixing this, so please, no overlapping efforts!

@9rnsr
Copy link
Contributor Author

9rnsr commented Feb 11, 2016

At first glance, it seems like a lot of code just to add a check.

The concept of this PR is, a symbol shadowing introduced by local imports, are essentially equivalent with the shadowing done by WithStatement. I think that using same mechanism for the two would increase language consistency.

@9rnsr
Copy link
Contributor Author

9rnsr commented Feb 11, 2016

This PR deals with shadowing of local symbols within a function.
It does not deal with shadowing when using an import in, say, a class declaration. For example:

In the example code, the import declaration is not local, it's in struct member scope (it affects all other member functions, if declared). So it's beyond the target of this PR.

@WalterBright
Copy link
Member

I understand. But this case is one of the things people complained about. I'm working on a fix that will address all cases of doing imports inside scopes.

@WalterBright
Copy link
Member

Anyhow, the reboot based on #deadalnix 's proposed algorithm is now in #5445

@quickfur
Copy link
Member

Now that #5445 is merged, can this PR be closed?

@ibuclaw
Copy link
Member

ibuclaw commented Feb 17, 2016

The tests may still be useful.

@quickfur
Copy link
Member

Should we merge the tests then?

@9rnsr
Copy link
Contributor Author

9rnsr commented Mar 19, 2016

The tests are useless anymore.

@9rnsr 9rnsr closed this Mar 19, 2016
@9rnsr 9rnsr deleted the fix10378 branch March 19, 2016 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants