Local imports cause outer imports to be excluded from overload set

[dlangBugzillaToGithub]

Jesse Phillips reported this on 2015-10-08T23:55:05Z

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

CC List

• Walter Bright (@WalterBright)
• Jesse Phillips
• Steven Schveighoffer (@schveiguy)

Description

This very basic program built with 2.067 and does not compile with 2.068.2:



------------
import std.conv;

void main() {
import core.time;
	"7".to!uint;
}
-------------

test.d(5): Error: template core.time.to cannot deduce function from argument types !(uint)(string), candidates are:
C:\opt\dmd\windows\bin\..\..\src\druntime\import\core\time.d(1934):        core.time.to(string units, T, D)(D td) if (is
(_Unqual!D == TickDuration) && (units == "seconds" || units == "msecs" || units == "usecs" || units == "hnsecs" || units
 == "nsecs"))

[dlangBugzillaToGithub]

k.hara.pg commented on 2015-10-09T02:04:14Z

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.

[dlangBugzillaToGithub]

schveiguy (@schveiguy) commented on 2015-10-09T04:01:26Z

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

[dlangBugzillaToGithub]

bugzilla (@WalterBright) commented on 2016-03-19T10:03:59Z

Probably not going to change the import lookup rules for this case.

[dlangBugzillaToGithub]

Jesse.K.Phillips+D commented on 2016-03-22T23:13:56Z

(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

[dlangBugzillaToGithub]

schveiguy (@schveiguy) commented on 2016-03-24T14:30:16Z

(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.

[dlangBugzillaToGithub]

Jesse.K.Phillips+D commented on 2016-03-25T01:04:14Z

(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.

[dlangBugzillaToGithub]

schveiguy (@schveiguy) commented on 2016-03-25T14:31:11Z

(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.

[dlangBugzillaToGithub]

Jesse.K.Phillips+D commented on 2016-03-30T03:38:48Z

(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

[dlangBugzillaToGithub]

schveiguy (@schveiguy) commented on 2016-03-31T13:37:11Z

(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.