Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.

Add imported for inline imports#1756

Merged
dlang-bot merged 4 commits into
dlang:masterfrom
andralex:from
Sep 15, 2021
Merged

Add imported for inline imports#1756
dlang-bot merged 4 commits into
dlang:masterfrom
andralex:from

Conversation

@andralex

@andralex andralex commented Feb 4, 2017

Copy link
Copy Markdown
Member

No description provided.

@JackStouffer

Copy link
Copy Markdown
Contributor

Is this intended to be a replacement or an addition to your DIP?

@andralex

andralex commented Feb 4, 2017

Copy link
Copy Markdown
Member Author

@JackStouffer looks like a solid candidate for replacement.

@JackStouffer

Copy link
Copy Markdown
Contributor

In this trivial example I am reading massive speedups. This looks promising.

But, the real test would converting a whole module from Phobos to this style, with no top level imports, and seeing the results.

import std.datetime;
import std.traits;

void func(T)(SysTime a, T value) if (isIntegral!T)
{
    import std.stdio : writeln;
    writeln(a, value);
}

void main() {}
$ time dmd test2.d
dmd test2.d  0.29s user 0.07s system 95% cpu 0.378 total

VS

template from(string moduleName)
{
    mixin("import from = " ~ moduleName ~ ";");
}

void func(T)(from!"std.datetime".SysTime a, T value) if (from!"std.traits".isIntegral!T)
{
    import std.stdio : writeln;
    writeln(a, value);
}

void main() {}
$ time dmd test2.d
dmd test2.d  0.07s user 0.02s system 83% cpu 0.114 total

@JackStouffer

Copy link
Copy Markdown
Contributor

I tried and failed to convert std.file to this format to get some performance numbers. Got some really odd and unhelpful error messages from the compiler when I removed std.range.primitives and std.traits from the top level.

@andralex

andralex commented Feb 5, 2017

Copy link
Copy Markdown
Member Author

@JackStouffer: yes, that was my experience as well, as I thoroughly documented in the initial discussion around DIP1005. It didn't produce much of an impression - it seems one must try it in order to understand the difficulties :).

@deadalnix

deadalnix commented Feb 5, 2017

Copy link
Copy Markdown
Contributor

If I may suggest a name change, I would use Module instead of from. Module!"foo.bar" will effectively be the foo.bar module so let's name it that way.

@jmdavis

jmdavis commented Feb 6, 2017

Copy link
Copy Markdown
Member

How would this work with UFCS? For some stuff, that doesn't necessarily matter much, but for something like the range primitives, it could matter a lot. For instance, if you have a function that accepts a range, and the template constraint uses the range primitives, the range primitives need to have been imported for the constraint to work with dynamic arrays. If the imports are just associated with the function as a whole like DIP 1005 does, then that's not a problem, but if you're using UFCS, then you need to import the symbol separately from using it, and this solution would appear to require that the import be associated with a symbol - or at least that's how all of the examples are using it.

Also, what happens with this if you're using multiple symbols from the same module inside of the parameters and/or template constraints? Do you have to use from with each symbol or only the first one? With DIP 1005, it's only once, so if this requires separate imports for each, that's a lot more verbose.

@atilaneves

Copy link
Copy Markdown
Contributor

@deadalnix I don't know, I think I like from better.

@MartinNowak

MartinNowak commented Feb 13, 2017

Copy link
Copy Markdown
Member
  • FWIW this achieves the same as lazy static import + FQN (Issue 13255 – static imports should be done lazily), except that from!"std.datetime" avoids the module-level declaration
  • seems like this doesn't (yet) drag in the imported ModuleInfo (doesn't add the import to ModuleInfo.importedModules), thus missing to run constructors of the imported module
  • from may not be the most prominent variable name, but it's definitely in use. Shouldn't collide much with this though.

@dnadlinger

dnadlinger commented Feb 13, 2017

Copy link
Copy Markdown
Contributor

+1 for Module. It actually documents the function, rather than just providing a cute syntax for a single use case. Even one of the examples in the documentation itself looks wrong:

with (from!"std.datetime")

Just observe how much better this reads:

with (Module!"std.datetime")

While cute-looking solutions have a habit of stroking one's ego, perpetuating the idea of programming as magic is precisely not what well-designed language artefacts are supposed to do.

@andralex

Copy link
Copy Markdown
Member Author

One matter is that the invocation solves to a module alias, not a type name, so by convention it needs to start with a lowecase letter, not uppercase. (I fail to grok the argument with cuteness, egos being stroked, and magic.)

@andralex

Copy link
Copy Markdown
Member Author

@klickverbot with will be rarely used because it cannot occur at top level and sheer import inside scopes is simpler and just as good if not better. So the prevalent use case would be in function signatures. So we'd be mainly looking at:

void popFrontN(R)(R range, size_t n) if (from!"std.range.primitives").isInputRange!R);

or

void popFrontN(R)(R range, size_t n) if (Module!"std.range.primitives").isInputRange!R);

etc. Both seem plausible (the naming convention is worth considering though).

@dnadlinger

dnadlinger commented Feb 14, 2017

Copy link
Copy Markdown
Contributor

One matter is that the invocation solves to a module alias, not a type name, so by convention it needs to start with a lowecase letter, not uppercase.

Depends on your convention. ;) I would assert that modules, just like ambiguous generic templates should be referred to as in uppercase. I suppose it depends on where you put the default – value-like or type-like.

Both seem plausible

Both do seem plausible. I'm just pointing out that the artefact admits more uses than just the <name>!"foo.bar".member syntax. For example, one might imagine use in template meta-programming code where string mixins are currently used to cobble together import statements. And if somebody does come up with another clever use in the future, a more descriptive name like Module would certainly be a better fit – even though from might be a tad shorter for the current use case and reads out in a suggestive way (what I was referring to as "cute" earlier).

@andralex

Copy link
Copy Markdown
Member Author

Depends on your convention.

Phobos uses this_convention for module names, see e.g. https://dlang.org/dstyle.html as well as phobos itself. It stands to reason that a construct yielding a module alias follows the same vein, just the same as a construct yielding a type uses the convention for types etc.

@WalterBright

Copy link
Copy Markdown
Member

The advantage to Module is it suggests that the string should be a module name, whereas from is considerably more generic and is not suggestive of what the string should be.

@jmdavis

jmdavis commented Feb 14, 2017

Copy link
Copy Markdown
Member

seems like this doesn't (yet) drag in the imported ModuleInfo (doesn't add the import to ModuleInfo.importedModules), thus missing to run constructors of the imported module

As long as this is the case, it seems like it would be a very bad idea to promote an idiom like this.

@WalterBright

Copy link
Copy Markdown
Member

seems like this doesn't (yet) drag in the imported ModuleInfo (doesn't add the import to ModuleInfo.importedModules), thus missing to run constructors of the imported module

Hmm, interesting. This also applies to local imports where the local import is inside a template, and the template is not instantiated by the module that defines the template.

However, the import should be added to the list of the module that instantiates it. Unfortunately, it does not. https://issues.dlang.org/show_bug.cgi?id=17181

@MartinNowak

Copy link
Copy Markdown
Member

Has anyone considered leaving away imports for fully qualified names?
We could likely teach the compiler to lazily static import std.range.primitives when it can't resolve std.range.primitives.InputRange.

@andralex

Copy link
Copy Markdown
Member Author

@MartinNowak Problem is it's unclear whether a.b.c.d refers to a fully qualified name or a lookup starting from the current scope. One possibility would be to enhance leading . to cause an attempt at an import if .a.b.c.d cannot be resolved. However, this leads to the second problem: what module should be imported? a, a.b, or a.b.c?

@dnadlinger

dnadlinger commented Feb 15, 2017

Copy link
Copy Markdown
Contributor

Would "non-locality" (e.g. having a preceding (static) import instead of using from/Module) be an issue even if it wouldn't come at any performance difference (which it shouldn't)? I certainly like that the fact that we can do this in a library artefact, but to be honest I'm still not sure whether there is an issue to solve in the first place.

@andralex: If we want to go for an auto-import feature, it seems like local scope disambiguation itself wouldn't be the biggest issue. We could simply anchor the automatic feature to the root of the lookup tree (i.e. "one above module-level imports"), with the same shadowing properties as today. The second problem – disambiguating between [a module].[b.c symbol] and [a.b module].[c symbol – seems to be the harder one to solve in a universal fashion.

(Edit for readability.)

@timotheecour

timotheecour commented Feb 15, 2017 via email

Copy link
Copy Markdown
Contributor

@MartinNowak

Copy link
Copy Markdown
Member

– disambiguating between [a module].[b.c symbol] and [a.b module].[c symbol – seems to be the harder one to solve in a universal fashion.

Maybe just try from longest to shortest module, taking the first match?
Collisions should be rare (also b/c covention for module and type names are different) and are an issue in the package.
This doesn't easily integrate with the rest of the lookup, e.g. a.b.T.create.ufcs, so indeed some separator, like a.b::T would be better to resolve ambiguity.

It seems the more important question is how important static imports in function/template parameters are. Modularity was named as a prime motivation.
Just wondering how much we'd be willing to do for this.
Making module level static imports lazily shouldn't be too complex and requires no language change either, same for selective imports.
https://issues.dlang.org/show_bug.cgi?id=13255

We could have a look at those after the current lookup changes are transitioned and the planned import refactorings are through (late H1, early H2).

@jmdavis

jmdavis commented Feb 22, 2017

Copy link
Copy Markdown
Member

Making module level static imports lazily shouldn't be too complex and requires no language change either, same for selective imports.

I would very much like to see lazy imports happen. I'm already getting sick of using local and selective imports all over the place instead of just importing modules. It does have the advantage of making it clear where stuff comes from and minimizing which parts of the module see the imports, but it's just plain tedious and adds maintenance overhead. As I understand it, having fully lazy imports would largely make local and selective imports unnecessary from a performance perspective, and as such, I would probably stop doing much with selective and local imports, because they're just so tedious. And going with from or DIP 1005 would just add to that. I'm not necessarily opposed to them existing, but I don't think that they're worth using - especially if we have lazy imports. IMHO, D function signatures already have way too much in the way of attributes and whatnot marking them up to add imports into the mix, much as it does have the nice property of making it clear which imports that function uses.

@joakim-noah

joakim-noah commented Mar 15, 2017

Copy link
Copy Markdown
Contributor

@jmdavis, I agree that it's too tedious to use local, selective imports everywhere, but I think the endgame is a process where you just stick a bunch of non-selective, module-scope imports up top while you're writing your code, ie the way almost all D code is written now, and then when done, run a tool over it to properly scope all the imports and make them selective. Doing it by hand is never going to scale, we need a tool.

Building such an import tool, by integrating the ddmd frontend with libdparse, is now third in my D queue, with the top two about to be cleared away. Lazy imports may take away the performance need for such a tool, but better documentation is well worth doing it anyway.

@jmdavis

jmdavis commented Mar 16, 2017

Copy link
Copy Markdown
Member

@jmdavis, I agree that it's too tedious to use local, selective imports everywhere, but I think the endgame is a process where you just stick a bunch of non-selective, module-scope imports up top while you're writing your code, ie the way almost all D code is written now, and then when done, run a tool over it to properly scope all the imports and make them selective. Doing it by hand is never going to scale, we need a tool.

IMHO, if we expect a tool to be used in order for your code to follow best practices in D, then we've screwed up. I have no problem with someone writing a tool that they think will make their life easier, but as I understand it, choices were made with D's features in order to avoid the need for tooling beyond the compiler - particularly with regards to boilerplate code. And I, for one, don't want to have to be running extra tools on my code. It should be reasonable to just write it and have it follow best practices.

And if having fully lazy imports largely gets rid of the need for scoped and selective imports, I'm very much in favor of having fully lazy imports as the solution. scoped and selective imports result in far too much boilerplate code and IMHO are a maintenance problem. They do provide some value in terms of encapsulating dependencies, but I just don't think that they're worth that.

@joakim-noah

joakim-noah commented Mar 17, 2017

Copy link
Copy Markdown
Contributor

I disagree. There is a certain class of source transformations that should be automated: for example, I can just write my code formatted fairly sloppily and then run dfmt over it when I'm done, to tidy up. Insisting that everything be done by hand is a relic of a bygone era. I agree that scoped imports are more verbose, but in this case, that's the appeal.

@jmdavis

jmdavis commented Mar 17, 2017

Copy link
Copy Markdown
Member

I disagree. There is a certain class of source transformations that should be automated: for example, I can just write my code formatted fairly sloppily and then run dfmt over it when I'm done, to tidy up. Insisting that everything be done by hand is a relic of a bygone era.

I have no problem with someone running a tool to do source transformations if they want to. I have a problem with us deciding that something is best practice which basically requires that we run a tool. You should be able to reasonably write good, clean code that follows best practices without needing to run additional tools if you don't want to. And scoped and selective imports border on needing a tool because of how tedious they are to write and maintain.

I agree that scoped imports are more verbose, but in this case, that's the appeal.

I don't understand this. The verbosity and the fact that they require a fair bit of upkeep in comparison to simply importing the module you need at the top is precisely the problem. You end up with a lot of extra lines in your code and a decent amount of extra effort - particularly when selective imports are added into the mix. If anything, selective imports make it exponentially worse. And if we had fully lazy imports, then the performance benefits of using them would be negated, making it far more reasonable to just import modules at the top and be done with it.

@joakim-noah

joakim-noah commented Mar 17, 2017

Copy link
Copy Markdown
Contributor

You should be able to reasonably write good, clean code that follows best practices without needing to run additional tools if you don't want to.

You can, nobody is stopping you from perfectly formatting your code and scoping all your imports from the first line you write. However, you can also just write your D code formatted sloppily and with top-level imports and then have the dfmt and dscope tools do that tedious work for you.

The key here is that nobody is making D coders perfectly format their code (like python's indentation rules) and scope all their imports. It is up to you if you want the benefits those provide and we'll be able to say, "Here are some tools to make your life easier."

You end up with a lot of extra lines in your code and a decent amount of extra effort

The extra lines have a purpose, they document where exactly the symbols are coming from, which lazy imports do not. I believe that is worth it, perhaps you do not.

As for extra effort, there's none required other than running the tool, but you seem to be against that. I agree that scoped, selective imports will not be used much if we require everybody to insert them by hand, but I'm not against tools to do that for us, if we choose to scope our imports.

@jmdavis

jmdavis commented Mar 17, 2017

Copy link
Copy Markdown
Member

I agree that scoped, selective imports will not be used much if we require everybody to insert them by hand, but I'm not against tools to do that for us, if we choose to scope our imports.

I'm not against the tool existing either. The problem is that increasingly, it seems to be that it's considered best practice to use scoped and selective imports, which is a pain to do by hand. So, if you want to follow "best practice," then it starts looking like a tool is needed in order for that to be manageable, which runs contrary to the idea that you should not need a tool for boilerplate in D (much as anyone is free to use a tool if they want one).

One of the reasons that scoped and selective imports are pushed for the way that they are is because of the performance benefits, and if we had fully lazy imports like Walter has talked about doing, then that benefit of scoped and selective imports would be negated. Folks could still use them if they wanted, and they could even use a tool to manage them if they wanted to, but I think that there would be a much weaker argument that it was best practice to use them. And after trying to use them for a while and getting sick of all of the extra, tedious work required to maintain them, I would very much like to see us not need them and not be in a position where it's tooted as best practice and that you should change your code to use them if you're not. It seems to me that fully lazy imports solve the primary problem while requiring a lot less work on the part of D programmers in general. IMHO, it would be a huge win.

@andralex

andralex commented Jul 26, 2021

Copy link
Copy Markdown
Member Author

@atilaneves good stuff. Wondering about the ambiguity of things like from.a.b.c.d -> could be either "get d from module a.b.c", or "get c.d from module a.b". As far as I understand from a quick read it's a first-match strategy. Is that the case?

@atilaneves

Copy link
Copy Markdown
Contributor

@atilaneves good stuff. Wondering about the ambiguity of things like from.a.b.c.d -> could be either "get d from module a.b.c", or "get c.d from module a.b". As far as I understand from a quick read it's a first-match strategy. Is that the case?

I'm not sure and hadn't considered that - it's a downside of not having the string I guess.

@RazvanN7

RazvanN7 commented Aug 3, 2021

Copy link
Copy Markdown
Contributor

@andralex Is this good to go? If so, please add a runnable unittest.

@ljmf00

ljmf00 commented Aug 4, 2021

Copy link
Copy Markdown
Member

This version seems to not need compile-time strings: taurus-d/taurus#14. There's also this: FR86/localimport.

Taurus implementation supports betterC, from.std.algorithm but also From!"std.algorithm" in case a string is really needed and also works well with modules ambiguities.

@iK4tsu

iK4tsu commented Aug 4, 2021

Copy link
Copy Markdown

@atilaneves good stuff. Wondering about the ambiguity of things like from.a.b.c.d -> could be either "get d from module a.b.c", or "get c.d from module a.b". As far as I understand from a quick read it's a first-match strategy. Is that the case?

It is a first-match strategy. It always imports d from a.b.c. However, this is only possible if the parent module has a package.d, as I needed to check if a symbol was part of a module or a module itself. Because I need to do this step by step (because of the way opDispatch works), some modules can not be imported with from, such as core, as they do not have a package.d importing sub-modules.

To fix this issue, I implemented From, which receives a string that sets up the starting point, bypassing the checks of previous modules, similar to this proposal.

From!"std.algorithm".split(...);

This implementation structure also allowed for some goodness with error messages such as:

From!"std.math.abs"(-1); // Error: static assert: "Call must be bound to a symbol! Do you mean 'From!"std.math".abs'?"

edit:
I also restricted imports to the symbol only, to prevent code like the following:

From!"std.algorithm".split(...);
auto range = [1, 2, 3].map!"a + 1"; // where does map come from?

@andralex

andralex commented Aug 4, 2021

Copy link
Copy Markdown
Member Author

@andralex Is this good to go? If so, please add a runnable unittest.

I was thinking we need more time to decide which approach to choose.

@andralex

andralex commented Aug 5, 2021

Copy link
Copy Markdown
Member Author

However, this is only possible if the parent module has a package.d

That's unfortunate. Was hoping for a scheme that minimizes the files opened and imported - the moment we get to package.d, that's liable to fetch a bunch of stuff.

@ljmf00

ljmf00 commented Aug 5, 2021

Copy link
Copy Markdown
Member

However, this is only possible if the parent module has a package.d

That's unfortunate. Was hoping for a scheme that minimizes the files opened and imported - the moment we get to package.d, that's liable to fetch a bunch of stuff.

Yes, that may impact performance. __traits(compiles, { import std.stdio; }) still opens a file descriptor. This can be optimized by the compiler because it only needs to search for module existence not actually opening it. Maybe a new trait is needed to achieve better performance.

This logic is needed for ambiguities like package.d containing a symbol with the same name of a module.

@dlang-bot

Copy link
Copy Markdown

Thanks for your pull request, @andralex!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + druntime#1756"

@andralex andralex changed the title Add import_ for inline imports Add imported for inline imports Aug 27, 2021
@dlang-bot

Copy link
Copy Markdown

Thanks for your pull request, @andralex!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + druntime#1756"

@ljmf00

ljmf00 commented Aug 27, 2021

Copy link
Copy Markdown
Member

Doesn't this get further discussion? 👀

I was thinking we need more time to decide which approach to choose.

@pbackus

pbackus commented Aug 27, 2021

Copy link
Copy Markdown
Contributor

@ljmf00 Given that the opDispatch version still needs the string-based version as a fallback in case of ambiguity, I do not think any opportunity is lost by merging the string-based version now and deferring the decision on the opDispatch version.

@ljmf00

ljmf00 commented Aug 27, 2021

Copy link
Copy Markdown
Member

@ljmf00 Given that the opDispatch version still needs the string-based version as a fallback in case of ambiguity, I do not think any opportunity is lost by merging the string-based version now and deferring the decision on the opDispatch version.

Fair point 👍

@RazvanN7

Copy link
Copy Markdown
Contributor

@andralex Please rebase this to fix the tester issues.

@dlang-bot

Copy link
Copy Markdown

Thanks for your pull request, @andralex!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + druntime#1756"

@nordlow

nordlow commented Sep 26, 2021

Copy link
Copy Markdown
Contributor

Are we supposed to make use of this template in as many places (in druntime phobos) as possible now? If so, I wonder if that is gonna lead to a non-insignificant overhead in space when doing, for instance, make -f posix.mak unittest in phobos. FYI, @kinke.

@andralex

Copy link
Copy Markdown
Member Author

I'd say cautiously at first, with an eye on impact on build times and quality of generated documentation.

@kinke

kinke commented Sep 26, 2021

Copy link
Copy Markdown
Contributor

[I'm mostly worried about aesthetics - from that POV, I'd prefer not seeing this used at all. ;)]

@nordlow

nordlow commented Sep 26, 2021

Copy link
Copy Markdown
Contributor

Why (again) where the following non-templated-bloated alternatives turned down:

foo(T)(T x)
if (isIntegral!T)
with(import std.traits)

,

foo(T)(T x)
if ((import std.traits).isIntegral!T)

, and

foo(T)(T x)
if ((import std.traits : isIntegral)!T)

turned down?

To site @andralex, all of these would "simply remove a limitation in the language".

@pbackus

pbackus commented Sep 26, 2021

Copy link
Copy Markdown
Contributor

Trying a library solution now does not mean we cannot add a new language feature later, if the library solution proves inadequate.

@nordlow

nordlow commented Sep 26, 2021

Copy link
Copy Markdown
Contributor

Trying a library solution now does not mean we cannot add a new language feature later, if the library solution proves inadequate.

Indeed. Moreover, a semi-automatic search--and-replace of imported!... would be a breeze.

Btw, when you say inadequate what comes to your mind, @pbackus?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.