Expression evaluation bug?

[erendrake]

I just had a report from a user who tried to set

set config:ipu to 1/10. 

It did not take. But kOS did take

set config:ipu to 10^-1.

which unsurprisingly, resulted in a non responsive terminal that could only be fixed in the config file. @marianoapp, mind dropping a pearl of wisdom on is about this?

I believe we should throw a hard error when the user attempts to set this config value to less than 1.

[Dunbaratu]

I agree, however it sounds like there's a second major error that's also been revealed by this.

And that error is this: Why is the result different when the expression is 1/10 versus when it's 10^-1? By everything about the design of kOS, those should end up giving identical behavior. Why aren't they?

With the statement:

set var to expression.

expression should be getting calculated in utter ignorance of what the var is that it's being stored in. It should, by all rights, be literally impossible for the call to Config.SetSuffix("IPU") to tell the difference between the expression 1/10 versus the expression 10^-1. They should both be becoming nothing more than the value ((double)0.1) left atop the stack, by the time Config.SetSuffix() gets called.

If what you're saying is true, it would seem to indicate that the value of the expression 1/10 and the expression 10^-1 are not coming out exactly the same in how they get stored on the stack.

[erendrake]

@Dunbaratu I agree and should have worded the bug differently. The difference in how kOS dealt with the two statements is the real bug, ill reword.

[marianoapp]

When the two arguments are ints the operation uses the CalculatorIntInt class so the result returned is also an int. The power operation is a particular case because even if the two arguments are ints there's no guarantee that the result will also be an int. In this example 10^-1 returns 0.1 which in turn is casted to int as zero. The solution would be to modify the PromoteToDouble function to also return a double if the argument has a decimal part different than zero.

[Dunbaratu]

Yes. The fact that integer division performs a truncation and therefore (1/10) is different from (1.0/10.0) makes sense in a strongly typed language where the programmer has a lot of tools at his disposal to be explicit about data types, but it doesn't make sense in a more weakly typed language like kerboscript. If we're trying to hide from the user the difference between floats and ints and just treat everything as the generic 'number' type, then we have to make sure that 1/10 returns 0.1 not 0, and make the user explicitly call floor() if they want the truncation behavior.

[Dunbaratu]

Does it make sense, as a good generic catch-all solution to all numeric type casting, to always say that ANY time the result of any math operation would be a value with a fractional component after the decimal point, then the result will be always returned as a double regardless of what the input types were, AND ALSO, any time the result of any math operation would result in a round integer value with nothing after the decimal point but zeroes, then the result will always be returned as an integer regardless of the input types?

i.e: (5/2), (5.0/2), (5/2.0), (5.0/2.0), and (0.5_5) would all return ((double)2.5). and (4/2), (4.0/2), (4/2.0), (4.0/2.0), and (0.5_4) would all return ((integer)2).

If we did this everywhere, then that would truly mean users could use numbers without thinking of their type. In places where you end up with a integer result, you will get an integer (important if users start doing things with the LIST type and trying to play with index math - values that are integers should be really integers so they can be used as array indeces).

[Dunbaratu]

@erendrake, @marianoapp. I'm in a position to work on this. I'd like a go or no-go approval or disapproval of my proposed solution in the comment above this one first.

Pros: Easy for the user - no worries about types - can treat floats and ints as just being "numbers".

Cons: Slight extra execution cost for arithmetic: That cost is this:

• 1: For (expr1)/(expr2) : If operands are both integer, perform (expr1)%(expr2) first and check for remainder. If there is one, promote expr1 and expr2 to double, then divide them as doubles.
• 2: For (expr1)^(expr2) : ONLY perform as integer exponentiation if both expr1 and expr2 are integers, AND expr2 is positive. (case 1 above should handle (int)^*(int/int) already because the (int/int) expression will become double if it's not a whole number, thus it will be (int)^(double) when it gets to the exponent evaluation).
• 3: For any expression returning double, anywhere: check to see if the value has no fractional component. If it doesn't, then convert it to integer before placing on the stack.

[erendrake]

@Dunbaratu i concur. I think that handling that mess for the user is worth the cost.