Issue 15179 - Local imports cause outer imports to be excluded from overload set
Summary: Local imports cause outer imports to be excluded from overload set
Status: NEW
Alias: None
Product: D
Classification: Unclassified
Component: druntime (show other issues)
Version: D2
Hardware: All All
: P4 enhancement
Assignee: No Owner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-08 23:55 UTC by Jesse Phillips
Modified: 2024-12-07 13:35 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Jesse Phillips 2015-10-08 23:55:05 UTC
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"))
Comment 1 Kenji Hara 2015-10-09 02:04:14 UTC
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.
Comment 2 Steven Schveighoffer 2015-10-09 04:01:26 UTC
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
Comment 3 Walter Bright 2016-03-19 10:03:59 UTC
Probably not going to change the import lookup rules for this case.
Comment 4 Jesse Phillips 2016-03-22 23:13:56 UTC
(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
Comment 5 Steven Schveighoffer 2016-03-24 14:30:16 UTC
(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.
Comment 6 Jesse Phillips 2016-03-25 01:04:14 UTC
(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.
Comment 7 Steven Schveighoffer 2016-03-25 14:31:11 UTC
(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.
Comment 8 Jesse Phillips 2016-03-30 03:38:48 UTC
(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
Comment 9 Steven Schveighoffer 2016-03-31 13:37:11 UTC
(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.
Comment 10 dlangBugzillaToGithub 2024-12-07 13:35:50 UTC
THIS ISSUE HAS BEEN MOVED TO GITHUB

https://github.com/dlang/dmd/issues/17313

DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB