scratchfoundation / scratch-vm

Virtual Machine used to represent, run, and maintain the state of programs for Scratch 3.0
http://scratchfoundation.github.io/scratch-vm/
BSD 3-Clause "New" or "Revised" License
1.21k stars 1.51k forks source link

Operators - Should `1/0` equal `Infinity`? #234

Open thisandagain opened 8 years ago

thisandagain commented 8 years ago

Follow-up discussion from latest test coverage PR #233 as well as discussion from Scratch 2.0: https://github.com/LLK/scratch-flash/issues/1060

thisandagain commented 8 years ago

A summary of my thoughts on the subject are here: https://github.com/LLK/scratch-flash/issues/1060#issuecomment-204765781

tmickel commented 8 years ago

This seems like a tricky topic. Options I see:

  1. Throw an error, like Scratch 1.4, Python, etc.
  2. Return undefined
  3. Return NaN
  4. Return +/- Infinity, as current.

Option 1 (errors): I would strongly suspect there are many Scratch 2.0 projects that could end up dividing by zero at some point in their execution - e.g.,:

screen shot 2016-10-04 at 11 19 54 am

For this reason I agree in leaning against going back to error throwing like Scratch 1.4 (it would likely break projects). And especially because I don't think we have any other scenarios that need/want error throwing.


Option 2 (undefined): I'm a little hesitant to return a literal undefined, because afaik at least in JavaScript this doesn't have mathematical semantics (e.g., it usually means "this variable is undefined", not "this is mathematically undefined"). It could also have compatibility implications:

screen shot 2016-10-04 at 11 33 16 am

But of course we could patch around these.


Option 3 (NaN): Probably has the same compatibility questions as undefined, although NaN is already returned for some operations in Scratch, too...


Option 4 (status quo): I personally don't mind this, but I understand it could be confusing. To complicate, how do we feel about other situations that can return Infinity? Like this:

screen shot 2016-10-04 at 11 47 26 am
tmickel commented 8 years ago

P.s, fairly cogent: https://scratch.mit.edu/discuss/topic/220324/?page=1#post-2235372

@carljbowman Thoughts on displaying warnings (not errors) in Scratch?

cwillisf commented 8 years ago

I don't have a strong opinion on what 1/0 should return. IEEE 754 says that it should either return ±infinity or it should throw an exception, and I'm inclined to avoid throwing errors for the same reasons that @tmickel mentioned, so I think I'd default to returning ±infinity. I could probably be convinced to go with one of the other options, though, if there's a way in which it could help younger Scratchers understand what's going on.

I really like the idea of associating runtime warnings with blocks in some subtle, "investigate if you want" sort of way. I can imagine doing this for any of the possible IEEE 754 exceptions that we can detect, messages that aren't received, trying to set a costume or play a sound that doesn't exist, etc.

I think we should also fix NaN being treated as zero (due to string parsing?) as mentioned in the forum post that @tmickel linked: https://scratch.mit.edu/discuss/topic/220324/?page=1#post-2235372

tmickel commented 8 years ago

The more I think about this, the more my opinion is that we should leave it as-is, +/- Infinity. It would keep things compatible, and maintains mathematical semantics like Infinity > -Infinity.

@cwillisf Regarding NaN being cast as 0: I think this actually makes sense for Scratch. For example, when you do [move (NaN) steps], the interpreter has to handle this in some way, and I think the most reasonable approach is to move 0 steps.

thisandagain commented 8 years ago

I agree with keeping the spec as-is.

cwillisf commented 8 years ago

@tmickel I definitely agree that "move NaN steps" should move 0 steps. My concern is that we "squash" NaN to zero as soon as possible rather than allowing it to propagate through mathematical expressions.

I think if we ever show "NaN" to a user we should be consistent in the way we treat it, which IMO means treating it the same way that IEEE 754 treats it. One specific example: sqrt(-1) + 0 should be NaN, but currently is 0.

tmickel commented 8 years ago

@cwillisf I think that should be an easy implementation. Add a new number cast method: https://github.com/LLK/scratch-vm/blob/develop/src/util/cast.js#L25

Cast.toNumberOperators, or something, that doesn't have the NaN-squashing property.

However I am slightly nervous that this will break compatibility with Scratch 2.0 and Scratch 1.4 operators. While in Scratch NaN might be squashed early in a long expression, leaving a useful value at the end, this change may cause a similar expression to propagate NaN to the end, which will be squashed to 0 at the end if used in a block.

tmickel commented 8 years ago

This could be a candidate for compatibility mode, maybe.

cwillisf commented 8 years ago

For what it's worth, Scratch 1.4 reports that sqrt(-1) is "Error!" and sqrt(-1) + 1 is also "Error!". I do recognize that we might break some Scratch 2.0 projects, though. Compatibility mode might be the right way to go with that. Hmmm...