Skip to content

Comments

Fix Issue 17630 - selective imports find symbols in private imports o…#7760

Merged
dlang-bot merged 1 commit intodlang:masterfrom
RazvanN7:Issue_17630
Jan 23, 2018
Merged

Fix Issue 17630 - selective imports find symbols in private imports o…#7760
dlang-bot merged 1 commit intodlang:masterfrom
RazvanN7:Issue_17630

Conversation

@RazvanN7
Copy link
Contributor

…f other modules

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @RazvanN7! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

  • smaller, focused PRs are easier to review than big ones

  • try not to mix up refactoring or style changes with bug fixes or feature enhancements

  • provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Auto-close Bugzilla Description
17630 selective imports find symbols in private imports of other modules

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

@mathias-lang-sociomantic
Copy link
Contributor

Sounds like this needs to be a deprecation first :(

@wilzbach
Copy link
Contributor

Thanks a lot for moving this forward, but I fear that this needs a deprecation message instead of a hard error.

@andralex
Copy link
Member

I'm discussing with @RazvanN7, he just pushed a change that issues a deprecation.


A private module-level import should not have any symbols
visible outside the module scope. For more information
see https://issues.dlang.org/show_bug.cgi?id=17630
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer that the changelog entry is extended to explain the issue and the solution inline without having to read the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave it another shot but I used "imported module" a lot. :(

AliasDeclaration ad = imp.aliasdecls[i];
//printf("\tImport %s alias %s = %s, scope = %p\n", toPrettyChars(), aliases[i].toChars(), names[i].toChars(), ad._scope);
if (imp.mod.search(imp.loc, imp.names[i]))
if (imp.mod.search(imp.loc, imp.names[i]) /*, IgnorePrivateImports*/)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this comment here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the deprecation is over, that argument should ne uncommented

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

@andralex
Copy link
Member

Nice work!

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.

6 participants