-
-
Notifications
You must be signed in to change notification settings - Fork 740
fix Issue 6531 - assertion failure in std.range.iota #1673
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
} | ||
else | ||
{ | ||
if (pastEnd <= nextDown(end)) ++count; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If end
is a whole number, then end is nextDown(end)
... correct? http://dlang.org/phobos/std_math.html#.nextDown documents everything but the normal behavior :/
In any case, doesn't that mean the if
conditions aren't strictly equivalent in run-time and ctfe? (<
vs <=
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, never mind, I think I got it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. Now I don't even understand why this code is here at all. How can pastEnd < end
at all? Doesn't this require wrap around, which can only happen with integrals? This is confusing :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code handles the case where count is rounded incorrectly (count = to!size_t((end - beg) / step))
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah... I miss-understood what "nextDown" does. Makes sense now.
Wouldn't:
real fcount = trunc((end - beg) / step);
count = to!size_t((fcount));
give an exact count, without further need for any rounding correction? It's real from end to end...?
- The assertion fails because of excess precision. - Can't use nextUp/nextDown in ctfe, but currently ctfe always computes with maximum precision so it doesn't run into this issue.
Pinging @MartinNowak : Did you read my last message? I think your pull is correct. It is just producing "unexpected" (but not incorrect) results. Fix it up? |
assert(iota(0.0, 3.0, 0.03).length == 100); | ||
assert(iota(0.0, 3.0, 0.03).back < 3.0); | ||
assert(iota(-936596.062f, 373124.094f, 327430.031f).length == 5); | ||
assert(iota(0.0, 3.6, 0.03).length == 120, text(iota(0.0, 3.6, 0.03).length)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test failure: This is just missing an import to std.conv
.
I'll close this until I have time to verify the solution. Or maybe somebody else fixes the issue. |
always computes with maximum precision so it doesn't
run into this issue.
https://d.puremagic.com/issues/show_bug.cgi?id=6531