Removed many global imports from std.experimental.logger.core #5092
Conversation
| import std.datetime; | ||
| import std.string; | ||
| import std.range; | ||
| import std.range.primitives; | ||
| import std.traits; |
There was a problem hiding this comment.
There's really no benefit to top-level selective imports at the moment. DMD still scans the entire imported file; all selective imports do is change visibility rules. This is something that can and should change (SDC does the right thing), and when it does, I will gladly fix this.
Plus, selective top level imports isn't the current style in the rest of Phobos.
There was a problem hiding this comment.
The biggest benefit for me personally is that they document explicitly what is being imported. This makes them easier to convert to convert to local imports and to whatever DIP1005 brings to the table (language change or something along the lines of dlang/druntime#1756).
Plus, selective top level imports isn't the current style in the rest of Phobos.
Where we are at the moment is not interesting to talk about. What matters is where we want to be and how to get there.
There was a problem hiding this comment.
There is a benefit right now: the compiler has to search all the top-level modules and any publicly imported modules they reference to see where a symbol is invoked from and check for name collisions. When you specify the module a symbol comes from, that search is skipped.
The real issue is that we have held off on using module-scope selective imports until we're sure the symbol leak bugs are fixed and downstream code has adapted. Now that 2.073 has shipped, maybe @MartinNowak will OK their use?
|
This debate isn't dependent on the changes here and can be done in a separate PR; this can be merged as is. |
Hence, why I first approved your PR ;) |
It is not completely independent, the only reason he's not using selective imports in this PR is because of this long-standing convention. I agree that it doesn't matter if this PR uses them or not, so I'm not suggesting that this PR wait on that. |
|
Auto-merge toggled on |
No description provided.