Bug 15966 – [REG 2.071] {public,protected} imports in base class ignored on symbol lookup

Quite similar to https://issues.dlang.org/show_bug.cgi?id=15925 ``` module definitions; import core.exception; public class RangeAssertError : RangeError { @safe pure nothrow this( string msg, string file = __FILE__, size_t line = __LINE__, Throwable next = null ) { super( msg, file, line, next ); } // Since we import it as protected, subclasses should be able to override // this function without import. protected import core.exception; public AssertError getAssert (); } ``` ``` module usage; import definitions; public class RangeAssertErrorImpl : RangeAssertError { @safe pure nothrow this( string msg, string file = __FILE__, size_t line = __LINE__, Throwable next = null ) { super( file, line, next ); } public override AssertError getAssert () { return null; } } ``` Works in DMD 2.070, not in 2.071. Tested with public import as well.

I've seen this in the code while fixing 15897 and am not sure why this decision was made. Let's figure it out and prolly rediscuss it, having public/private imports behave in classes like everywhere else seems like a simple decision.

We definitely shouldn't break code, but I also don't see a reason why we should ignore public imports in base classes. Maybe we can find the prior forum discussion that Walter mentioned.

The trouble is that imports in the base class will override global imports referred to by the derived class. This is exactly the kind of problem the import changes were so strongly desired to fix. The compiler is working as designed, this is deliberate behavior.

There was a PR for this: https://github.com/dlang/dmd/pull/5749

> Since we import it as protected, subclasses should be able to override > this function without import This is easily resolved by adding: import core.exception : AssertError;

(In reply to Walter Bright from comment #3) > The trouble is that imports in the base class will override global imports > referred to by the derived class. This is exactly the kind of problem the > import changes were so strongly desired to fix. > > The compiler is working as designed, this is deliberate behavior. I don't see why it should be a problem. A base class in another module can already override global imports by introducing new local symbols. It's expected when changing a base class than touching any non-private symbol can affect derived class.

(In reply to Walter Bright from comment #3) > The trouble is that imports in the base class will override global imports > referred to by the derived class. This is exactly the kind of problem the > import changes were so strongly desired to fix. > > The compiler is working as designed, this is deliberate behavior. Even if desired we'd at least have to integrate this w/ our -transition=checkimport/imports infrastructure. Should this really apply to public imports as well?

(In reply to Mathias Lang from comment #6) > A base class in another module can > already override global imports by introducing new local symbols. The problem is if symbols are added to that import, then they have an unintended side effect of overriding local symbols in the derived class code. Again, this is exactly the kind of problem this change was designed to stop.

(In reply to Martin Nowak from comment #7) > Should this really apply to public imports as well? Yes.

NB: please make sure that if this is not allowed anymore, it is appropriately covered by -transition=checkimport , as far as I know we had quite some code relying on such behaviour as an intended feature.

(In reply to Walter Bright from comment #8) > (In reply to Mathias Lang from comment #6) > > A base class in another module can > > already override global imports by introducing new local symbols. > > The problem is if symbols are added to that import, then they have an > unintended side effect of overriding local symbols in the derived class code. > > Again, this is exactly the kind of problem this change was designed to stop. They will only override imported symbols from outer scopes, not local ones. So it would override module imports, but not module aliases, for example. The same problem exists for mixin template, and by extension any "non-final scope" have this "issue", but the recent change are much more sane and easy to work with. If you don't want this behavior, then `import` from base classes should not be visible at all (even within the same module), or we'll end up in a weird place with 2 or more different rules. There is a better description of how I thought it would / should work as part of: https://issues.dlang.org/show_bug.cgi?id=16004

(In reply to Dicebot from comment #10) > NB: please make sure that if this is not allowed anymore, it is > appropriately covered by -transition=checkimport , as far as I know we had > quite some code relying on such behaviour as an intended feature. Yes, we'll make sure this is the case before the next release. (In reply to Mathias Lang from comment #11) > If you don't want this behavior, then `import` from base classes should not > be visible at all (even within the same module), or we'll end up in a weird > place with 2 or more different rules. It isn't visible even in the same module, see https://github.com/dlang/dmd/blob/7d00095301c4780b41addcfeb50f4743a9a6c5d4/test/fail_compilation/lookup.d.

Works as expected with -transition=import/checkimports and that behavior is already covered by test cases.