elaborated-enum-base incompatible with NS_ENUM

[llvmbot]

Bugzilla Link	49413
Version	11.0
OS	MacOS X
Reporter	LLVM Bugzilla Contributor
CC	@AaronBallman,@DougGregor,@zygoloid

Extended Description

Related to c90e198

The new -Welaborated-enum-base appears to be incompatible with macros to define nonfixed type enums.

One such example is Apple's NS_ENUM, which effectively translates:

typedef NS_ENUM( X, unsigned int) {
...
}

into:

typedef enum X : unsigned int X;
enum X : unsigned int {

Which triggers:

fatal error: non-defining declaration of enumeration with a fixed underlying type is only permitted as a standalone declaration; missing list of enumerators? [-Welaborated-enum-base]

The idea behind macros such as NS_ENUM is that they type the enum if supported by the compiler, as well as hide the enum type keyword.

Unfortunately, the only way to do this now would be to remove the nonfixed type from the typedef, but this causes another problem:

error: enumeration previously declared with nonfixed underlying type

This can only be addressed by moving the typedef below the enum declaration, but unfortunately it appears that this is impossible to achieve with a macro.

Consequently, clang can no longer compile NS_ENUM-style macros, and there is no path for resolving the issue other than by disabling elaborated-enum-base.

[zygoloid]

The macro in question generates ill-formed code in C++; note that GCC and ICC also reject. As a result, Clang generates an error by default. (The code generated by the macro is valid in Objective-C and Objective-C++, though.)

Because Clang historically incorrectly accepted this, and in particular because Apple's NS_ENUM macro currently relies on that compiler bug, we allow the error for this non-conforming code to be disabled with -Wno-elaborated-enum-base. If you need to keep building such code and can't fix it to be valid C++, you can use that flag to allow Clang to accept the code (but it will not be portable to other compilers).

Hopefully Apple will fix their macro at some point, if they've not done so already. For example, this can be done with something like:

#ifdef __cplusplus
#define NS_ENUM(name, underlying) \
  void NS_ENUM_##name##_typedef(); \
  enum name : underlying
#else
// old definition
#endif

(where the first line of the macro expansion exists only to effectively swallow the preceding typedef keyword, which is useless in C++).

[llvmbot]

Should this warning only be active for -x c++?

[zygoloid]

The error is not produced in -x objective-c nor in -x objective-c++.

The 'enum name : underlying' syntax is not valid at all in C. As a Clang extension, we permit the C++ language feature in C as well, so the error is produced in -x c++ and in -x c. Apparently we produce no diagnostic for that extension by default (for cases that are valid in C++), but all uses of 'enum E : underlying' are rejected in -x c under -pedantic-errors at least.

Perhaps it would be better if, under -x c, we produced the warning for 'enum name : underlying' being a Clang extension by default, instead of silently accepting the invalid code. If we do, then producing a second extension diagnostic for nesting such an enum in a typedef seems unnecessary and we could turn that off for C compilations (effectively meaning that the C extension picks up the Objective-C feature, not the similar-but-more-constraining C++ feature).

Aaron, as our champion of C conformance, what do you think? Silently accepting this C++ feature in C by default seems at least questionable to me.

[AaronBallman]

The error is not produced in -x objective-c nor in -x objective-c++.

The 'enum name : underlying' syntax is not valid at all in C. As a Clang
extension, we permit the C++ language feature in C as well, so the error is
produced in -x c++ and in -x c. Apparently we produce no diagnostic for that
extension by default (for cases that are valid in C++), but all uses of
'enum E : underlying' are rejected in -x c under -pedantic-errors at least.

Perhaps it would be better if, under -x c, we produced the warning for 'enum
name : underlying' being a Clang extension by default, instead of silently
accepting the invalid code. If we do, then producing a second extension
diagnostic for nesting such an enum in a typedef seems unnecessary and we
could turn that off for C compilations (effectively meaning that the C
extension picks up the Objective-C feature, not the
similar-but-more-constraining C++ feature).

Aaron, as our champion of C conformance, what do you think? Silently
accepting this C++ feature in C by default seems at least questionable to me.

I agree that silently accepting the feature in C by default is not the right behavior.

WG14 is currently considering enumerations with underlying types for C (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2575.pdf) and is trying to get as close to the C++ semantics as possible (but are struggling a bit due to type system differences between C and C++), so my instinct is that we would want Clang to be closer to the C++ feature than the ObjC feature.

[zygoloid]

Thanks, Aaron. It looks like N2575 as currently written does introduce the grammar production

  enum-specifier ::= enum identifier enum-type-specifier[opt]

which is the subject of this bug and would allow the testcases here. C++ has no such production because it makes things like enum E : int const x; ambiguous; is that enum E : (int const) x; (x is not const) or is it (enum E : int) const x; (x is const)?

Instead of adding such a general grammar production that would introduce ambiguities, C++ instead introduced a new form of top-level declaration

  opaque-enum-declaration ::= enum-key attribute-specifier-seq[opt] enum-head-name enum-base[opt] ;

... which is always a standalone declaration and is the only way to introduce an enum name without also specifying a braced list of enumerators.

If C intends to follow C++, the wording will presumably need to be changed. (And if not, it looks like a disambiguation rule is missing.)

my instinct is that we would want Clang to be closer to the C++ feature than
the ObjC feature.

OK, then keeping the error for the NS_ENUM case enabled by default in C seems like the right choice. Also, if N2575 is likely to be accepted by WG14, then leaving the extension warning for this feature off by default is probably reasonable; we usually silently accept conforming extensions from later C standards in earlier standards (and I expect we'll change our default C standard to C2x pretty quickly anyway, since -- so far -- it looks like it'll be backwards-compatible).

[AaronBallman]

Thanks, Aaron. It looks like N2575 as currently written does introduce the
grammar production

enum-specifier ::= enum identifier enum-type-specifier[opt]

which is the subject of this bug and would allow the testcases here. C++ has
no such production because it makes things like 'enum E : int const x;'
ambiguous; is that 'enum E : (int const) x;' (x is not const) or is it
'(enum E : int) const x;' (x is const)?

Instead of adding such a general grammar production that would introduce
ambiguities, C++ instead introduced a new form of top-level declaration

opaque-enum-declaration ::= enum-key attribute-specifier-seq[opt]
enum-head-name enum-base[opt] ;

... which is always a standalone declaration and is the only way to
introduce an enum name without also specifying a braced list of enumerators.

If C intends to follow C++, the wording will presumably need to be changed.
(And if not, it looks like a disambiguation rule is missing.)

I agree. The wording in the current paper is known to be problematic and the author has been very welcoming of feedback on improvements to get better alignment. I'll pass along this bug report as a data point, but in the meantime, I would recommend we see what comes out of WG14 in this space.

my instinct is that we would want Clang to be closer to the C++ feature than
the ObjC feature.

OK, then keeping the error for the NS_ENUM case enabled by default in C
seems like the right choice. Also, if N2575 is likely to be accepted by
WG14, then leaving the extension warning for this feature off by default is
probably reasonable; we usually silently accept conforming extensions from
later C standards in earlier standards (and I expect we'll change our
default C standard to C2x pretty quickly anyway, since -- so far -- it looks
like it'll be backwards-compatible).

Based on everything I know, I think this is a good approach for now.

[llvmbot]

@llvm/issue-subscribers-c

Author: None (llvmbot)

| | | | --- | --- | | Bugzilla Link | [49413](https://llvm.org/bz49413) | | Version | 11.0 | | OS | MacOS X | | Reporter | LLVM Bugzilla Contributor | | CC | @AaronBallman,@DougGregor,@zygoloid |

Extended Description

Related to c90e198

The new -Welaborated-enum-base appears to be incompatible with macros to define nonfixed type enums.

One such example is Apple's NS_ENUM, which effectively translates:

typedef NS_ENUM( X, unsigned int) {
...
}

into:

typedef enum X : unsigned int X;
enum X : unsigned int {

Which triggers:

fatal error: non-defining declaration of enumeration with a fixed underlying type is only permitted as a standalone declaration; missing list of enumerators? [-Welaborated-enum-base]

The idea behind macros such as NS_ENUM is that they type the enum if supported by the compiler, as well as hide the enum type keyword.

Unfortunately, the only way to do this now would be to remove the nonfixed type from the typedef, but this causes another problem:

error: enumeration previously declared with nonfixed underlying type

This can only be addressed by moving the typedef below the enum declaration, but unfortunately it appears that this is impossible to achieve with a macro.

Consequently, clang can no longer compile NS_ENUM-style macros, and there is no path for resolving the issue other than by disabling elaborated-enum-base.