Closed Bug 899859 Opened 6 years ago Closed 6 years ago

Enum class also in structures/classes

Categories

(Core :: MFBT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: guillaume.abadie, Assigned: guillaume.abadie)

References

Details

Attachments

(2 files)

This patch enable to have same feature has MOZ_BEGIN_ENUM_CLASS, in a structure / class.
OS: Mac OS X → All
Hardware: x86 → All
Attached patch patch revision 1Splinter Review
Hi waldo, jgilbert has said me that your are a MFBT reviewer.

I'm consolidating some GL code, and my mind is that would be nice to be able to have C++11's typed enum in classes. This patch introduces 3 defines MOZ_{BEGIN,END,FINISH}_ENUM_CLASS_SCOPE to be able to define an type in an struct or class. So if it's fully compatible with already used MOZ_{BEGIN,END}_ENUM_CLASS.
Attachment #783515 - Flags: review?(jwalden+bmo)
Comment on attachment 783515 [details] [diff] [review]
patch revision 1

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

Seems reasonable.

::: mfbt/TypedEnum.h
@@ +93,5 @@
>   * not support it.
>   *
> + * not than you can also define a strongly=typed enumeration feature C++11
> + * ("enum class") in another class or struct with MOZ_BEGIN_ENUM_CLASS_SCOPE,
> + * MOZ_END_ENUM_CLASS_SCOPE and MOZ_FINISH_ENUM_CLASS_SCOPE. For example,

You have a few typos here, and the wording is a bit weirdish.  How about:

MOZ_{BEGIN,END}_ENUM_CLASS doesn't work for defining enum classes nested inside classes.  To define an enum class nested inside another class, use MOZ_{BEGIN,END}_NESTED_ENUM_CLASS, and place a MOZ_FINISH_NESTED_ENUM_CLASS in namespace scope to handle bits that can only be implemented with namespace-scoped code.  For example:

@@ +97,5 @@
> + * MOZ_END_ENUM_CLASS_SCOPE and MOZ_FINISH_ENUM_CLASS_SCOPE. For example,
> + *
> + *   class FooBar {
> + *
> + *     MOZ_BEGIN_ENUM_CLASS_SCOPE(Enum, int32_t)

Let's make this MOZ_{BEGIN,END}_NESTED_ENUM_CLASS, to be slightly clearer maybe about the why of it.

@@ +115,5 @@
> +#  define MOZ_BEGIN_ENUM_CLASS_SCOPE(Name, type) \
> +     enum class Name : type {
> +#  define MOZ_END_ENUM_CLASS_SCOPE(Name) \
> +     };
> +#  define MOZ_FINISH_ENUM_CLASS_SCOPE(Name)

Add a /* nothing */ after this for readability.
Attachment #783515 - Flags: review?(jwalden+bmo) → review+
(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #3)
> Comment on attachment 783515 [details] [diff] [review]
> patch revision 1
> 
> Review of attachment 783515 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Seems reasonable.
> 
> ::: mfbt/TypedEnum.h
> @@ +93,5 @@
> >   * not support it.
> >   *
> > + * not than you can also define a strongly=typed enumeration feature C++11
> > + * ("enum class") in another class or struct with MOZ_BEGIN_ENUM_CLASS_SCOPE,
> > + * MOZ_END_ENUM_CLASS_SCOPE and MOZ_FINISH_ENUM_CLASS_SCOPE. For example,
> 
> You have a few typos here, and the wording is a bit weirdish.  How about:
> 
> MOZ_{BEGIN,END}_ENUM_CLASS doesn't work for defining enum classes nested
> inside classes.  To define an enum class nested inside another class, use
> MOZ_{BEGIN,END}_NESTED_ENUM_CLASS, and place a MOZ_FINISH_NESTED_ENUM_CLASS
> in namespace scope to handle bits that can only be implemented with
> namespace-scoped code.  For example:
Oups... Fixed! =P
> 
> @@ +97,5 @@
> > + * MOZ_END_ENUM_CLASS_SCOPE and MOZ_FINISH_ENUM_CLASS_SCOPE. For example,
> > + *
> > + *   class FooBar {
> > + *
> > + *     MOZ_BEGIN_ENUM_CLASS_SCOPE(Enum, int32_t)
> 
> Let's make this MOZ_{BEGIN,END}_NESTED_ENUM_CLASS, to be slightly clearer
> maybe about the why of it.
Fixed.
> 
> @@ +115,5 @@
> > +#  define MOZ_BEGIN_ENUM_CLASS_SCOPE(Name, type) \
> > +     enum class Name : type {
> > +#  define MOZ_END_ENUM_CLASS_SCOPE(Name) \
> > +     };
> > +#  define MOZ_FINISH_ENUM_CLASS_SCOPE(Name)
> 
> Add a /* nothing */ after this for readability.
Fixed.
Here is the patch that will be landed.
https://hg.mozilla.org/integration/mozilla-inbound/rev/646082ecc3a9
Target Milestone: --- → mozilla25
https://hg.mozilla.org/mozilla-central/rev/646082ecc3a9
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.