[cfe-commits] [PATCH] C++0x strongly typed enums
Douglas Gregor
dgregor at apple.com
Fri Oct 1 08:15:50 PDT 2010
Hi Daniel,
On Sep 30, 2010, at 1:05 PM, Daniel Wallin wrote:
> Attached is a patch that implements partial support for strongly typed
> enums. It lacks support for opaque enums, and there are at least a few
> cases where a scoped enum is accepted where it shouldn't be. Comments
> welcome!
diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h
index 1b1b1df..fcc6220 100644
--- a/include/clang/AST/Decl.h
+++ b/include/clang/AST/Decl.h
@@ -1823,6 +1823,14 @@ protected:
unsigned NumPositiveBits : 8;
unsigned NumNegativeBits : 8;
+ /// IsScoped - True if this is tag declaration is a scoped enumeration. Only
+ /// possible in C++0x mode.
+ bool IsScoped : 1;
+
+ /// IsFixed - True if this is an enumeration with fixed underlying type. Only
+ /// possible in C++0x mode.
+ bool IsFixed : 1;
It occurs to me that what we want for Microsoft-style forward enum declarations is probably that they have a fixed underlying type of "int". That doesn't affect your patch, though.
@@ -2119,6 +2131,23 @@ public:
NumNegativeBits = Num;
}
+ /// \brief Returns true if this is a C++0x scoped enumeration.
+ bool isScoped() const {
+ return IsScoped;
+ }
+ void setScoped(bool Scoped) {
+ IsScoped = Scoped;
+ }
+
+ /// \brief Returns true if this is a C++0x enumeration with fixed underlying
+ /// type.
+ bool isFixed() const {
+ return IsFixed;
+ }
+ void setFixed(bool Fixed) {
+ IsFixed = Fixed;
+ }
+
I don't think we actually need the setScoped/setFixed mutators. The only client is the ASTReader, which should just be granted friendship.
--- a/include/clang/Basic/DiagnosticParseKinds.td
+++ b/include/clang/Basic/DiagnosticParseKinds.td
@@ -378,6 +378,9 @@ def err_friend_decl_defines_class : Error<
def warn_deleted_function_accepted_as_extension: ExtWarn<
"deleted function definition accepted as a C++0x extension">, InGroup<CXX0x>;
+def err_scoped_enum_missing_identifier : Error<
+ "scoped enumeration requires an identifier">;
Nit-pick alert: "scoped enumeration requires a name" would feel a little less pedantic and more friendly, to me.
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -820,6 +820,10 @@ def err_final_function_overridden : Error<
"declaration of %0 overrides a 'final' function">;
def err_final_base : Error<
"derivation from 'final' %0">;
+
+// C++0x scoped enumerations
+def err_enum_invalid_underlying : Error<
+ "%0 is an invalid underlying type">;
I'd prefer that this diagnostic said specifically what's wrong with the type: either it's not an integral type (the common case), or it's a wide-character type. Something like:
+def err_enum_invalid_underlying : Error<
+ "%select{non-integral|wide character}0 type %1 is an invalid underlying type">;
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -785,7 +785,8 @@ public:
IdentifierInfo *Name, SourceLocation NameLoc,
AttributeList *Attr, AccessSpecifier AS,
MultiTemplateParamsArg TemplateParameterLists,
- bool &OwnedDecl, bool &IsDependent);
+ bool &OwnedDecl, bool &IsDependent, bool ScopedEnum,
+ ParsedType BaseType, SourceLocation BaseTyLoc);
We shouldn't actually need BaseTyLoc, since the BaseType will contain a source range covering the entire type. It *might* be handy to have the location of the ':', however.
diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp
index 8e6aa23..43ce7ee 100644
--- a/lib/AST/Type.cpp
+++ b/lib/AST/Type.cpp
@@ -470,6 +470,20 @@ bool Type::isIntegralOrEnumerationType() const {
return false;
}
+bool Type::isIntegralOrUnscopedEnumerationType() const {
+ if (const BuiltinType *BT = dyn_cast<BuiltinType>(CanonicalType))
+ return BT->getKind() >= BuiltinType::Bool &&
+ BT->getKind() <= BuiltinType::Int128;
+
+ // Check for a complete enum type; incomplete enum types are not properly an
+ // enumeration type in the sense required here.
+ if (const TagType *TT = dyn_cast<TagType>(CanonicalType))
+ if (const EnumDecl *ED = dyn_cast<EnumDecl>(TT->getDecl()))
+ return ED->isDefinition() && !ED->isScoped();
+
+ return false;
+}
Perhaps it isn't modeled yet, but isn't a forward-declared enumeration type in C++0x (which has a fixed underlying type) considered complete?
@@ -591,7 +605,12 @@ bool Type::isArithmeticType() const {
if (const EnumType *ET = dyn_cast<EnumType>(CanonicalType))
// GCC allows forward declaration of enum types (forbid by C99 6.7.2.3p2).
// If a body isn't seen by the time we get here, return false.
- return ET->getDecl()->isDefinition();
+ //
+ // C++0x: Enumerations are not arithmetic types. For now, just return
+ // false for scoped enumerations since that will disable any
+ // unwanted implicit conversions.
+ return !cast<EnumDecl>(ET->getDecl())->isScoped() &&
+ ET->getDecl()->isDefinition();
return isa<ComplexType>(CanonicalType);
}
The cast to EnumDecl here is redundant.
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index 0a48d4d..3e9d4f5 100644
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -1991,6 +1991,14 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
}
}
+ bool IsScopedEnum = false;
+
+ if (getLang().CPlusPlus0x && (Tok.is(tok::kw_class)
+ || Tok.is(tok::kw_struct))) {
+ ConsumeToken();
+ IsScopedEnum = true;
+ }
+
Could you update the grammar in the comment for ParseEnumSpecifier?
+ if (!Name && IsScopedEnum) {
+ // C++0x 7.2p2: The optional identifier shall not be omitted in the
+ // declaration of a scoped enumeration.
+ Diag(Tok, diag::err_scoped_enum_missing_identifier);
+ }
>From a recovery standpoint, it's probably safest to set IsScopedEnum = false; in here. That way, we'll maintain the AST invariant that scoped enumerations always have names.
+ ParsedType BaseType;
+ SourceLocation BaseTyLoc;
+
+ if (getLang().CPlusPlus0x && Tok.is(tok::colon)) {
+ ConsumeToken();
+ SourceRange Range;
+ TypeResult BaseTy(ParseTypeName(&Range));
+ BaseType = BaseTy.get();
+ BaseTyLoc = Range.getBegin();
+ }
If parsing the type fails, we should probably recover by setting the underlying type to "int", rather than not having an underlying type. For one, this means that we'll be able to consider such a type as "complete" for the rest of the translation unit.
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -5757,14 +5758,43 @@ CreateNewDecl:
TagDecl *New;
if (Kind == TTK_Enum) {
+
+ QualType UnderlyingType;
+ bool IsFixed = false;
+
+ if (!BaseType && ScopedEnum) {
+ // No underlying type explicitly specified, default to int.
+ IsFixed = true;
+ UnderlyingType = Context.IntTy;
+ }
+ else if (BaseType) {
+ TypeSourceInfo *TInfo;
+ UnderlyingType = GetTypeFromParser(BaseType, &TInfo);
+ UnderlyingType = UnderlyingType.getUnqualifiedType();
+ IsFixed = true;
+
+ if (!UnderlyingType->isIntegralType(Context) ||
+ UnderlyingType->isWideCharType()) {
What about char16_t/char32_t?
@@ -6817,7 +6851,12 @@ EnumConstantDecl *Sema::CheckEnumConstant(EnumDecl *Enum,
// is the type of its initializing value:
// - If an initializer is specified for an enumerator, the
// initializing value has the same type as the expression.
- EltTy = Val->getType();
+ if (Enum->isFixed()) {
+ EltTy = Enum->getIntegerType();
+ }
+ else {
+ EltTy = Val->getType();
+ }
}
}
}
@@ -6834,7 +6873,12 @@ EnumConstantDecl *Sema::CheckEnumConstant(EnumDecl *Enum,
//
// GCC uses 'int' for its unspecified integral type, as does
// C99 6.7.2.2p3.
- EltTy = Context.IntTy;
+ if (Enum->isFixed()) {
+ EltTy = Enum->getIntegerType();
+ }
+ else {
+ EltTy = Context.IntTy;
+ }
} else {
// Assign the last value + 1.
EnumVal = LastEnumConst->getInitVal();
@@ -6854,7 +6898,7 @@ EnumConstantDecl *Sema::CheckEnumConstant(EnumDecl *Enum,
// sufficient to contain the incremented value. If no such type
// exists, the program is ill-formed.
QualType T = getNextLargerIntegralType(Context, EltTy);
- if (T.isNull()) {
+ if (T.isNull() || Enum->isFixed()) {
// There is no integral type larger enough to represent this
// value. Complain, then allow the value to wrap around.
EnumVal = LastEnumConst->getInitVal();
I don't think this is quite enough. When there is a fixed underlying type and we have a value explicitly given, we should check that the value is representable in that fixed underlying type, and convert the expression to that fixed underlying type (this is [dcl.enum]p5).
This is a very good start, thanks!
- Doug
More information about the cfe-commits
mailing list