fix Issue 313 - Fully qualified names bypass private imports #5426
Conversation
|
|
Seems like we need to reconsider the semantics of |
MartinNowak
force-pushed
the
fix313
branch
2 times, most recently
from
3bee255
to
fc01a25
Compare
|
|
||
| /**************************************** | ||
| * Check access to package/module p for scope sc. | ||
| * |
There was a problem hiding this comment.
Need Ddoc style Params: section.
|
done |
| if (sc.scopesym && sc.scopesym.isPackageAccessible(p)) | ||
| return false; | ||
| } | ||
| deprecation(loc, "%s %s is not accessible here", p.kind(), p.toPrettyChars()); |
There was a problem hiding this comment.
Append something like Perhaps add 'import %s;', as it's helpful to include corrective action as a hint to the user.
|
done |
MartinNowak
force-pushed
the
fix313
branch
2 times, most recently
from
1ed6bed
to
d815495
Compare
C++ will always be needed, whether or not we've switched to D is irrelevant. |
https://yourlogicalfallacyis.com/begging-the-question |
Because porting the glue layer to D would require porting a lot of GCC to D. |
|
Added a simple BitArray implementation. |
- b/c we're using a global package tree, imported modules were accessible in other scopes using fully qualified names - maintain a whitelist of imported modules in the current scope
- to avoid phobos dependency
|
Anything left? |
|
LGTM. @WalterBright ? |
|
|
||
| ~this() | ||
| { | ||
| mem.xfree(ptr); |
There was a problem hiding this comment.
This will cause multiple frees when copied. Could you e,g. disable postblit?
|
|
||
| ~BitArray() | ||
| { | ||
| mem.xfree(ptr); |
There was a problem hiding this comment.
this, too, is incorrect C++ code
|
If we leave BitArray radioactive like this one day or another it'll ruin someone's day. |
|
done |
|
There is one aspect of this change I'm not 100% sure of. Should the fully qualified module name of a public import in an imported module be available? module a;
import b;
void test()
{
core.stdc.stdio.printf("");
b.core.stdc.stdio.printf("");
}module b;
public import core.stdc.stdio;
public static import core.stdc.stdlib; // or maybe a public static importI stongly tend towards deprecating this as well, because it's relying on the exact imports in another module leaks implementation details. The purpose of public imports is to delegate functionality to another module, which module is that is should be the decision of the implementation as in this example. version (UseA)
public import drivers.a;
else
public import drivers.b;If reexporting a module is really wanted it's possible to add an explicit alias. public import cstdio=core.stdc.stdio;
alias driver = drivers.a; |
|
Alrighty then. Let's cross the Rubicon! |
|
Auto-merge toggled on |
fix Issue 313 - Fully qualified names bypass private imports
|
Great, I already spent 1-2 hour on the original BitArray implementation debugging a memory corruption b/c I mixed up number of words w/ number of bytes. |
fixup for #5426 - must clean newly reallocated bytes in BitArray
- public imports in imported modules were not accessbile using their FQN, this was an oversight when adding the package tree masking to fix Bugzilla 313 (see dlang#5426) - fixed by recursively checking imported scopes for accessible packages - Uses the same reduced visibility distinction (only private vs. rest) as the unqualified symbol search, b/c extending importedScopes to track rich visibility (e.g. package(a.b)) was out of scope for a regression fix. This should be implemented when combining the search/import w/ the symbol visibility mechanism.
- public imports in imported modules were not accessbile using their FQN, this was an oversight when adding the package tree masking to fix Bugzilla 313 (see dlang#5426) - fixed by recursively checking imported scopes for accessible packages - Uses the same reduced visibility distinction (only private vs. rest) as the unqualified symbol search, b/c extending importedScopes to track rich visibility (e.g. package(a.b)) was out of scope for a regression fix. This should be implemented when combining the search/import w/ the symbol visibility mechanism.
- public imports in imported modules were not accessbile using their FQN, this was an oversight when adding the package tree masking to fix Bugzilla 313 (see dlang#5426) - fixed by recursively checking imported scopes for accessible packages - reuse Module.insearch to not follow import cycles - Uses the same reduced visibility distinction (only private vs. rest) as the unqualified symbol search, b/c extending importedScopes to track rich visibility (e.g. package(a.b)) was out of scope for a regression fix. This should be implemented when combining the search/import w/ the symbol visibility mechanism.
- public imports in imported modules were not accessbile using their FQN, this was an oversight when adding the package tree masking to fix Bugzilla 313 (see dlang#5426) - fixed by recursively checking imported scopes for accessible packages - reuse Module.insearch to not follow import cycles - Uses the same reduced visibility distinction (only private vs. rest) as the unqualified symbol search, b/c extending importedScopes to track rich visibility (e.g. package(a.b)) was out of scope for a regression fix. This should be implemented when combining the search/import w/ the symbol visibility mechanism.
|
Edit: this is already fixed on master. |
|
This pull request introduced a regression: This variation is still present on master. |
accessible in other scopes using fully qualified names
The implementation is a bit more complicated than I'd like it to be, mostly b/c of package modules (handling of which is spreaded accross many places), and the fact that a module can have a different name on the import side than in it's module declaration.