1125096 - Implement JS_DECLARE_NEW_METHODS and JS_DECLARE_MAKE_METHODS using variadic templates

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

These macros are complete garbage right now, with a gazillion cases repeated almost identically.  Happily variadic templates allow us to write a single method and be done with it.

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Patch

I love how the variadic template syntax forces us to write types with the reference sigil next to the type, as God intended -- not next to the name as is the spawn of Satan syntax.  ;-)

Also whose idea was it ever that putting the backslashes in macros *right up* against the last character on a line ever preserved readability.  :-\  Fixed.

Comment 2

[Jason Orendorff [:jorendorff]]

Comment on attachment 8553684 [details] [diff] [review]
Patch

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

Ahhh. Very nice.

::: js/public/Utility.h
@@ +206,5 @@
>   *
>   * Note: Do not add a ; at the end of a use of JS_DECLARE_MAKE_METHODS,
>   * or the build will break.
>   */
>  #define JS_DECLARE_MAKE_METHODS(MAKENAME, NEWNAME, QUALIFIERS)\

This one is only used in one place (in MallocProvider) and I don't see how there could be a bunch more of them, JS::DeletePolicy being hardcoded in there. So I wouldn't object to deleting this macro, expanding it in the one place where it's used. Your call.

Comment 3

[Jeff Walden [:Waldo]]

Opted to leave the macro bits, somewhat out of laziness, for now.

If we can somehow kill off JS_DECLARE_NEW_METHODS such that *both* are only used/defined inside MallocProvider, at that point I don't mind inlining.  But for now, I think you sort of want them both the same, defined the same place, so their definitions evolve (if they evolve) in lockstep, or at least more likely such.

https://hg.mozilla.org/integration/mozilla-inbound/rev/0db39f732328

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0db39f732328