Skip to content

Commit

Permalink
Fix parsing of enum-base to follow C++11 rules.
Browse files Browse the repository at this point in the history 
Previously we implemented non-standard disambiguation rules to
distinguish an enum-base from a bit-field but otherwise treated a :
after an elaborated-enum-specifier as introducing an enum-base. That
misparses various examples (anywhere an elaborated-type-specifier can
appear followed by a colon, such as within a ternary operator or
_Generic).

We now implement the C++11 rules, with the old cases accepted as
extensions where that seemed reasonable. These amount to:
 * an enum-base must always be accompanied by an enum definition (except
   in a standalone declaration of the form 'enum E : T;')
 * in a member-declaration, 'enum E :' always introduces an enum-base,
   never a bit-field
 * in a type-specifier (or similar context), 'enum E :' is not
   permitted; the colon means whatever else it would mean in that
   context.

Fixed underlying types for enums are also permitted in Objective-C and
under MS extensions, plus as a language extension in all other modes.
The behavior in ObjC and MS extensions modes is unchanged (but the
bit-field disambiguation is a bit better); remaining language modes
follow the C++11 rules.

Fixes PR45726, PR39979, PR19810, PR44941, and most of PR24297, plus C++
core issues 1514 and 1966.
  • Loading branch information
@zygoloid
zygoloid committed May 9, 2020
1 parent 49b32d8 commit c90e198
Show file tree
Hide file tree
Showing 18 changed files with 298 additions and 224 deletions.
8 changes: 8 additions & 0 deletions clang/include/clang/Basic/DiagnosticParseKinds.td
View file
Expand Up @@ -105,6 +105,14 @@ def ext_clang_c_enum_fixed_underlying_type : Extension<
def warn_cxx98_compat_enum_fixed_underlying_type : Warning<
"enumeration types with a fixed underlying type are incompatible with C++98">,
InGroup<CXX98Compat>, DefaultIgnore;
def ext_enum_base_in_type_specifier : ExtWarn<
"non-defining declaration of enumeration with a fixed underlying type is "
"only permitted as a standalone declaration"
"%select{|; missing list of enumerators?}0">, InGroup<DiagGroup<"enum-base">>;
def err_anonymous_enum_bitfield : Error<
"ISO C++ only allows ':' in member enumeration declaration to introduce "
"a fixed underlying type, not an anonymous bit-field">;

def warn_cxx98_compat_alignof : Warning<
"alignof expressions are incompatible with C++98">,
InGroup<CXX98Compat>, DefaultIgnore;
Expand Down
2 changes: 2 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Expand Up @@ -5484,6 +5484,8 @@ def err_bitfield_width_exceeds_type_width : Error<
def err_anon_bitfield_width_exceeds_type_width : Error<
"width of anonymous bit-field (%0 bits) exceeds %select{width|size}1 "
"of its type (%2 bit%s2)">;
def err_anon_bitfield_init : Error<
"anonymous bit-field cannot have a default member initializer">;
def err_incorrect_number_of_vector_initializers : Error<
"number of elements must be either one or match the size of the vector">;

Expand Down
77 changes: 68 additions & 9 deletions clang/include/clang/Parse/Parser.h
View file
Expand Up @@ -2181,6 +2181,68 @@ class Parser : public CodeCompletionHandler {
llvm_unreachable("Missing DeclSpecContext case");
}

/// Whether a defining-type-specifier is permitted in a given context.
enum class AllowDefiningTypeSpec {
/// The grammar doesn't allow a defining-type-specifier here, and we must
/// not parse one (eg, because a '{' could mean something else).
No,
/// The grammar doesn't allow a defining-type-specifier here, but we permit
/// one for error recovery purposes. Sema will reject.
NoButErrorRecovery,
/// The grammar allows a defining-type-specifier here, even though it's
/// always invalid. Sema will reject.
YesButInvalid,
/// The grammar allows a defining-type-specifier here, and one can be valid.
Yes
};

/// Is this a context in which we are parsing defining-type-specifiers (and
/// so permit class and enum definitions in addition to non-defining class and
/// enum elaborated-type-specifiers)?
static AllowDefiningTypeSpec
isDefiningTypeSpecifierContext(DeclSpecContext DSC) {
switch (DSC) {
case DeclSpecContext::DSC_normal:
case DeclSpecContext::DSC_class:
case DeclSpecContext::DSC_top_level:
case DeclSpecContext::DSC_alias_declaration:
case DeclSpecContext::DSC_objc_method_result:
return AllowDefiningTypeSpec::Yes;

case DeclSpecContext::DSC_condition:
case DeclSpecContext::DSC_template_param:
return AllowDefiningTypeSpec::YesButInvalid;

case DeclSpecContext::DSC_template_type_arg:
case DeclSpecContext::DSC_type_specifier:
return AllowDefiningTypeSpec::NoButErrorRecovery;

case DeclSpecContext::DSC_trailing:
return AllowDefiningTypeSpec::No;
}
llvm_unreachable("Missing DeclSpecContext case");
}

/// Is this a context in which an opaque-enum-declaration can appear?
static bool isOpaqueEnumDeclarationContext(DeclSpecContext DSC) {
switch (DSC) {
case DeclSpecContext::DSC_normal:
case DeclSpecContext::DSC_class:
case DeclSpecContext::DSC_top_level:
return true;

case DeclSpecContext::DSC_alias_declaration:
case DeclSpecContext::DSC_objc_method_result:
case DeclSpecContext::DSC_condition:
case DeclSpecContext::DSC_template_param:
case DeclSpecContext::DSC_template_type_arg:
case DeclSpecContext::DSC_type_specifier:
case DeclSpecContext::DSC_trailing:
return false;
}
llvm_unreachable("Missing DeclSpecContext case");
}

/// Is this a context in which we can perform class template argument
/// deduction?
static bool isClassTemplateDeductionContext(DeclSpecContext DSC) {
Expand Down Expand Up @@ -2408,17 +2470,14 @@ class Parser : public CodeCompletionHandler {
True, False, Ambiguous, Error
};

/// Based only on the given token kind, determine whether we know that
/// we're at the start of an expression or a type-specifier-seq (which may
/// be an expression, in C++).
/// Determine whether we could have an enum-base.
///
/// This routine does not attempt to resolve any of the trick cases, e.g.,
/// those involving lookup of identifiers.
/// \p AllowSemi If \c true, then allow a ';' after the enum-base; otherwise
/// only consider this to be an enum-base if the next token is a '{'.
///
/// \returns \c TPR_true if this token starts an expression, \c TPR_false if
/// this token starts a type-specifier-seq, or \c TPR_ambiguous if it cannot
/// tell.
TPResult isExpressionOrTypeSpecifierSimple(tok::TokenKind Kind);
/// \return \c false if this cannot possibly be an enum base; \c true
/// otherwise.
bool isEnumBase(bool AllowSemi);

/// isCXXDeclarationSpecifier - Returns TPResult::True if it is a
/// declaration specifier, TPResult::False if it is not,
Expand Down
142 changes: 70 additions & 72 deletions clang/lib/Parse/ParseDecl.cpp
View file
Expand Up @@ -4443,14 +4443,20 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
TemplateInfo.Kind == ParsedTemplateInfo::ExplicitSpecialization);
SuppressAccessChecks diagsFromTag(*this, shouldDelayDiagsInTag);

// Enum definitions should not be parsed in a trailing-return-type.
bool AllowDeclaration = DSC != DeclSpecContext::DSC_trailing;
// Determine whether this declaration is permitted to have an enum-base.
AllowDefiningTypeSpec AllowEnumSpecifier =
isDefiningTypeSpecifierContext(DSC);
bool CanBeOpaqueEnumDeclaration =
DS.isEmpty() && isOpaqueEnumDeclarationContext(DSC);
bool CanHaveEnumBase = (getLangOpts().CPlusPlus11 || getLangOpts().ObjC ||
getLangOpts().MicrosoftExt) &&
(AllowEnumSpecifier == AllowDefiningTypeSpec::Yes ||
CanBeOpaqueEnumDeclaration);

CXXScopeSpec &SS = DS.getTypeSpecScope();
if (getLangOpts().CPlusPlus) {
// "enum foo : bar;" is not a potential typo for "enum foo::bar;"
// if a fixed underlying type is allowed.
ColonProtectionRAIIObject X(*this, AllowDeclaration);
// "enum foo : bar;" is not a potential typo for "enum foo::bar;".
ColonProtectionRAIIObject X(*this);

CXXScopeSpec Spec;
if (ParseOptionalCXXScopeSpecifier(Spec, /*ObjectType=*/nullptr,
Expand All @@ -4471,9 +4477,9 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
SS = Spec;
}

// Must have either 'enum name' or 'enum {...}'.
// Must have either 'enum name' or 'enum {...}' or (rarely) 'enum : T { ... }'.
if (Tok.isNot(tok::identifier) && Tok.isNot(tok::l_brace) &&
!(AllowDeclaration && Tok.is(tok::colon))) {
Tok.isNot(tok::colon)) {
Diag(Tok, diag::err_expected_either) << tok::identifier << tok::l_brace;

// Skip the rest of this declarator, up until the comma or semicolon.
Expand Down Expand Up @@ -4503,78 +4509,61 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
diagsFromTag.done();

TypeResult BaseType;
SourceRange BaseRange;

// Parse the fixed underlying type.
bool CanBeBitfield = getCurScope()->getFlags() & Scope::ClassScope;
if (AllowDeclaration && Tok.is(tok::colon)) {
bool PossibleBitfield = false;
if (CanBeBitfield) {
// If we're in class scope, this can either be an enum declaration with
// an underlying type, or a declaration of a bitfield member. We try to
// use a simple disambiguation scheme first to catch the common cases
// (integer literal, sizeof); if it's still ambiguous, we then consider
// anything that's a simple-type-specifier followed by '(' as an
// expression. This suffices because function types are not valid
// underlying types anyway.
EnterExpressionEvaluationContext Unevaluated(
Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
TPResult TPR = isExpressionOrTypeSpecifierSimple(NextToken().getKind());
// If the next token starts an expression, we know we're parsing a
// bit-field. This is the common case.
if (TPR == TPResult::True)
PossibleBitfield = true;
// If the next token starts a type-specifier-seq, it may be either a
// a fixed underlying type or the start of a function-style cast in C++;
// lookahead one more token to see if it's obvious that we have a
// fixed underlying type.
else if (TPR == TPResult::False &&
GetLookAheadToken(2).getKind() == tok::semi) {
// Consume the ':'.
ConsumeToken();
} else {
// We have the start of a type-specifier-seq, so we have to perform
// tentative parsing to determine whether we have an expression or a
// type.
TentativeParsingAction TPA(*this);

// Consume the ':'.
ConsumeToken();

// If we see a type specifier followed by an open-brace, we have an
// ambiguity between an underlying type and a C++11 braced
// function-style cast. Resolve this by always treating it as an
// underlying type.
// FIXME: The standard is not entirely clear on how to disambiguate in
// this case.
if ((getLangOpts().CPlusPlus &&
isCXXDeclarationSpecifier(TPResult::True) != TPResult::True) ||
(!getLangOpts().CPlusPlus && !isDeclarationSpecifier(true))) {
// We'll parse this as a bitfield later.
PossibleBitfield = true;
TPA.Revert();
} else {
// We have a type-specifier-seq.
TPA.Commit();
}
}
} else {
// Consume the ':'.
ConsumeToken();
}

if (!PossibleBitfield) {
SourceRange Range;
BaseType = ParseTypeName(&Range);
// Parse the fixed underlying type.
if (Tok.is(tok::colon)) {
// This might be an enum-base or part of some unrelated enclosing context.
//
// 'enum E : base' is permitted in two circumstances:
//
// 1) As a defining-type-specifier, when followed by '{'.
// 2) As the sole constituent of a complete declaration -- when DS is empty
// and the next token is ';'.
//
// The restriction to defining-type-specifiers is important to allow parsing
// a ? new enum E : int{}
// _Generic(a, enum E : int{})
// properly.
//
// One additional consideration applies:
//
// C++ [dcl.enum]p1:
// A ':' following "enum nested-name-specifier[opt] identifier" within
// the decl-specifier-seq of a member-declaration is parsed as part of
// an enum-base.
//
// Other lamguage modes supporting enumerations with fixed underlying types
// do not have clear rules on this, so we disambiguate to determine whether
// the tokens form a bit-field width or an enum-base.

if (CanBeBitfield && !isEnumBase(CanBeOpaqueEnumDeclaration)) {
// Outside C++11, do not interpret the tokens as an enum-base if they do
// not make sense as one. In C++11, it's an error if this happens.
if (getLangOpts().CPlusPlus11 && !getLangOpts().ObjC &&
!getLangOpts().MicrosoftExt)
Diag(Tok.getLocation(), diag::err_anonymous_enum_bitfield);
} else if (CanHaveEnumBase || !ColonIsSacred) {
SourceLocation ColonLoc = ConsumeToken();

BaseType = ParseTypeName(&BaseRange);
BaseRange.setBegin(ColonLoc);

if (!getLangOpts().ObjC) {
if (getLangOpts().CPlusPlus11)
Diag(StartLoc, diag::warn_cxx98_compat_enum_fixed_underlying_type);
Diag(ColonLoc, diag::warn_cxx98_compat_enum_fixed_underlying_type)
<< BaseRange;
else if (getLangOpts().CPlusPlus)
Diag(StartLoc, diag::ext_cxx11_enum_fixed_underlying_type);
Diag(ColonLoc, diag::ext_cxx11_enum_fixed_underlying_type)
<< BaseRange;
else if (getLangOpts().MicrosoftExt)
Diag(StartLoc, diag::ext_ms_c_enum_fixed_underlying_type);
Diag(ColonLoc, diag::ext_ms_c_enum_fixed_underlying_type)
<< BaseRange;
else
Diag(StartLoc, diag::ext_clang_c_enum_fixed_underlying_type);
Diag(ColonLoc, diag::ext_clang_c_enum_fixed_underlying_type)
<< BaseRange;
}
}
}
Expand All @@ -4590,9 +4579,9 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
// enum foo {..}; void bar() { enum foo x; } <- use of old foo.
//
Sema::TagUseKind TUK;
if (!AllowDeclaration) {
if (AllowEnumSpecifier == AllowDefiningTypeSpec::No)
TUK = Sema::TUK_Reference;
} else if (Tok.is(tok::l_brace)) {
else if (Tok.is(tok::l_brace)) {
if (DS.isFriendSpecified()) {
Diag(Tok.getLocation(), diag::err_friend_decl_defines_type)
<< SourceRange(DS.getFriendSpecLoc());
Expand Down Expand Up @@ -4623,6 +4612,15 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
diagsFromTag.redelay();
}

// A C++11 enum-base can only appear as part of an enum definition or an
// opaque-enum-declaration. MSVC and ObjC permit an enum-base anywhere.
if (BaseType.isUsable() && TUK != Sema::TUK_Definition &&
!getLangOpts().ObjC && !getLangOpts().MicrosoftExt &&
!(CanBeOpaqueEnumDeclaration && Tok.is(tok::semi))) {
Diag(BaseRange.getBegin(), diag::ext_enum_base_in_type_specifier)
<< (AllowEnumSpecifier == AllowDefiningTypeSpec::Yes) << BaseRange;
}

MultiTemplateParamsArg TParams;
if (TemplateInfo.Kind != ParsedTemplateInfo::NonTemplate &&
TUK != Sema::TUK_Reference) {
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/Parse/ParseDeclCXX.cpp
View file
Expand Up @@ -1283,7 +1283,8 @@ bool Parser::isValidAfterTypeSpecifier(bool CouldBeBitfield) {
case tok::annot_pragma_ms_pointers_to_members:
return true;
case tok::colon:
return CouldBeBitfield; // enum E { ... } : 2;
return CouldBeBitfield || // enum E { ... } : 2;
ColonIsSacred; // _Generic(..., enum E : 2);
// Microsoft compatibility
case tok::kw___cdecl: // struct foo {...} __cdecl x;
case tok::kw___fastcall: // struct foo {...} __fastcall x;
Expand Down Expand Up @@ -1680,7 +1681,7 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,

const PrintingPolicy &Policy = Actions.getASTContext().getPrintingPolicy();
Sema::TagUseKind TUK;
if (DSC == DeclSpecContext::DSC_trailing)
if (isDefiningTypeSpecifierContext(DSC) == AllowDefiningTypeSpec::No)
TUK = Sema::TUK_Reference;
else if (Tok.is(tok::l_brace) ||
(getLangOpts().CPlusPlus && Tok.is(tok::colon)) ||
Expand Down

0 comments on commit c90e198

Please sign in to comment.