December 20, 2016 Re: DIP10005: Dependency-Carrying Declarations is now available for community feedback | ||||
|---|---|---|---|---|
| ||||
 Posted in reply to Andrei Alexandrescu | On Tuesday, 20 December 2016 at 04:04:14 UTC, Andrei Alexandrescu wrote: > On 12/19/2016 01:47 AM, Joakim wrote: >> Why do you care _so_ much about the module dependency graph? To make >> this question concrete, let's look at an example, the std.array module >> you keep mentioning. >> This is what it looked like before Ilya scoped as >> many imports as he could: >> >> https://github.com/dlang/phobos/commit/3fcf723aa498b96de165361b5abb9d3450fdc069#diff-54cf8402b22024ae667d4048a5126f0e >> >> That was a mess, similar to opaque C/C++ code, 13 modules imported at >> module-scope were whittled down to 4. You just made those more specific >> in a commit related to this DIP, by listing the actual symbols >> selectively imported from those four modules: >> >> https://github.com/dlang/phobos/commit/e064d5664f92c4b2f0866c08f6d0290ba66825ed#diff-54cf8402b22024ae667d4048a5126f0e >> >> >> If I'm looking at the template constraints for any particular function >> and see a couple symbols I don't recognize, I don't think it's a big >> deal to find the symbols in that list at the top. > > It actually is. Some symbols in e.g. std.range are used in constraints, some others used in definitions; some in a UFCS manner. Some code runs during compilation, with errors gagged (as in __traits(compiles)) or not gagged. I don't know of an easy manual method to figure out who's who (which makes Vladimir's idea of tooling it with brute force awesome and scary at the same time). I'm not sure why you care who's who. I noted that if I'm looking at the constraints for a function, it's easy to check the selective imports at the top of the file for any symbols. That's the common scenario. I don't know why anyone would be checking the selective imports at the top to see where each symbol is actually used, which is the uncommon scenario you now present. > At any way, I don't see how I can use this tidbit to improve the DIP. Anything I could add to it to make it more compelling? You could answer my question above. We have already scoped 90% of the dependencies, why is it so important to remove the remaining 10% that we need to add new syntax? >> In other words, D already allows you to scope most imports. I don't >> consider the dozen or two remaining symbols from templaint constraints >> and function arguments to provide much overhead. Rather, I consider the >> weight of this additional syntax, ie the cognitive overhead from having >> to remember and parse more syntax in my head, to be worse than the >> remaining dependency reasoning problem you're trying to solve: the cost >> outweights the benefit. Perhaps that's subjective and others may disagree. > > It is subjective and difficult to convert into action. Online review among folks who know next to nothing about each other does have its challenges. What happens here is I post a proposal for a problem that I believe is important for large D projects. And I get back "eh, that's not as important to me." At a traditional work place we'd know a lot about one another's strengths, specialization areas, and design sensibilities. Here, all I can do is look at your past PRs (that's why I emailed you asking for your github handle; I figure it's @joakim-noah). This makes it difficult to act on "I don't buy it" feedback. I didn't just say "eh:" I gave evidence for why I think the problem is minimal and asked why it's so important to scope those last 3-4 imported modules too, which you didn't answer. As for looking at my PRs, there were some links above, which show that is my handle, but I don't think you'll get much from that, as I haven't designed anything in github.com/dlang, only cleaning up and porting. >> Now, there's also the question of purely technical benefits, like >> compilation speed or executable bloat. I looked at the latter a little >> last summer, after Ilya had cleaned up a lot of the standard library: >> >> http://forum.dlang.org/thread/gmjqfjoemwtvgqrtdsdr@forum.dlang.org >> >> I found that commenting out a single scoped, selective import of >> "std.string: format" in std.utf led to a 5% decrease in executable size >> for "hello world." This is a problem with how dmd compiles or appends >> these module dependencies and would presumably still be there after this >> DIP, as you would not remove the dependency. > > That might be a related but distinct issue. Can you reproduce that experiment? I went back and looked at that particular import and it appears Walter subsequently removed it: https://github.com/dlang/phobos/pull/3455 I will experiment a bit more with some sample code and see what I find. >> I think scoped, selective imports have been great at hacking away at the >> module dependency graph, as you lay out. It is not clear what technical >> costs you see from the remaining few dependencies and if this DIP is the >> best way to remove them. I think you should explain why you want to >> untangle the remaining dependency graph, and consider if this DIP is >> really doing that much. > > I've made a few more passes through the DIP, but throughout I assume a couple of things without stressing them too much - dependencies are important, and associating dependencies with declarations makes the dependency graph as fine and as precise it could get. I don't see a reasonable way to make it better or clearer. > > Should I add an introductory section on why dependencies are important? If it deals with technical costs of dependencies that you're aiming to alleviate with this DIP, yes. I don't see why the module constructor reason you just gave can't be automated. We appear to disagree on the value of localizing the remaining selective imports needed for constraints/declarations, from the standpoint of reasoning and refactoring. Let me note that I'm not the only one in this thread saying that this isn't a problem worth adding syntax for and wondering why this DIP is worth doing. If we're going to do it, I think Dominik's suggestion is the best: just change the semantics of scoped imports to apply to the constraints and function arguments too. I don't think it's as surprising as you think, as the constraints are part of the function/struct. If somebody buries the import deep in the function and far away from the constraint, that's just bad style on their part, and is already possible (and annoying) now. We can automate the imports for declarations or leave that case alone. Finally, while all this dependency-localizing support is good, we really need tools to help us do it. I just happened to be thinking again about implementing such a tool a couple days before you posted this DIP. It'll have to wait till I push out the latest Android beta in the next week or two. | |||
Permalink
ReplyDecember 20, 2016 Re: DIP10005: Dependency-Carrying Declarations is now available for community feedback | ||||
|---|---|---|---|---|
| ||||
 Posted in reply to Andrei Alexandrescu | On 12/13/16 11:33 PM, Andrei Alexandrescu wrote:
> Destroy.
>
> https://github.com/dlang/DIPs/pull/51/files
>
>
> Andrei
Just a thought but with all of proliferation of imports down to each declaration comes the pain that e.g. renaming a module cascades to countless instances of import statements. This is true of local imports as well but the problem gets bigger.
----
Dmitry Olshansky
| |||
December 20, 2016 Re: DIP10005: Dependency-Carrying Declarations is now available for community feedback | ||||
|---|---|---|---|---|
| ||||
 Posted in reply to Dmitry Olshansky | On Tuesday, 20 December 2016 at 14:31:38 UTC, Dmitry Olshansky wrote:
> On 12/13/16 11:33 PM, Andrei Alexandrescu wrote:
>> Destroy.
>>
>> https://github.com/dlang/DIPs/pull/51/files
>>
>>
>> Andrei
>
> Just a thought but with all of proliferation of imports down to each declaration comes the pain that e.g. renaming a module cascades to countless instances of import statements. This is true of local imports as well but the problem gets bigger.
>
> ----
> Dmitry Olshansky
Could you not have the old module just be empty and publicly import the new one, and also deprecate it so people have time to change their code? Also having special syntax makes finding every occurrence of an inline import a fairly simple search.
| |||
December 20, 2016 Re: DIP10005: Dependency-Carrying Declarations is now available for community feedback | ||||
|---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On Tuesday, 20 December 2016 at 01:06:01 UTC, Andrei Alexandrescu wrote:
> Pushed again, now with the syntax using "with" proposed by Hatem Oraby and others.
>
> https://github.com/dlang/DIPs/pull/51
>
> https://github.com/dlang/DIPs/blob/71bde077488b566fba7603de6095b45984d9294a/DIPs/DIP1005.md
>
>
> Andrei
"In addition, we propose the statement and declaration with (import ImportList). ImportList is any syntactical construct currently accepted by the import declaration. The with (import ImportList) obeys the following rules:
- Inside any function, with (Import ImportList) is a statement that introduces a scope. Inside the with, lookup considers the import local to the declaration (similar to the current handling of nested imports).
- Everywhere else, with (Import ImportList) is always a declaration and does not introduce a new scope. Lookup of symbols is the same as for the statement case."
I must've somehow missed this when reading through for the first time after your changes as I thought this was a glaring omission. Glad to see that this was covered as this rightly makes it turtles all the way down. Actually, you could completely replace import statements with this new construct, having the `import std.range` at the top level replaced with a module-wide attribute `with (import std.range):`. In that light my only real criticism is that it's kind of redundant having both this new form *and* local imports, and it's not clear which is better and why to a new user.
| |||
December 20, 2016 Re: DIP10005: Dependency-Carrying Declarations is now available for community feedback | ||||
|---|---|---|---|---|
| ||||
 Posted in reply to Dmitry Olshansky | On 12/20/2016 09:31 AM, Dmitry Olshansky wrote:
> On 12/13/16 11:33 PM, Andrei Alexandrescu wrote:
>> Destroy.
>>
>> https://github.com/dlang/DIPs/pull/51/files
>>
>>
>> Andrei
>
> Just a thought but with all of proliferation of imports down to each
> declaration comes the pain that e.g. renaming a module cascades to
> countless instances of import statements. This is true of local imports
> as well but the problem gets bigger.
Good point, I'll integrate it in the document. -- Andrei
| |||
December 20, 2016 Re: DIP10005: Dependency-Carrying Declarations is now available for community feedback | ||||
|---|---|---|---|---|
| ||||
Posted in reply to Joakim | I've asked Joakim about this via email just now, likely other folks also know the answer: 1. I found these PRs related to local imports: https://github.com/dlang/phobos/pull/4361 https://github.com/dlang/phobos/pull/4365 https://github.com/dlang/phobos/pull/4370 https://github.com/dlang/phobos/pull/4373 https://github.com/dlang/phobos/pull/4379 https://github.com/dlang/phobos/pull/4392 https://github.com/dlang/phobos/pull/4467 Are there more? Is there a central point for them (such as a bugzilla issue)? 2. I see you've done a bunch of work in the area. Where, in your estimate, are we on the spectrum of making imports local without a major reorganization? Any low-hanging fruit to look at, or have Joakim, Ilya, Jack and others made a pass through most/all modules already? Thanks! | |||
December 20, 2016 Re: DIP10005: Dependency-Carrying Declarations is now available for community feedback | ||||
|---|---|---|---|---|
| ||||
 Posted in reply to deadalnix | On 19.12.2016 06:31, deadalnix wrote:
> On Sunday, 18 December 2016 at 23:18:27 UTC, Andrei Alexandrescu wrote:
>> Great, thanks. Please take a look at the accuracy of the discussion. I
>> expanded the "Workaround" section and moved it near the top.
>>
>> https://github.com/dlang/DIPs/pull/51
>>
>> https://github.com/dlang/DIPs/blob/dd46252e820dce66df746540d7ab94e0b00a6505/DIPs/DIP1005.md
>>
>>
>>
>> Andrei
>
> What's wrong with the parentheseless version ? The DIP says it looks
> "out of place" but that doesn't strike me as a very good argument. IMO
> the version without ";" is the way to go, as it doesn't require to add a
> new syntax for imports.
>
> Identical things looking identical is valuable.
>
+1.
| |||
December 20, 2016 Re: DIP10005: Dependency-Carrying Declarations is now available for community feedback | ||||
|---|---|---|---|---|
| ||||
Posted in reply to Joakim | On 12/20/2016 03:46 AM, Joakim wrote: > I didn't just say "eh:" I gave evidence for why I think the problem is > minimal and asked why it's so important to scope those last 3-4 imported > modules too, which you didn't answer. You have asked for a smoking gun, and one has been found. I have just uploaded a major update that carefully analyzes the improvements brought about by switching to local imports in the D Standard Library. Please refer to the section "Workaround: Are Local Imports Good Enough?" and Appendix A: https://github.com/dlang/DIPs/pull/51/files https://github.com/dlang/DIPs/blob/91baecedcfe7cb75ac22e66478722ec797ebb901/DIPs/DIP1005.md Andrei | |||
December 20, 2016 Re: DIP10005: Dependency-Carrying Declarations is now available for community feedback | ||||
|---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On 12/20/2016 05:08 PM, Andrei Alexandrescu wrote: > On 12/20/2016 03:46 AM, Joakim wrote: >> I didn't just say "eh:" I gave evidence for why I think the problem is >> minimal and asked why it's so important to scope those last 3-4 imported >> modules too, which you didn't answer. > > You have asked for a smoking gun, and one has been found. > > I have just uploaded a major update that carefully analyzes the > improvements brought about by switching to local imports in the D > Standard Library. Please refer to the section "Workaround: Are Local > Imports Good Enough?" and Appendix A: > > https://github.com/dlang/DIPs/pull/51/files > > https://github.com/dlang/DIPs/blob/91baecedcfe7cb75ac22e66478722ec797ebb901/DIPs/DIP1005.md I've also added a supporting script that creates the table in Appendix A: https://github.com/andralex/DIPs/blob/9e9ebc8738ce04b2d85a92feafacc7ef81e59811/DIPs/DIP1005-countlines.zsh -- Andrei | |||
Copyright © 1999-2021 by the D Language Foundation