JoeStrout / miniscript

source code of both C# and C++ implementations of the MiniScript scripting language
MIT License
275 stars 64 forks source link

"val" intrinsic gives inconsistent results between C# and C++ MiniScript, and generally doesn't handle octal/binary #125

Open MineRobber9000 opened 7 months ago

MineRobber9000 commented 7 months ago

Exactly what it says on the tin.

C++ MiniScript can resolve "0x"-prefixed hexadecimal numbers, but not "0o" or "0b" (octal and binary respectively). Meanwhile, C# MiniScript can't even resolve "0x"-prefixed hexadecimal numbers (tested in Mini Micro). Ideally, MiniScript should be able to support hex, octal, and binary strings across all versions.

MineRobber9000 commented 7 months ago

Interestingly enough, I'm not sure C++ MiniScript's support of "0x"-prefixed hex was intentional. The C# code responsible for val is:

            // val
            //  Return the numeric value of a given string.  (If given a number,
            //  returns it as-is; if given a list or map, returns null.)
            //  May be called with function syntax or dot syntax.
            // self (string or number): string to get the value of
            // Returns: numeric value of the given string
            // Example: "1234.56".val       returns 1234.56
            // See also: str
            f = Intrinsic.Create("val");
            f.AddParam("self", 0);
            f.code = (context, partialResult) => {
                Value val = context.self;
                if (val is ValNumber) return new Intrinsic.Result(val);
                if (val is ValString) {
                    double value = 0;
                    double.TryParse(val.ToString(), NumberStyles.Any, CultureInfo.InvariantCulture, out value);
                    return new Intrinsic.Result(value);
                }
                return Intrinsic.Result.Null;
            };

This is why val doesn't support hex/octal/binary strings; C#'s double.TryParse doesn't attempt to parse them and just leaves value as 0.

The corresponding C++ code is:

    static IntrinsicResult intrinsic_val(Context *context, IntrinsicResult partialResult) {
        Value val = context->GetVar("self");
        if (val.type == ValueType::Number) return IntrinsicResult(val);
        if (val.type == ValueType::String) return IntrinsicResult(val.GetString().DoubleValue());
        return IntrinsicResult::Null;
    }

Which calls SimpleString.DoubleValue:

    double String::DoubleValue(const char* formatSpec) const {
        double retval = 0;
        sscanf(c_str(), formatSpec, &retval);
        return retval;
    }

sscanf, for some inexplicable reason, when told to find a double (%lf), will parse a hex string, but not octal or binary. (This implementation also has the side effect of yet another inconsistency: as long as a string starts with something parseable as a float, C++ MiniScript will report that as the value, even if the rest of the string is decidedly not a float; print "1foo2".val will print 1 in C++ and 0 in C#, but that's another issue for another time.)

That being said, I do feel like being able to handle these prefixed numbers is a thing MiniScript should do; either that, or go with the Lua/Python approach where you supply a radix to convert a string in that radix to a number.

JoeStrout commented 7 months ago

You are correct; there is no intended support for "0x" or other prefixes. "val" is meant to interpret numbers in decimal format only.