Closed Bug 899652 Opened 8 years ago Closed 8 years ago

eliminate static constructors for WebIDL constant tables on constexpr-supporting compilers

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: froydnj, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Waldo informed me over IRC yesterday that JS::UndefinedValue() should be used instead
of JSVAL_VOID.  Since the former is a function and can be constexpr-folded, let's use
that instead.
Attachment #783221 - Flags: review?(bzbarsky)
Some simple modifications to make generatePrefableArray attach the appropriate
constexpr qualifier when necessary.  Note that this depends on the code in bug
899309.

I'm double-checking right now with Try whether the arrays actually need
to be constexpr in order to fold them to static data with GCC.
Attachment #783224 - Flags: review?(bzbarsky)
Comment on attachment 783221 [details] [diff] [review]
use JS::UndefinedValue instead of JSVAL_VOID to encourage constexpr-ness

Oy, JSVAL_VOID is no longer a constant expression?  :(  Why did it get changed to not be the same as UndefinedValue()??

In any case, this is definitely a good change.  r=me

That said, should we file a followup to mark UndefinedValue as MOZ_CONSTEXPR?
Attachment #783221 - Flags: review?(bzbarsky) → review+
> That said, should we file a followup to mark UndefinedValue as MOZ_CONSTEXPR?

Ah, I see, bug 899309 is that.
(In reply to Boris Zbarsky (:bz) from comment #3)
> Oy, JSVAL_VOID is no longer a constant expression?  :(  Why did it get
> changed to not be the same as UndefinedValue()??

JSVAL_VOID has never (?) been a constant expression:

http://mxr.mozilla.org/mozilla-central/source/js/src/jsapi.h#1175

Maybe you are thinking of JSID_VOID?
> JSVAL_VOID has never (?) been a constant expression:

As of rev 5a3e49205389, the definition in opt builds was:

#define JSVAL_VOID   BUILD_JSVAL(JSVAL_TAG_UNDEFINED, 0)

(and at all points before that JSVAL_VOID was definitely a constant integer value) but this got changed to the current setup in bug 684526, looks like....  I just missed it at the time.

We should go through Gecko code and get rid of all uses of JSVAL_*.
Comment on attachment 783224 [details] [diff] [review]
modify bindings codegen to make webidl constant arrays constexpr

>+        arraySpecType = ("JS_VALUE_CONSTEXPR_VAR " if useValueConstexpr else "const ") + specType

If we can just always have the JS_VALUE_CONSTEXPR_VAR here, that would be great; then we don't need the new useValueConstexpr argument.

r=me either way.
Attachment #783224 - Flags: review?(bzbarsky) → review+
Comment on attachment 783224 [details] [diff] [review]
modify bindings codegen to make webidl constant arrays constexpr

It looks like bare 'const' is sufficient; I must have thought constexpr was needed based on some earlier bogus tests.  So that's a nice bonus.  I'll just be checking in the first part.
Attachment #783224 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/b0e1bdb61af3
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.