Skip to content

Conversation

Poita
Copy link
Contributor

@Poita Poita commented Oct 27, 2012

std.range.Zip contains a Tuple, whose toString() function calls formatElement which eventually calls this function when the range element type isSomeChar, leading to a static call to walkLength on infinite ranges. Since pull request 880, walkLength doesn't work with infinite ranges, so it fails to compile.

This change ensures that walkLength is called with only valid range types.

Bug: http://d.puremagic.com/issues/show_bug.cgi?id=8900

`std.range.Zip` contains a `Tuple`, whose `toString()` function calls `formatElement` which eventually calls this function, leading to a static call to `walkLength` on infinite ranges. Since pul request 880, `walkLength` doesn't work with infinite ranges, so it fails to compile.

This change ensures that `walkLength` is called with only valid range types.

Bug: http://d.puremagic.com/issues/show_bug.cgi?id=8900
@dsimcha
Copy link
Collaborator

dsimcha commented Oct 27, 2012

Looks good. Just waiting for auto-tester to pass.

@monarchdodra
Copy link
Collaborator

Oops (kind of). I removed support for infinite ranges in walklength. I did not see this impact.

Well, they do say a compile error is better than a link error, which is definitly better than a run-time error.

I hate debugging infinite loops with a passion.


Fix looks good to me, though the first check for hasLength is kind of redundant, since walkLength is specifically optimized for ranges with length anyways (so the coder doesn't have to bother checking himself). I guess you save on a call to save though.

Well now that it is written, we might as well keep it. LGTM.

@andralex
Copy link
Member

What's the matter with FreeBSD_32?

@andralex
Copy link
Member

LGTM otherwise

andralex added a commit that referenced this pull request Dec 13, 2012
Fix bug 8900 - Zip with infinite char range fails.
@andralex andralex merged commit 259cd23 into dlang:master Dec 13, 2012
@andralex
Copy link
Member

merged, thanks

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.

4 participants