Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSRT API to access javascript relational comparison function #3568

Closed
marktjsonar opened this issue Aug 22, 2017 · 6 comments
Closed

JSRT API to access javascript relational comparison function #3568

marktjsonar opened this issue Aug 22, 2017 · 6 comments

Comments

@marktjsonar
Copy link

It would be tremendously convenient if there were a JSRT API that could compare the magnitude of two JsValueRef's, as per the the ecmascript abstract relational comparison algorithm.

@liminzhu liminzhu changed the title [suggested tag: feature request] JSRT API to access javascript relational comparison function JSRT API to access javascript relational comparison function Aug 23, 2017
@liminzhu
Copy link
Collaborator

liminzhu commented Aug 23, 2017

Thanks for the feature request @markt- .

This seems generally useful. Any proposal/suggestion on the shape of the API? Given undefined can be a valid result, I imagine returning a JsValueRef rather than a bool, which can be very annoying to deal with.

@marktjsonar
Copy link
Author

marktjsonar commented Aug 23, 2017

Indeed, I would expect a JsValueRef as well. Testing for boolean value true or false would generally involve calling a JsGetValueType() call on the result first to make sure it isn't undefined.

The API, I would imagine, would have a function that takes two JsValueRef's, a boolean indicating whether to evaluate the left or right side first, and a pointer to the JsValueRef to put the result in.

This, I think, is the cleanest solution.

Alternative approaches that just output a bool, or even a 3-way comparison function return -1, 0, or 1 to indicate two values' relatve magnitudes have deficiencies when working with javascript types in that where a is not equal to b and a is not less than b, for instance, does not necessarily translate to a being greater than b, where this point is mitigated by allowing the relational comparison function being able to return undefined. In my view the only right way to deal with this other than to use the aforementioned abstract relational comparison algorithm would be to define a very general API comparison function that takes two values and outputs a small bitvector (stored within a single int), which explicitly indicates which comparative relation operators would succeed for those values, and there would be one bit used for each relational comparison operation, ie, each of ==, !=, <=, >=, <, and >. It's worth noting that for any given relational comparison, multiple bits would often be set in this output (eg, for the values 2 and 3 in that order, the bits corresponding to !=, <, and <= would each be set). The only concern I would have with this being the partial overlap of such a function with the output of the existing JsEquals API call.

@liminzhu
Copy link
Collaborator

liminzhu commented Aug 23, 2017

// + spec guru @bterlson and @curtisman for feedback

Thanks for the suggestion! I got some more time to look into the spec. Instead of the abstract relational comparison algorithm (ARCA), would an API that mirrors the < operator (alway returns a boolean) be more useful and intuitive? From what I've read in the spec, it looks like undefined from ARCA is mapped as false. In addition to having LHS and RHS as in-param, the API can have an additional in-param to indicate whether the host wants < or <=. Something like,

JsLessThan(
  _In_ JsValueRef lhs,
  _In_ JsValueRef rhs,
  _In_ bool allowsEqual,
  _out_ bool *lessThan
)

Of course this means a>b or a>=b where a is evaluated first is missing, but I'm not sure many would need that.

@marktjsonar
Copy link
Author

When I first saw your comment, I was concerned about the possibility of comparing two different non-primitive values, where both <= and >= comparisons could return true, but ==, <, and > would all return false, but it seems to be true in all cases where the comparison will not throw an exception that regardless of what < may evaluate to, >= is always the opposite and regardless of what <= is, > is always the opposite as well. So yes, that would probably be fine.

@bterlson
Copy link
Contributor

@liminzhu your proposed API is good I think. What it loses from the ARC algorithm is the signal that any of the parameters are NaN. This doesn't seem too important to me.

@liminzhu
Copy link
Collaborator

Shall we tag this as accepting PR? @akroshg

chakrabot pushed a commit that referenced this issue Oct 18, 2017
Merge pull request #3931 from xiaoyinl:JsLessThan

This PR implements the API JsLessThan and JsLessThanOrEqual, proposed in issue #3568.

Fixes #3568

Todo:
- [x] Add more test cases
- [x] Update documents (microsoft/ChakraCore-wiki#43)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants