Skip to content

toLeopard: Static number casting issues in equality operator + NaN leaking #123

@towerofnix

Description

@towerofnix

Branching from #9 (comment).

We try to use === for operator_equals where possible, but at the moment are too greedy. Casting to a number doesn't work exactly the same in Cast.compare and that's something sb-edit is currently unaware of.

We only use === in operator_equals if at least one of the arguments can be parsed as a number. Since it's impossible to represent NaN this way (parseNumber only returns true if the value is non-NaN after casting to number), we don't need to worry about (...) === NaN type comparisons (which are treated as String(...).toLowerCase() === "nan" in Scratch).

Therefore, if a block expression returns NaN, it can't be equal to the written number value. This will be useful later.

We similarly have an issue where the division operator can return NaN (zero divided by zero) and this is treated as satisfying number inputs. While this is fine for certain situations, in the context of most math operators (in Scratch), NaN is supposed to be treated as zero. Thus NaN may leak from operators which currently identify as satisfying Number, and this is a compatibility issue.

My proposal is to implement a "strict" mode for casting to number in Leopard: this.toNumber(..., true). It would report NaN for any values which can never resolve a numerical comparison (in Scratch). This isn't drastically complicated; it mostly means returning NaN for cases we already detect (and currently return 0 for). It also means specifically banning the values null and whitespace-only strings from casting to zero (see Cast.isWhiteSpace.)

Then replace InputShape.Number with three new shapes:

  • InputShape.NumberPossiblyNaN: A number value which was representable as a number in the first place or is now represented as NaN.
    • This is indirectly satisfied by NumberNotNaN.
    • Directly satisfied by the division operator (0/0 = NaN) if, and only if, both of its inputs are blocks or one of its inputs is zero.
    • Probably not directly satisfied by any other ops, but check operator_mathop for sure.
    • To cast: this.toNumber((...), true)
  • InputShape.NumberNotNaN: A number value which was representable as a number in the first place or is now represented as zero.
    • This is not indirectly satisfied by NumberPossiblyNaN.
    • Directly satisfied by most math operators. (Because math ops treat NaN as zero, NaN will immediately be "squashed" into zero if it's passed into another math op, rather than leaking. Scratch casts NaN to zero every time; we will, when we have to, by making these math ops take NumberNotNaN inputs.)
    • To cast: this.toNumber((...))
  • InputShape.LooseNumber: A number value for which 0 and NaN are dynamically treated as equal.
    • Indirectly satisfied by both NumberNotNaN and NumberPossiblyNaN.
    • Not directly satisfied by anything.
    • To cast: this.toNumber((...))

Then differentiate the desired input types for all existing uses of InputShape.Number. For example, math blocks require NumberNotNaN. Equals-comparison of numbers requires NumberPossiblyNaN. Blocks like "repeat", "wait", and "say and wait" all benefit from using LooseNumber because they will still behave correctly if provided NaN, treating that value as zero, and don't require casting more strictly like equals-comparison - casting to zero instead of NaN is fine.

We also need to adapt staticBlockInputToLiteral. Since it already takes a desired input shape, this doesn't necessitate an (internal) interface change, conveniently. At the moment values like 3ee3e3e3e33eee333 always get returned as strings, which is clearly dangerous. (These are functionally NaN being returned for the desired input shape Number!) Non-numeric values should be cast to zero or NaN adapting to the desired input shape.

In summary, we want to avoid using the casting provided by NumberPossiblyNaN everywhere except for equals-comparison; we also want to ensure that NaN never leaks out of division and into operators that are supposed to treat NaN as zero but don't currently perform any dynamic casting for any number-shaped reporters.

Although the implementation for all this is relatively trivial, it's quite a bit to reason about when ensuring Leopard number behavior matches Scratch number behavior. But these tools are necessary, and together, enable rather clean translation. In the vast majority of cases, only equals-comparison is affected; division is also affected when both operands are a block or one operand is zero, but not when one of the operands is a non-zero constant - likely a large portion of uses of division in general. And inputs which don't care whether passed NaN or zero don't get affected at all, regardless what the reporter satisfies.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcompatibilityMismatch or currently unsupported language behaviorfmt: LeopardPertains to Leopard format (JavaScript)

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions