Thread overview
Re: Pathological import symbol shadowing
Nov 18, 2020
H. S. Teoh
Nov 18, 2020
matheus
Nov 18, 2020
Paul Backus
November 18, 2020
On Wed, Nov 18, 2020 at 10:14:44AM +0000, FeepingCreature via Digitalmars-d wrote: [...]
> This is why our style guide doesn't allow unqualified imports in functions.  The more I think about it, the more I suspect function local imports were just a mistake to begin with.
> 
> Bouncing back from module scope into import scope is just fundamentally bad for understandability. The set of relevant symbols goes from "all the ones imported" to "all the ones in the module" back to "all the ones imported" again, with imports shadowing local functions, which is exactly the opposite of what should happen.

There are several factors at work here.

Local imports are good in the sense that you minimize namespace pollution: import the symbol only where you need them, not anywhere else.

They are also good for encapsulation: cut and paste your function into another module, and it Just Works.  Whereas if it depended on symbols imported at the module level, you now have to copy-n-paste that import into the new module.  And we all know what happens to that blob of imports at the top of the file: it just keeps growing and growing, and nobody dares delete it because you don't know what else might depend on it. (Well, theoretically it's easy, just delete them one by one and put back whichever causes a compile error. In practice, though, that's too much work just for deleting a line of code, so nobody does it.)

The problems come from *unqualified* local imports, and on this point I agree with you that unqualified local imports are generally a bad idea. We've known of this since a long time ago, actually:

	void fun(string text) {
		import std.conv;
		writeln(text);
	}
	void main() {
		fun("hello world");
	}

This (used to) output a blank line, because `text` got hijacked by std.conv.text.  IIRC somebody subsequently merged a DMD PR to change the lookup rules to fix this case.  I thought that PR solved the problem somehow, but as Walter said, it led to one special case after another. And now here we are.


> Putting aside how Walter is completely correct that "import std" was a mistake, I'd go further and also remove unqualified module imports completely. There's a set of features that are good for writing but bad for reading. Generally as time goes on, people discover those features are bad ideas, but because writing comes before reading, everyone thinks they're a good idea at first, so languages are doomed to reinvent them.

I don't agree that `import std` is a mistake.  It's a wonderful shortcut for one-off scripts and `echo ... | dmd -` one-liners.  Having to explicitly spell out individual Phobos modules would completely kill the latter usage because it becomes too onerous an effort for something that's meant to be a one-time quick hack.

In fact, even in larger programs I think `import std` is a net plus, because as somebody has already said, who really cares about that blob of imports at the top of the file spelling out individual Phobos modules?  Nobody reads it, and nobody cares except to satisfy the compiler.  I can understand if you're importing some 3rd party library and the compiler needs to be told where to find those symbols, but this is the *standard* library, darnit.  It's supposed to be standard symbols common across all D programs (more or less). Having to spell that out every single time I use it is just Java-style needless boilerplate.

But I do agree that unqualified `import std` as a local import is a bad idea, since it pulls in a whole truckload of symbols that, in all likelihood, will clash with / shadow local symbols.  As I said, in general unqualified local imports are bad news, because you don't know what symbols might get pulled in; a local symbol can easily get hijacked by a new symbol added to some 3rd party dependency.

Though I have to say, even despite the landmines, local `import std` is useful for inserting temporary one-off debug code into otherwise import-clean code. I often insert `import std; stderr.writefln("debug info");` into strategic points in my code while debugging. It gives you instant access to regular Phobos algorithms and functions without the tedium of painstakingly spelling out every module you might need.  And it's temporary and fits on one line, so it's just as easy to delete afterwards without polluting the rest of the code.

So love it or hate it, `import std` *does* have its uses.

In this particular instance of problems caused by it, my opinion is leaning towards std.curl.get being poorly-named. HTTP GET is a rather narrow and specific function, as opposed to object.get for AA's which are built into the language.  If it weren't for that ever-looming spectre of breaking existing code, I'd say rename it to something else. Like curlGet or something.


T

-- 
First Rule of History: History doesn't repeat itself -- historians merely repeat each other.
November 18, 2020
On Wednesday, 18 November 2020 at 18:10:58 UTC, H. S. Teoh wrote:
> Local imports are good in the sense that you minimize namespace pollution: import the symbol only where you need them, not anywhere else.

> They are also good for encapsulation: cut and paste your function into another module, and it Just Works.  Whereas if it depended on symbols imported at the module level, you now have to copy-n-paste that import into the new module.  And we all know what happens to that blob of imports at the top of the file: it just keeps growing and growing, and nobody dares delete it because you don't know what else might depend on it.

Yes that's true, but on the other hand you will need to duplicate or repeat yourself with local imports, like:

void bar(){
    import std.stdio;
    writeln("bar");
}

void foo(){
    import std.stdio;
    writeln("foo");
}

void main(string[ ] args){
    foo();
    bar();
}

Wouldn't this generate some pollution as well as your program grows?

Matheus.
November 18, 2020
On Wednesday, 18 November 2020 at 18:10:58 UTC, H. S. Teoh wrote:
> In this particular instance of problems caused by it, my opinion is leaning towards std.curl.get being poorly-named. HTTP GET is a rather narrow and specific function, as opposed to object.get for AA's which are built into the language.  If it weren't for that ever-looming spectre of breaking existing code, I'd say rename it to something else. Like curlGet or something.

std.curl.get is absolutely not at fault here. The whole point of modules (and namespaces in general) is that you can use whatever names you want in them without having to worry about conflicts. And if you need to use two different `get`s in the same scope, D has plenty of tools to help you disambiguate:

    import curl = std.curl;
    // ...
    curl.get(whatever);

    import std.curl;
    alias curlGet = std.curl.get;
    // ...
    curlGet(whatever);

    static import std.curl;
    // ...
    std.curl.get(whatever);