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

Fix Issue 11038 - static has no effect as a block attribute for imports #10541

Merged
merged 1 commit into from
Nov 18, 2019

Conversation

RazvanN7
Copy link
Contributor

@RazvanN7 RazvanN7 commented Nov 6, 2019

The main function of the compiler (mars.d) calls importAll on the module before doing any semantic analysis. Since for blocks such as static{}, the storage class is updated during semantic analysis, the imports inside static blocks are brought in the module scope which results in the present bug.

The fix is to update the isstatic field of imports when it is first used.

I know that this requires a deprecation cycle, but I suspect that this is not a prevalent pattern and maybe we can get away directly with an error. Let's see what the tests say.

Later edit; it seems that phobos has this pattern:

package template MemberFunctionGenerator(alias Policy)
{
private static:
    import std.format;
    /* use of format without fully qualified name in some function */
}

It seems we have to go through a deprecation cycle.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @RazvanN7! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
11038 enhancement static has no effect as a block attribute for imports

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#10541"

@RazvanN7
Copy link
Contributor Author

I think that this case requires way too much engineering to trigger a deprecation. Currently, dmd has a pass that loads the scope of top-level non-static imports and then the symbols are considered in the scope of the module. So, for a deprecation to be emitted, you would need to test every symbol that is searched if it came from a static import; this might add overhead to highly used path of the code, raising compilation times. I suggest we just fix phobos and ship this as is.

@RazvanN7
Copy link
Contributor Author

Ping. I think this one is ready to merge.

@thewilsonator
Copy link
Contributor

Auto-merge toggled on

@RazvanN7
Copy link
Contributor Author

Again, I can't see the label :-?

@thewilsonator thewilsonator merged commit 7bf58ff into dlang:master Nov 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants