[cfe-commits] [PATCH] C++0x strongly typed enums
Douglas Gregor
dgregor at apple.com
Fri Oct 8 16:55:45 PDT 2010
On Oct 8, 2010, at 5:22 AM, Daniel Wallin wrote:
> On Fri, Oct 1, 2010 at 5:15 PM, Douglas Gregor <dgregor at apple.com> wrote:
>> On Sep 30, 2010, at 1:05 PM, Daniel Wallin wrote:
>> @@ -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).
>
> Yep, I've tried to do this in the new patch. I could be missing
> something though. This also adds support for forward declaration of
> enums with fixed underlying type.
Noticed a few more things...
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index 0a48d4d..0f46b16 100644
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -2008,6 +2031,21 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
NameLoc = ConsumeToken();
}
+ 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);
+ IsScopedEnum = false;
+ }
+
+ TypeResult BaseType;
+
+ if (getLang().CPlusPlus0x && Tok.is(tok::colon)) {
+ ConsumeToken();
+ SourceRange Range;
+ BaseType = ParseTypeName(&Range);
+ }
+
If we're in the context of a class, we have to do some extra disambiguation here to determine whether the ':' is indicating that we have a bit-field of enum type or whether it's indicating a fixed underlying type. Here's a moderately crazy case:
struct X {
constexpr X() : value(10) { }
constexpr operator int() const { return value; }
int value;
};
struct Y {
enum E : X();
};
@@ -5621,6 +5650,38 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
}
}
+ if (Kind == TTK_Enum && PrevTagDecl->getTagKind() == TTK_Enum) {
+ const EnumDecl *PrevEnum = cast<EnumDecl>(PrevTagDecl);
+
+ // All conflicts with previous declarations are recovered by
+ // returning the previous declaration.
+ if (ScopedEnum != PrevEnum->isScoped()) {
+ Diag(KWLoc, diag::err_enum_redeclare_scoped_mismatch)
+ << PrevEnum->isScoped();
+ Diag(PrevTagDecl->getLocation(), diag::note_previous_use);
+ return PrevTagDecl;
+ }
+ else if (EnumUnderlying && PrevEnum->isFixed()) {
+ QualType T;
+ if (TypeSourceInfo *TI = EnumUnderlying.dyn_cast<TypeSourceInfo*>())
+ T = TI->getType();
+ else
+ T = QualType(EnumUnderlying.get<const Type*>(), 0);
+
+ if (!Context.hasSameUnqualifiedType(T, PrevEnum->getIntegerType())) {
+ Diag(KWLoc, diag::err_enum_redeclare_type_mismatch);
+ Diag(PrevTagDecl->getLocation(), diag::note_previous_use);
+ return PrevTagDecl;
+ }
+ }
+ else if (!EnumUnderlying.isNull() != PrevEnum->isFixed()) {
+ Diag(KWLoc, diag::err_enum_redeclare_fixed_mismatch)
+ << PrevEnum->isFixed();
+ Diag(PrevTagDecl->getLocation(), diag::note_previous_use);
+ return PrevTagDecl;
+ }
+ }
+
With the type-mismatch diagnostics, it is desirable to print out the type declared in both cases; they may have come from template instantiations, where the resulting type isn't always obvious.
@@ -6746,8 +6822,10 @@ static bool isRepresentableIntegerValue(ASTContext &Context,
assert(T->isIntegralType(Context) && "Integral type required!");
unsigned BitWidth = Context.getIntWidth(T);
- if (Value.isUnsigned() || Value.isNonNegative())
- return Value.getActiveBits() < BitWidth;
+ if (Value.isUnsigned() || Value.isNonNegative()) {
+ if (T->isSignedIntegerType()) --BitWidth;
+ return Value.getActiveBits() <= BitWidth;
+ }
Generally, we'd like this to be in a separate patch, because it doesn't actually have anything to do with scoped enums per se. But, I see that the test depends on it, so it's okay here.
diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp
index d876291..6be419f 100644
--- a/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -594,7 +594,26 @@ Decl *TemplateDeclInstantiator::VisitEnumDecl(EnumDecl *D) {
EnumDecl *Enum = EnumDecl::Create(SemaRef.Context, Owner,
D->getLocation(), D->getIdentifier(),
D->getTagKeywordLoc(),
- /*PrevDecl=*/0);
+ /*PrevDecl=*/0,
+ D->isScoped(), D->isFixed());
+ if (D->isFixed()) {
+ if (TypeSourceInfo* TI = D->getIntegerTypeSourceInfo()) {
+ // If we have type source information for the underlying type, it means it
+ // has been explicitly set by the user. Perform substitution on it before
+ // moving on.
+ SourceLocation UnderlyingLoc = TI->getTypeLoc().getBeginLoc();
+ Enum->setIntegerTypeSourceInfo(SemaRef.SubstType(TI,
+ TemplateArgs,
+ UnderlyingLoc,
+ DeclarationName()));
If substitution fails, we should fall back to 'int' here. I've added this code.
It'd be great if the declaration printer/dumper was updated to print the "class" in "enum class" and the fixed underlying type (when present), so that our -ast-dump and -ast-print output would be helpful.
I've gone ahead and committed a tweaked version of your patch in r116122, with the minor tweaks above. It'd be great if you could address the issues above in a separate patch (disambiguation, diagnostics, declaration printer).
Thanks!
- Doug
More information about the cfe-commits
mailing list