899859 - Enum class also in structures/classes

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Description

[Guillaume Abadie]

This patch enable to have same feature has MOZ_BEGIN_ENUM_CLASS, in a structure / class.

Comment 1

[Guillaume Abadie]

Attachment: patch revision 1

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.

Comment 2

[Guillaume Abadie]

As green as an apple : https://tbpl.mozilla.org/?tree=Try&rev=8e5f235a1f89

Comment 3

[Jeff Walden [:Waldo]]

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.

Comment 4

[Guillaume Abadie]

(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.

Comment 5

[Guillaume Abadie]

Attachment: patch for landing

Here is the patch that will be landed.

Comment 6

[Guillaume Abadie]

https://hg.mozilla.org/integration/mozilla-inbound/rev/646082ecc3a9

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/646082ecc3a9