Fixed std.algorithm imports in std.format #4467
Conversation
| @@ -668,7 +668,7 @@ struct FormatSpec(Char) | |||
| if (is(Unqual!Char == Char)) | |||
| { | |||
| import std.ascii : isDigit; | |||
| import std.algorithm : startsWith; | |||
| import std.algorithm.searching : startsWith; | |||
There was a problem hiding this comment.
Why not move this to top, ie alphabetize, as long as you're changing it?
|
It seems I created an import cycle. I'm not really sure how this was introduced given the nature of the import changes. |
|
@JackStouffer usually the cause of this is a weird trait of importing a module that has The only way I've been able to fix this is to hunt down unit tests that are inside templates and remove them from the templates. There's no easy way to do this. |
Great |
|
Sorry, I was wrong on what I said. It's not templated unit tests, but templated module constructors. See bug report here: https://issues.dlang.org/show_bug.cgi?id=14517 |
|
OK, so the These are legitimately marked (std.encoding + std.parallelism), as both of those have static ctors (and neither have templated ones). What I don't understand at this point is why it works if you include std.algorithm, but not if you include the specific submodule. The cycles should still exist. |
|
@schveiguy I removed the std.range import from std.parallelism here #4481 Perhaps that will remove the cycle. I can't test on my machine because the tests pass locally. |
Then something is really strange. The cycle detection should be deterministic. Indeed I see sometimes the tests pass! This could be a bug in the cycle detection code. |
|
Manually went through the cycle -- it exists without your changes. There's definitely something wrong with the cycle code. I'll have to diagnose it and get back to you. Likely we are in for some painful changes to fix this... |
|
I ran into this cycle bug on a similar PR before, #4379, just broke the cycle in |
|
Ran some more brute force cycle detection -- the cycle showing up in this PR is actually not because of your PR, it's already in Phobos. There must be a bug in the cycle detection. I'm going to fix the bug, and then we will have to eliminate the cycles, and THEN your PR should work :) So hold off for a while. I'll let you know when I figure it out. |
|
Ok, thanks! |
|
Yep, somebody screwed up the algorithm after I fixed it... https://issues.dlang.org/show_bug.cgi?id=16211 |
|
See #4493, should allow merging this (but I am going to fix the cycle detection code too, it can be done separately, as the phobos change is a prerequisite anyway). |
|
And the druntime fix: dlang/druntime#1602 |
|
Thanks |
Fixed std.algorithm imports in std.format
No description provided.