elaborated-enum-base incompatible with NS_ENUM

[Quuxplusone]

Bugzilla Link	PR49413
Status	REOPENED
Importance	P normal
Reported by	Maarten Billemont (lhunath@lyndir.com)
Reported on	2021-03-03 09:02:35 -0800
Last modified on	2021-03-04 04:21:43 -0800
Version	11.0
Hardware	All MacOS X
CC	aaron@aaronballman.com, blitzrakete@gmail.com, dgregor@apple.com, erik.pilkington@gmail.com, llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Related to https://github.com/llvm/llvm-project/commit/c90e198107431f64b73686bdce31c293e3380ac7

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.

[Quuxplusone]

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 NSENUM##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++).

[Quuxplusone]

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

[Quuxplusone]

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.

[Quuxplusone]

(In reply to Richard Smith from comment #3)
> 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.

[Quuxplusone]

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

[Quuxplusone]

(In reply to Richard Smith from comment #5)
> 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.