Open Jackie365 opened 1 year ago
there probably should be a check to see if end == string->value
(meaning nothing has been consumed) before skipping the whitespaces.
I don't know why the string is strip
ed here, but it doesn't seems the sensible way to do. Instead, a strip
method should be provided instead and let the user decide to strip
or not the string before parsing to number.
[Trim](https://wren.io/modules/core/string.html#trim()) is user space already fwiw
Considering how often Num.fromString
is used, it's surprising this bug hasn't been noticed before. Perhaps people had thought that, since strtod
returns 0
for invalid numeric strings, this was the expected behavior.
Anyway, on the face of it, it seems (as @minirop said) that all we need to do is to check whether any part of the string has been consumed by strtod
and return NULL
if it hasn't.
If we do that, there should be no need to treat an empty string as a special case or to skip past any trailing whitespace.
So that would give us:
DEF_PRIMITIVE(num_fromString)
{
if (!validateString(vm, args[1], "Argument")) return false;
ObjString* string = AS_STRING(args[1]);
errno = 0;
char* end;
double number = strtod(string->value, &end);
if (errno == ERANGE) RETURN_ERROR("Number literal is too large.");
// Check we have consumed any part of the string. Otherwise, it contains non-number
// characters and we can't parse it.
if (end == string->value) RETURN_NULL;
RETURN_NUM(number);
}
Some time ago, I tried removing our dependency on the standard strtod
and strtoll
functions with #984.
It certainly is a very extreme solution to this specific problem but thought it could be more intuitive this way and get rid of a few weird quirks of strtoXX
.
With wrenParseNum
we have easier access to different error messages than strtoXX
, for example: if there were no digits when parsing. fromString
currently discards this information but I find it could still be helpful for the compiler or if someone else wanted to use wrenParseNum
in their host C code.
I think the biggest issue of #984 is that the results of wrenParseNum
may be less accurate to strtoXX
as the double's exponents gets too high or too low.
Yes, I remember that PR.
If we introduce octal (0oxxx
) and binary (0bxxx
) literals, which I think we should, then strtod
will always return 0.0
when passed those literals in string form even though it does currently deal properly with hexadecimal strings. For consistency, we'd therefore need something like your solution to deal with all three.
Hi, I'm using wren v0.4.0 and I find
Should it be
null
?