Closed Bug 1253265 Opened 4 years ago Closed 4 years ago

make some nsRestyleHint operators constexpr

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: tbsaunde, Unassigned)

Details

Attachments

(1 file)

This gets rid of at least one static constructor
Comment on attachment 8726243 [details] [diff] [review]
make some nsRestyleHint operators constexpr

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

::: layout/base/nsChangeHint.h
@@ +463,5 @@
>  // The functions below need an integral type to cast to to avoid
>  // infinite recursion.
>  typedef decltype(nsRestyleHint(0) + nsRestyleHint(0)) nsRestyleHint_size_t;
>  
> +inline MOZ_CONSTEXPR nsRestyleHint operator|(nsRestyleHint aLeft,

Is there any benefit to the "inline" annotation here, or does it become vestigial now that this is MOZ_CONSTEXPR? (evaluated at compile-time)

From a mxr search, it looks like the extremely-vast majority of our MOZ_CONSTEXPR functions do not use "inline". So, if you agree that MOZ_CONSTEXPR makes it unnecessary/redundant/vestigial, feel free to remove that keyword as part of this change.

r=me either way.
Attachment #8726243 - Flags: review?(dholbert) → review+
(In reply to Daniel Holbert [:dholbert] (mostly AFK March 3rd - 6th) from comment #2)
> From a mxr search, it looks like the extremely-vast majority of our
> MOZ_CONSTEXPR functions do not use "inline". So, if you agree that
> MOZ_CONSTEXPR makes it unnecessary/redundant/vestigial, feel free to remove
> that keyword as part of this change.

But methods defined inline within the definition of a class are implicitly |inline|, so they don't need |inline|.  These are not class methods, so I think they should have it.
Thanks, that makes sense. The patch should land as-is, then; disregard my question in comment 2.
https://hg.mozilla.org/mozilla-central/rev/69d55ff0a1da
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.