Closed Bug 1516796 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: Waldo, Assigned: Waldo)

References

Details

Attachments

(1 file)

No description provided.
Attached 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)
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+
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
https://hg.mozilla.org/mozilla-central/rev/6fc24c4e7798
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
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.