Skip to content

std.range and std.functional: use locally-scoped selective imports and document module-scope symbol imports#4379

Merged
andralex merged 2 commits intodlang:masterfrom
joakim-noah:range_imports
Jun 18, 2016
Merged

std.range and std.functional: use locally-scoped selective imports and document module-scope symbol imports#4379
andralex merged 2 commits intodlang:masterfrom
joakim-noah:range_imports

Conversation

@joakim-noah
Copy link
Contributor

@joakim-noah joakim-noah commented May 29, 2016

Checked with ddmd, similar to #4365.

Update: I was getting a lot of shared ctor cycles in the auto-tester from updating std.range, which appeared to be from importing std.algorithm.comparison directly instead of std.algorithm. I tried changing all those instances to std.algorithm and that fixed it, but seemed like a hack. If I simply didn't touch the pre-existing uses of std.algorithm for symbols from std.algorithm.comparison and didn't change the few pre-existing direct imports of std.algorithm.comparison, the cycle still came up, ie simply from adding or modifying other imports. That seems like a bug.

Rather than going back to importing the package std.algorithm everywhere, I went down the list of imports in the cycle, conveniently listed by the druntime error, and found it was enabled by a stray, unused local import of std.stdio in std.functional, so I removed that import and went ahead and used all selective imports for std.functional too.

@joakim-noah joakim-noah changed the title std.range: use locally-scoped selective imports and document module-scope symbol imports std.range and std.functional: use locally-scoped selective imports and document module-scope symbol imports May 30, 2016

import std.meta, std.traits;
import std.meta; // AliasSeq, Reverse
import std.traits; // isCallable, Parameters
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AliasSeq is only called from one unittest block and Parameters from four templates, but if I make these two symbols into local selective imports and have only the two remaining symbols selectively imported at module scope, I started getting template instantiation errors. Not sure what's going on, so just moved those two up here.

…bols imported at module scope, checked with ddmd
@wilzbach
Copy link
Contributor

wilzbach commented Jun 7, 2016

This unused import was causing a shared ctor cycle after changing the imports in std.range.

Great catch!
Why wasn't this pulled yet? I think that by this passive inactivity @joakim-noah has lost (parts of) his motivation :/

@joakim-noah
Copy link
Contributor Author

No, nothing affecting my motivation, I just stopped here. I want to automate the rest, thought I'd use these import PRs I manually modified and submitted as training data for that. I know PRs sometimes take awhile to get in, doesn't affect me.

@andralex andralex merged commit 75ad1df into dlang:master Jun 18, 2016
@andralex
Copy link
Member

great work, thx!

@joakim-noah joakim-noah deleted the range_imports branch June 23, 2016 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants