Normative: Allow NaN values to be *optionally* canoncalized
This patch legalizes V8's occasional canonicalization of NaNs by changing SetValueInBuffer to either a particular value for the implementation-distinguishable NaN value or an implementation-defined canonical value.
This semantic change reached consensus at the November 2016 TC39 meeting.
Closes https://github.com/tc39/ecma262/issues/635
This patch still needs the associated test262 update. The tests that are affected include these, but maybe more:
built-ins/Object/internals/DefineOwnProperty/nan-equivalence
built-ins/TypedArray/prototype/fill/fill-values-conversion-operations-consistent-nan
built-ins/TypedArrays/internals/DefineOwnProperty/conversion-operation-consistent-nan
built-ins/TypedArrays/internals/Set/conversion-operation-consistent-nan
cc @erights @allenwb
@allenwb does this seem fine to you? (Just checking as I think you were involved in the discussions closely at the last meeting).
What do we need from here to decide whether to land this patch?
@jfbastien has clarified that, even if this patch lands, we won't really have a full description of what happens with NaN values in practice. This is because processors sometimes change the NaN bit pattern later, e.g., in MOV instructions, in a way that JS implementations which don't canonicalize are probably not guarding against.
Even as I'd personally prefer that we leave the NaN bit pattern completely unspecified, I think this patch still brings us a little bit closer to reality and would be helpful to land.
Thanks for the reviews everyone!
@littledan would you mind rebasing this PR, and summarizing the current status? I believe it has consensus, and i'm not sure if it still needs tests.
@rwaldron can you rereview after the PR is rebased?
Well, I would not feel comfortable making the change suggested above, to give more guarantees to Float32, given that this is unlikely to correspond to actual implementations. I would rather move this patch to removing any guarantees about how floats are canonicalized, given the information from @jfbastien, about how this is the implementation reality and is unlikely to change. @waldemarhorwat encouraged this path as well. @erights, would you be OK with this change?
Yes, this change looks good to me.
@erights To be concrete, I'm talking about, not this PR, but instead a PR to say that there's no stability for NaN (since this is the implementation reality).
@erights To be concrete, I'm talking about, not this PR, but instead a PR to say that there's no stability for NaN (since this is the implementation reality).
Thanks for clarifying. I did not know that. Link to other PR?