Small clean-up in std.conv.parse#5067
Conversation
JackStouffer
force-pushed
the
patch-21
branch
from
95c0e4c to
bd289fd
Compare
std/conv.d
Outdated
| import core.checkedint : mulu, addu; | ||
| import std.exception : enforce; | ||
|
|
||
| enforce!ConvException(!s.empty, "s must not be empty in integral parse"); |
There was a problem hiding this comment.
s.empty is being called twice, and the logic has been moved around parse!Target(s) - is that significant?
There was a problem hiding this comment.
I moved the logic because atStart was just a poor-man's range.empty. It makes much more sense to do sanity checks on input at the start of the function.
There was a problem hiding this comment.
I meant the check is now before lines 2344 and 2345, whereas it used to be after.
There was a problem hiding this comment.
That function also throws on empty, see line 2007
There was a problem hiding this comment.
So when parse is called, the check is done twice. I suggest moving the check back to where it was before.
std/conv.d
Outdated
| Target v = 0; | ||
| bool atStart = true; | ||
|
|
||
| for (; !s.empty; s.popFront()) |
There was a problem hiding this comment.
changing this to do..while will eliminate the redundant s.empty
There was a problem hiding this comment.
Ok, will look into this
JackStouffer
force-pushed
the
patch-21
branch
from
bd289fd to
9faf3e1
Compare
|
@WalterBright Done |
JackStouffer
force-pushed
the
patch-21
branch
from
9faf3e1 to
c6fb78e
Compare
|
@WalterBright Fixed |
|
Auto-merge toggled on |
| auto nextv = v.mulu(radix, overflow).addu(c - '0', overflow); | ||
| if (overflow || nextv > Target.max) | ||
| goto Loverflow; | ||
| enforce!ConvOverflowException(!overflow && nextv < Target.max, "Overflow in integral conversion"); |
There was a problem hiding this comment.
I think this should have been <=?
|
|
4 participants
Removed
gotos and usedenforcewhich IMO is more in keeping with D style.