Fix range violation for cartesianProduct of empty ranges. #2328
Conversation
| current[i] = r.save; | ||
| if (!current[i].empty) | ||
| empty = false; |
There was a problem hiding this comment.
I think it should start with empty = false and set empty to true upon encountering an empty range, otherwise as long as there's one non-empty range you'll get undefined behaviour in front and popFront.
There was a problem hiding this comment.
Agreed. Otherwise you will also have a non-empty Range.init which feels rather error-prone to me.
There was a problem hiding this comment.
You guys are totally right. As long as one of the arguments is empty, the entire product by definition is empty. I don't know what I was thinking when I wrote that code... must've been past my bed time and I wasn't thinking straight. :P Anyway, I redid the fix, should be better now.
|
EDIT: Is there an issue with non-empty Range.init that Dicebot mentioned? Otherwise lgtm. |
|
Well it is not something that @quickfur has introduced so not a blocker but I'd appreciate to see it fixed so that default-initialized range will be empty ;) |
|
That make you happy? ;-) |
|
Yay ^_^ |
|
Auto-merge toggled on |
Fix range violation for cartesianProduct of empty ranges.
|
This PR caused a regression: https://issues.dlang.org/show_bug.cgi?id=13393 :-( |
I think it just exposed the bug that |
|
Well, yes, I figured that out. :-P |
Completes the fix for: https://issues.dlang.org/show_bug.cgi?id=10693