With the new import rules:
```
module A;
public mixin template AddImports ()
{
protected import core.thread;
Thread th;
}
```
```
module B;
import A;
public class Foo
{
mixin AddImports;
Thread t;
}
```
b.d(9): Error: undefined identifier 'Thread'
It used to compile in 2.070.
Since its not even a deprecation, and the specs mention it's possible ("The declarations in a mixin are ‘imported’ into the surrounding scope."), it looks like a regression.
What about public imports? Previously you have stated that mixins should be viewed identical to imports when it comes to symbol visibility/access but that would be not true if public ones are not propagated.
Comment #8 by code — 2016-05-26T17:43:02Z
Weird, need to work on this.
With 2.071.0 -transition=import works as expected (but -transition=checkimports doesn't report an error).
With the latest nightly dmd-2016-05-26 it will accept the code w/o -transition=import b/c we haven't yet merged back the revert.
With the latest stable 64f3fdb27 it will fail even with -transition=import.
The original bug report here, I think has some merit.
If you need to add imports to a class/struct/function in order to allow for your declaration to make sense, then I don't see a reason why it should be disallowed.
For instance, the original example imports core.thread, and then declares th of type Thread. You can use members of th inside Foo, but not declare another 'Thread'. You also could not use any UFCS functions defined in core.thread (if there are any). This leaves you with a half-working type. As far as I know, there's no workaround.
(In reply to Walter Bright from comment #5)
> This is not a bug, the behavior is on purpose and as designed. This was
> discussed at length for an analogous case, imports in base classes.
Where is the discussion? I want to add the explanation for this to the blog post I wrote on the import differences in 2.071, but I can't find any.
Comment #15 by 2krnk — 2016-07-18T03:26:16Z
having the status marked as RESOLVED FIXED is confusing at least - and i would argue plain wrong. walter's post declared the original report as invalid, so it has to be marked as such.
Comment #16 by mathias.lang — 2016-07-19T18:45:26Z
The original post / report was "Declaration from mixin templates are ignored".
The details of the bug were wrong (it should be ignored unlike what I originally believed), however the bug was still here: -transition=[check]imports did not take it into account, which was fixed in https://github.com/dlang/dmd/pull/5810
The issue was later renamed to better reflect where the bug actually was, and the changelog was edited to reflect the new name. Nothing ambiguous here.
Hence, I'm gonna revert the status change.
Comment #17 by schveiguy — 2016-07-19T19:15:50Z
(In reply to Mathias Lang from comment #16)
> The details of the bug were wrong (it should be ignored unlike what I
> originally believed)
While this has your attention, can you point me at the rationale to why this is so? See my earlier comment 14.
BTW, we shouldn't rename or repurpose bugs like this.
Comment #18 by mathias.lang — 2016-07-19T21:21:29Z
(In reply to Steven Schveighoffer from comment #17)
> (In reply to Mathias Lang from comment #16)
> > The details of the bug were wrong (it should be ignored unlike what I
> > originally believed)
>
> While this has your attention, can you point me at the rationale to why this
> is so? See my earlier comment 14.
Sorry, I forgot to reply to c14.
The rationale is that this behavior allowed symbol hijacking.
The behavior of the compiler was changed to perform 2 passes for symbol resolution, the first one on local symbol, the second one on imports.
E.g. the infamous:
```
static immutable text = "Hello world";
void main ()
{
import std.conv;
writeln(text);
}
```
Used to print nothing because the imported 'std.conv.text' symbol used to take priority. This proved to be a frequent source of confusion for developers.
See https://issues.dlang.org/show_bug.cgi?id=10378 for the actual issue.
Now there is something special about 'mixin template' and 'class' / 'interface' declaration: they are the only remote (as in, outside of the module) symbol one can inherit it's scope from.
But inheriting the imports leads to the same issues 10378 describe: you can end up in a situation where adding a symbol to module 'foo' which is 'protected import' of class 'A' will suddenly change which symbol is selected in subclass 'B : A'. However, if you have both imports at the same level, you'll get a conflict.
The only kind of hijacking I can think of ATM is when an import which is more nested than another one introduce an already-used symbol.
E.g. if this class used 'b.myFunc' before and a new library introduce 'd.myFunc':
```
import b;
class Foo
{
import d;
pragma(msg, typeof(myFunc));
}
```
Hope that was clear enough. Feel free to reach out to me by email if not.
> BTW, we shouldn't rename or repurpose bugs like this.
I generally agree. In this case, the P.R. was already merged and appeared in the changelog. It created confusion ( https://forum.dlang.org/post/[email protected] ), so I took the pragmatic road and just fixed stuff.
Comment #19 by 2krnk — 2016-07-19T23:27:46Z
(In reply to Mathias Lang from comment #18)
> The rationale is that this behavior allowed symbol hijacking.
>
> The behavior of the compiler was changed to perform 2 passes for symbol
> resolution, the first one on local symbol, the second one on imports.
> ...
> See https://issues.dlang.org/show_bug.cgi?id=10378 for the actual issue.
i think the problem steve and i have is that the communicated changes that came with 2.071 are the following two
1) don't be able to use symbols that are not visible (which is of course according to specs but was not enforced beforehand)
2) change in lookup sequence to prevent hijacking
(2) relates to your examples in c18 and https://issues.dlang.org/show_bug.cgi?id=10378
however, the issue here WRT mixin templates is the following:
it is NOT just (2) at work, i.e. a change in lookup sequence - which would be straightforward. but rather, whole module imports mixed in are completely ignored. why such an enforcement? it seems overly excessive and restrictive as just an application of (2) would have prevented hijacking.
and unlike the well communicated changes (1) and (2) this new behavior is NOT supported by the current language specs and has so far not been officially communicated anywhere.
when ppl's code fails (as happened to me), they search the specs, find nothing, search the bug tracker/issues, find this very example of the problem, and see it is marked as fixed - in contradiction to the issue being persistent. yes, the changed subject gives a hint but i still favor a split into a 2 issues:
o this one being 'resolved invalid'
o a new 'see also' linked issue that is 'resolved fixed'
and of course official documentation/specs that reflect the changes in behavior.
Comment #20 by schveiguy — 2016-07-20T01:05:10Z
(In reply to Mathias Lang from comment #18)
> Feel free to reach out to me by email if not.
In the interest of keeping the discussion public, I'll just reply here.
> Now there is something special about 'mixin template' and 'class' /
> 'interface' declaration: they are the only remote (as in, outside of the
> module) symbol one can inherit it's scope from.
>
> But inheriting the imports leads to the same issues 10378 describe: you can
> end up in a situation where adding a symbol to module 'foo' which is
> 'protected import' of class 'A' will suddenly change which symbol is
> selected in subclass 'B : A'. However, if you have both imports at the same
> level, you'll get a conflict.
I think what you are saying is that a mixed in template could affect existing *imports*, right? But an import can already do that! Ignoring imports with mixins doesn't fix this problem.
The mixin could not hijack local symbols, right? I thought that was the larger point -- you are referring to class/local symbols, or even module functions, and suddenly that changes through a hijacked import.
I get that it's good to prevent hijacking as much as possible. But this doesn't disallow hijacking to any further degree than is already disallowed. In other words, preventing mixins from hijacking imported symbols doesn't remove the problem, and just makes the language inconsistent.
> The only kind of hijacking I can think of ATM is when an import which is
> more nested than another one introduce an already-used symbol.
>
> E.g. if this class used 'b.myFunc' before and a new library introduce
> 'd.myFunc':
Right, but this still exists. Why prevent it in the case of a mixin?
For example, d could public import x. x could define myFunc. This is pretty much equivalent to the mixin case.
Comment #21 by schveiguy — 2016-07-20T01:43:26Z
To add to this, I think the use case being held as the example that shows why this was done is:
a.d:
module a;
struct S {}
void foo() {}
void m(S s) {}
// uncomment to hijack!
// void bar() {}
b.d:
module b;
class Foo
{
import a;
S s;
}
c.d:
module c;
void bar() {}
d.d:
module d;
import b;
import c;
foo();
class Bar : Foo
{
void x() {
foo();
bar();
s.m();
}
}
So x calling foo calls local foo. That's fine, with new import rules, nothing outside of d.d can change that. The fix is to avoid calling a.bar if it was added at a later time.
But what about the prevention of Bar from seeing m? Surely, when we want to include an S member, we want to include it's entire interface. Now if some other module defines an m that accepts an S, then s.m means something different in Bar than it does in Foo! This can't be the right solution, can it?
This seems like it was broken before, and is broken in a different way now. It's impossible to tell what the original code "wanted" to do. But it seems pretty clear to me that Foo has a certain view of what an S API looks like, and it meant to forward that view to its subclasses.
I can at least document the change in my blog article, because even if this decision is reversed, 2.071 is already out there.
Comment #22 by mathias.lang — 2016-07-23T03:18:28Z
> I think what you are saying is that a mixed in template could affect existing *imports*, right? But an import can already do that! Ignoring imports with mixins doesn't fix this problem.
An import can already hijack an import if it appears at scope lexically 'closer' to the looked-up symbol, but yes, imports can still hijack.
I don't know if there is any solution to that issue, short of allowing only one level of import, which of course is out of question.
> The mixin could not hijack local symbols, right? I thought that was the larger point -- you are referring to class/local symbols, or even module functions, and suddenly that changes through a hijacked import.
It could be implemented both ways, since those are two different lists of imports. I believe the sane way to do it (and the approach I used IIRC) would be to look up mixed in / inherited imports after local ones.
> I get that it's good to prevent hijacking as much as possible. But this doesn't disallow hijacking to any further degree than is already disallowed. In other words, preventing mixins from hijacking imported symbols doesn't remove the problem, and just makes the language inconsistent.
It disallows some imports, so it *does* prevent hijacking by definition.
Whether or not those hijackings are a common enough source of bugs, and the feature it disallows is useful, is up to one's judgment.
As for inconsistency, I disagree. With the separated lookup, there is no way to inherit an 'import scope'. This means any hijacking can only ever come from an import that is in your module. I believe that was the important property Walter was after: https://issues.dlang.org/show_bug.cgi?id=15966#c3
Note that as the original author of this issue (and the similar #15966) I'm only the messenger, and not the one that took the decision.
IOW, I'm fighting someone else's war here, and he's the one you have to convince if you want this overturned :-)
Comment #23 by github-bugzilla — 2016-10-01T11:45:31Z