Move {js::,JS_}{{Strictly,Loosely}Equal,SameValue} into js/public/Equality.h and js/src/vm/EqualityOperations.{cpp,h}

RESOLVED FIXED in Firefox 66

Status

()

enhancement
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: Waldo, Assigned: Waldo)

Tracking

unspecified
mozilla66
Points:
---

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

5 months ago
More slimming of jsapi.h.
Assignee

Comment 1

5 months ago
Posted patch PatchSplinter Review
Attachment #9033611 - Flags: review?(arai.unmht)
Comment on attachment 9033611 [details] [diff] [review]
Patch

Review of attachment 9033611 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/public/Equality.h
@@ +23,5 @@
> + *
> + * This operation can fail only if an internal error occurs (e.g. OOM while
> + * linearizing a string value).
> + */
> +extern JS_PUBLIC_API bool StrictlyEqual(JSContext* cx,

it's worrisome to have identical name/signature functions between JS:: and js:: with different behavior (it's only in debug build and nightly tho),
given some function definition may be inside a big namespace block, and the function call itself isn't clear which one it's calling if it lacks namespace prefix.

might be nice to somehow force namespace prefix in such case (I don't come up with solution tho...)

::: js/src/vm/EqualityOperations.h
@@ +3,5 @@
> + * This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +/** The equality comparisons of js/Equality.h, but with internal visibility. */

can you explicitly write the difference between js:: and JS:: functions here and/or in Equality.h ?
Attachment #9033611 - Flags: review?(arai.unmht) → review+
Assignee

Comment 3

5 months ago
An unprefixed function call that could to multiple functions (because of |using namespace js;| or so) is ambiguous, which means it'll be a compile error.  If you're in a function defined inside the js:: namespace, tho, without any usings bringing the JS:: one into view, there's no need to prefix.

I also beefed up the comment there a bit as to differences.  I did not change the js/public comments, tho -- users of public APIs ought not know about internal private APIs they cannot actually touch themselves; this complexity is something only we should ever be thinking about.

Will probably land...well, today.

Comment 4

5 months ago
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab2e4a2e5dd2
Move {js::,JS_}{{Strictly,Loosely}Equal,SameValue} into js/public/Equality.h and js/src/vm/EqualityOperations.{cpp,h}.  r=arai

Comment 5

5 months ago
https://hg.mozilla.org/mozilla-central/rev/ab2e4a2e5dd2
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

Comment 6

5 months ago
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf02f2ce30a2
Move {js::,JS_}{{Strictly,Loosely}Equal,SameValue} into js/public/Equality.h and js/src/vm/EqualityOperations.{cpp,h}.  r=arai
You need to log in before you can comment on or make changes to this bug.