Closed Bug 1111290 Opened 5 years ago Closed 5 years ago

replace TypedEnum.h usage with real enum classes

Categories

(Core :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38

People

(Reporter: heycam, Assigned: emk)

References

Details

Attachments

(4 files)

According to https://developer.mozilla.org/en-US/docs/Using_CXX_in_Mozilla_code we should be able to use enum classes one we drop support for MSVC 2010.
Note that GCC emits a warning (wrongly) if you use a typed enum (enum class as well?) as a bit field type, so be careful when making the switch.
BTW I think we already should be able to use typed enums instead of MOZ_ENUM_TYPE (according to the above wiki page), but dropping MSVC 2010 support also lets us use enum classes instead of MOZ_BEGIN_ENUM_CLASS etc.  We'll want to keep MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS though.
Executed

convert 'MOZ_BEGIN_ENUM_CLASS(\([^,]*\),\s*\([^)]*\))' 'enum class \1 : \2 {'
convert 'MOZ_BEGIN_ENUM_CLASS(\([^,)]*\))' 'enum class \1 {'
convert 'MOZ_END_ENUM_CLASS([^,)]*)' '};'

on the tree. See bug 1118486 comment #1 about the convert function.
Attachment #8554130 - Flags: review?(jwalden+bmo)
Attachment #8554131 - Flags: review?(jwalden+bmo)
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Attachment #8554132 - Flags: review?(jwalden+bmo)
Comment on attachment 8554130 [details] [diff] [review]
Remove MOZ_(BEGIN|END)_ENUM_CLASS

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

It's a bit odd how many of these enums seem to be explicitly requesting int as the underlying type, given that's to some approximation what will be used by default in most situations.  Wonder why this code requests it so often...

::: mfbt/tests/TestTypedEnum.cpp
@@ +111,5 @@
>  MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(Nested::AutoEnumBitField)
>  MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(Nested::CharEnumBitField)
>  
>  #define MAKE_STANDARD_BITFIELD_FOR_TYPE(IntType)                   \
> +  enum class BitFieldFor_##IntType : IntType {             \

Realign here.

@@ +116,4 @@
>      A = 1,                                                         \
>      B = 2,                                                         \
>      C = 4,                                                         \
> +  };                        \

And here.
Attachment #8554130 - Flags: review?(jwalden+bmo) → review+
Attachment #8554131 - Flags: review?(jwalden+bmo) → review+
Comment on attachment 8554132 [details] [diff] [review]
Remove MOZ_HAVE_CXX11_STRONG_ENUMS and macro definitions

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

::: mfbt/TypedEnum.h
@@ -53,5 @@
>   *   };
>   *
>   *   MOZ_FINISH_NESTED_ENUM_CLASS(FooBar::Enum)
>   */
> -#if defined(MOZ_HAVE_CXX11_STRONG_ENUMS)

It's fine enough just removing this all for reviewing purposes.  But you should go back and unindent everything in this if-block, once you've done this.  And it looks like there's a bunch of comments documentation above that needs editing to remove a bunch of detritus.  Separate patch to do that, please?

::: mfbt/tests/TestTypedEnum.cpp
@@ +159,5 @@
>  TestNonConvertibilityForOneType()
>  {
>    using mozilla::IsConvertible;
>  
> +#if defined(MOZ_HAVE_EXPLICIT_CONVERSION)

I've reviewed a patch somewhere else that might have removed this last test as well -- you might be racing someone to land.  :-)
Attachment #8554132 - Flags: review?(jwalden+bmo) → review+
Comment on attachment 8554160 [details] [diff] [review]
Remove TypedEnum.h and fold TypedEnumInternal.h into TypedEnumBits.h

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

::: mfbt/tests/TestTypedEnum.cpp
@@ -10,1 @@
>  #include "mozilla/TypedEnumBits.h"

I am now grateful that I insisted on the bitwise operator additions to typed arrays being in a separate header from TypedEnum.h, back in the day -- no worries about anyone bootlegging those macros from TypedEnum.h.  :-)
Attachment #8554160 - Flags: review?(jwalden+bmo) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7d1736f58e1
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fab2faa7f9d
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f7f003696ad

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #9)
> It's fine enough just removing this all for reviewing purposes.  But you
> should go back and unindent everything in this if-block, once you've done
> this.  And it looks like there's a bunch of comments documentation above
> that needs editing to remove a bunch of detritus.  Separate patch to do
> that, please?

I folded the next patch instead :)
Flags: in-testsuite-
Blocks: 1126269
You need to log in before you can comment on or make changes to this bug.