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

Kenji Hara <k.hara.pg@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|dmd                         |druntime
           Hardware|x86                         |All
                 OS|Windows                     |All

--- Comment #1 from Kenji Hara <k.hara.pg@gmail.com> ---
This is caused by druntime change.

1. Local import does not make overload sets with the symbols come from outer imports. About that there's no specification and implementation change in 2.068.

2. In 2.068, a free function 'to' is added to core.time module.

https://github.com/D-Programming-Language/druntime/pull/1190

In the test code, it's hiding std.conv.to function and compilation fails.

--

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

Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy@yahoo.com

--- Comment #2 from Steven Schveighoffer <schveiguy@yahoo.com> ---
See my comments in the forum. I feel like this is really an issue that should be fixed in the compiler.

http://forum.dlang.org/post/mv7dla$ib1$1@digitalmars.com

--

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

Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla@digitalmars.com

--- Comment #3 from Walter Bright <bugzilla@digitalmars.com> ---
Probably not going to change the import lookup rules for this case.

--

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

Jesse Phillips <Jesse.K.Phillips+D@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |Jesse.K.Phillips+D@gmail.co
                   |                            |m

--- Comment #4 from Jesse Phillips <Jesse.K.Phillips+D@gmail.com> ---
(In reply to Walter Bright from comment #3)
> Probably not going to change the import lookup rules for this case.

Consider:

Module bar provides a function, foobar, that takes an int.

----------
module bar;

bool foobar(int i) {
    return true;
}
----------

Module foo defines foobar to take a string.

----------
module foo;

bool foobar(string m) {
    return false;
}
----------

Main calls bar.foobar and the assertion passes.

----------
import bar;

void main() {
import foo;
    assert(7.foobar);
}
----------


Module foo is modified, it now defines a function, foobar, that takes an int.

----------
module foo;

bool foobar(string m) {
    return false;
}

/// Highjack call to bar.foobar
bool foobar(int m) {
    return false;
}
----------

The assertion will now fail in main due to function highjacking (no compiler
error):

$  rdmd test.d .\foo.d .\bar.d

core.exception.AssertError@test.d(5): Assertion failure

--

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

--- Comment #5 from Steven Schveighoffer <schveiguy@yahoo.com> ---
(In reply to Jesse Phillips from comment #4)
> Main calls bar.foobar and the assertion passes.
> 
> ----------
> import bar;
> 
> void main() {
> import foo;
>     assert(7.foobar);
> }

I'm surprised this works, because I would expect foo to not overload with bar's functions, it has priority. I'm not sure how the current lookup rules work or the rules for the new system about to be released (with all the import rule fixes).

But in the final version, I would expect foo's foobar overload set to be prioritized, because it was imported locally and has priority.

But we can "fix" the demonstration by removing the foobar(string) from foo, so
it wouldn't have been used as an overload set in the first place (similar to
the original code example).

In any case, the solution here is to do a selective import in a scope, which works as expected. Alternatively, you can import both modules at the same scope so they are used at the same priority.

--

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

--- Comment #6 from Jesse Phillips <Jesse.K.Phillips+D@gmail.com> ---
(In reply to Steven Schveighoffer from comment #5)
> In any case, the solution here is to do a selective import in a scope, which works as expected. Alternatively, you can import both modules at the same scope so they are used at the same priority.

That doesn't solve the highjacking, Walter has made a big point about D's anti-highjacking feature.

> But in the final version, I would expect foo's foobar overload set to be prioritized, because it was imported locally and has priority.

I would not, this would be a huge hole in D's anti-highjacking features. I would expect local imports to be included in the lookup set just as global imports. Only would an alias create the priority, just as global imports.

--

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

--- Comment #7 from Steven Schveighoffer <schveiguy@yahoo.com> ---
(In reply to Jesse Phillips from comment #6)
> That doesn't solve the highjacking, Walter has made a big point about D's anti-highjacking feature.

Yes, it does. Selective imports override any non-selective import because it's aliased into the local namespace.

--

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

--- Comment #8 from Jesse Phillips <Jesse.K.Phillips+D@gmail.com> ---
(In reply to Steven Schveighoffer from comment #7)
> (In reply to Jesse Phillips from comment #6)
> > That doesn't solve the highjacking, Walter has made a big point about D's anti-highjacking feature.
> 
> Yes, it does. Selective imports override any non-selective import because it's aliased into the local namespace.

It does not. Since I wrote code where selective imports was not necessary, an update to a second library being used hijacked my call and the compiler gave no warning. That is the hijack problem, the fact that I can modify my code to use the desired function is not relevant to how the compiler is supposed to help prevent hijacking: http://dlang.org/hijack.html

--

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

Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|regression                  |enhancement

--- Comment #9 from Steven Schveighoffer <schveiguy@yahoo.com> ---
(In reply to Jesse Phillips from comment #8)
> It does not. Since I wrote code where selective imports was not necessary, an update to a second library being used hijacked my call and the compiler gave no warning. That is the hijack problem, the fact that I can modify my code to use the desired function is not relevant to how the compiler is supposed to help prevent hijacking: http://dlang.org/hijack.html

Of course selective imports aren't necessary. But importing locally without using selective imports is prone to this issue.

I would argue any time you do a scoped import of a module you don't control, you should use static, renamed, or selective imports. This narrows the focus of what you are looking for, and is not subject to overriding or hijacking since it's treated as a local alias.

D can do some things to prevent hijacking, but you must remember that it doesn't have a history of code. It cannot always deduce that something was hijacked and that's not what you intended.

Note, I just realized this isn't a regression, it's an enhancement. The import rules are working as they were designed. If we are to change anything, it would be a new design.

--

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

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P1                          |P4

--