miniscript icon indicating copy to clipboard operation
miniscript copied to clipboard

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

Open MineRobber9000 opened this issue 2 years ago • 2 comments

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 avatar Feb 05 '24 06:02 MineRobber9000

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.

MineRobber9000 avatar Feb 05 '24 07:02 MineRobber9000

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

JoeStrout avatar Feb 05 '24 12:02 JoeStrout