Move property-descriptor APIs into a minimal js/public/PropertyDescriptor.h header

RESOLVED FIXED in Firefox 66

Status

()

enhancement
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: Waldo, Assigned: Waldo)

Tracking

unspecified
mozilla66
Points:
---

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Comment 1

5 months ago
Posted patch PatchSplinter Review
jsapi.h still contains stuff like JS_GetOwnPropertyDescriptor and such that need JS::PropertyDescriptor, so jsapi.h has to keep #include'ing this.  But it at least slims the header conceptually some, and that's worth it.
Attachment #9033658 - Flags: review?(arai.unmht)
(Assignee)

Updated

5 months ago
Blocks: 1517624
Comment on attachment 9033658 [details] [diff] [review]
Patch

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

::: js/public/PropertyDescriptor.h
@@ +29,5 @@
> +class JS_PUBLIC_API JSTracer;
> +
> +#ifdef JS_BROKEN_GCC_ATTRIBUTE_WARNING
> +#pragma GCC diagnostic pop
> +#endif  // JS_BROKEN_GCC_ATTRIBUTE_WARNING

the above block exists in 3 files (GCAPI.h, ProfilingStack.h, and now this file).
might be nice to move into single place?

::: js/src/jsapi.h
@@ +2591,5 @@
> +/* native that can be called as a ctor */
> +static constexpr unsigned JSFUN_CONSTRUCTOR = 0x400;
> +
> +/* | of all the JSFUN_* flags */
> +static constexpr unsigned JSFUN_FLAGS_MASK = 0x400;

can we add some assertion that this flag doesn't overlap with others in PropertyDescriptor.h ?

maybe define yet another mask (for free space) in PropertyDescriptor.h and compare with it here.

or at least add some comment why this is 0x400 here, and clarify that 0x400 isn't free in PropertyDescriptor.h.
Attachment #9033658 - Flags: review?(arai.unmht) → review+

Comment 3

5 months ago
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6fc24c4e7798
Move property-descriptor APIs into a minimal js/public/PropertyDescriptor.h header, that (at least for now) jsapi.h #includes because it still contains some property-definition stuff.  r=arai

Comment 4

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

Comment 5

4 months ago
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc3c004e74d8
Move property-descriptor APIs into a minimal js/public/PropertyDescriptor.h header, that (at least for now) jsapi.h #includes because it still contains some property-definition stuff.  r=arai
You need to log in before you can comment on or make changes to this bug.