https://issues.dlang.org/show_bug.cgi?id=15925

--- Comment #15 from det <2krnk@gmx.net> ---
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.

--

https://issues.dlang.org/show_bug.cgi?id=15925

det <2krnk@gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://issues.dlang.org/sh
                   |                            |ow_bug.cgi?id=12735

--

https://issues.dlang.org/show_bug.cgi?id=15925

det <2krnk@gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://issues.dlang.org/sh
                   |                            |ow_bug.cgi?id=16287

--

https://issues.dlang.org/show_bug.cgi?id=15925

Mathias Lang <mathias.lang@sociomantic.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|INVALID                     |FIXED

--- Comment #16 from Mathias Lang <mathias.lang@sociomantic.com> ---
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.

--

https://issues.dlang.org/show_bug.cgi?id=15925

--- Comment #17 from Steven Schveighoffer <schveiguy@yahoo.com> ---
(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.

--

https://issues.dlang.org/show_bug.cgi?id=15925

--- Comment #18 from Mathias Lang <mathias.lang@sociomantic.com> ---
(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/nl2bgi$2oim$1@digitalmars.com ), so I took the pragmatic road and just fixed stuff.

--

https://issues.dlang.org/show_bug.cgi?id=15925

--- Comment #19 from det <2krnk@gmx.net> ---
(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.

--

https://issues.dlang.org/show_bug.cgi?id=15925

--- Comment #20 from Steven Schveighoffer <schveiguy@yahoo.com> ---
(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.

--

https://issues.dlang.org/show_bug.cgi?id=15925

--- Comment #21 from Steven Schveighoffer <schveiguy@yahoo.com> ---
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.

--

https://issues.dlang.org/show_bug.cgi?id=15925

--- Comment #22 from Mathias Lang <mathias.lang@sociomantic.com> ---
> 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 :-)

--