Index: include/clang/AST/Decl.h =================================================================== --- include/clang/AST/Decl.h +++ include/clang/AST/Decl.h @@ -3449,7 +3449,9 @@ /// \brief Returns true if this can be considered a complete type. bool isComplete() const { - return isCompleteDefinition() || isFixed(); + // IntegerType is set for fixed type enums and non-fixed but implicitly + // int-sized Microsoft enums. + return isCompleteDefinition() || IntegerType; } /// Returns true if this enum is either annotated with Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -2315,8 +2315,7 @@ Expr *val); bool CheckEnumUnderlyingType(TypeSourceInfo *TI); bool CheckEnumRedeclaration(SourceLocation EnumLoc, bool IsScoped, - QualType EnumUnderlyingTy, - bool EnumUnderlyingIsImplicit, + QualType EnumUnderlyingTy, bool IsFixed, const EnumDecl *Prev); /// Determine whether the body of an anonymous enumeration should be skipped. Index: lib/AST/Type.cpp =================================================================== --- lib/AST/Type.cpp +++ lib/AST/Type.cpp @@ -1997,12 +1997,7 @@ EnumDecl *EnumD = cast(CanonicalType)->getDecl(); if (Def) *Def = EnumD; - - // An enumeration with fixed underlying type is complete (C++0x 7.2p3). - if (EnumD->isFixed()) - return false; - - return !EnumD->isCompleteDefinition(); + return !EnumD->isComplete(); } case Record: { // A tagged type (struct/union/enum/class) is incomplete if the decl is a Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -13236,11 +13236,9 @@ /// Check whether this is a valid redeclaration of a previous enumeration. /// \return true if the redeclaration was invalid. -bool Sema::CheckEnumRedeclaration( - SourceLocation EnumLoc, bool IsScoped, QualType EnumUnderlyingTy, - bool EnumUnderlyingIsImplicit, const EnumDecl *Prev) { - bool IsFixed = !EnumUnderlyingTy.isNull(); - +bool Sema::CheckEnumRedeclaration(SourceLocation EnumLoc, bool IsScoped, + QualType EnumUnderlyingTy, bool IsFixed, + const EnumDecl *Prev) { if (IsScoped != Prev->isScoped()) { Diag(EnumLoc, diag::err_enum_redeclare_scoped_mismatch) << Prev->isScoped(); @@ -13260,10 +13258,6 @@ << Prev->getIntegerTypeRange(); return true; } - } else if (IsFixed && !Prev->isFixed() && EnumUnderlyingIsImplicit) { - ; - } else if (!IsFixed && Prev->isFixed() && !Prev->getIntegerTypeSourceInfo()) { - ; } else if (IsFixed != Prev->isFixed()) { Diag(EnumLoc, diag::err_enum_redeclare_fixed_mismatch) << Prev->isFixed(); @@ -13559,14 +13553,14 @@ // this early, because it's needed to detect if this is an incompatible // redeclaration. llvm::PointerUnion EnumUnderlying; - bool EnumUnderlyingIsImplicit = false; + bool IsFixed = !UnderlyingType.isUnset() || ScopedEnum; if (Kind == TTK_Enum) { - if (UnderlyingType.isInvalid() || (!UnderlyingType.get() && ScopedEnum)) + if (UnderlyingType.isInvalid() || (!UnderlyingType.get() && ScopedEnum)) { // No underlying type explicitly specified, or we failed to parse the // type, default to int. EnumUnderlying = Context.IntTy.getTypePtr(); - else if (UnderlyingType.get()) { + } else if (UnderlyingType.get()) { // C++0x 7.2p2: The type-specifier-seq of an enum-base shall name an // integral type; any cv-qualification is ignored. TypeSourceInfo *TI = nullptr; @@ -13582,11 +13576,12 @@ EnumUnderlying = Context.IntTy.getTypePtr(); } else if (Context.getTargetInfo().getCXXABI().isMicrosoft()) { - if (getLangOpts().MSVCCompat || TUK == TUK_Definition) { - // Microsoft enums are always of int type. + // For MSVC ABI compatibility, unfixed enums must use an underlying type + // of 'int'. However, if this is an unfixed forward declaration, don't set + // the underlying type unless the user enables -fms-compatibility. This + // makes unfixed forward declared enums incomplete and is more conforming. + if (TUK == TUK_Definition || getLangOpts().MSVCCompat) EnumUnderlying = Context.IntTy.getTypePtr(); - EnumUnderlyingIsImplicit = true; - } } } @@ -13612,8 +13607,7 @@ if (Kind == TTK_Enum) { New = EnumDecl::Create(Context, SearchDC, KWLoc, Loc, Name, nullptr, - ScopedEnum, ScopedEnumUsesClassTag, - !EnumUnderlying.isNull()); + ScopedEnum, ScopedEnumUsesClassTag, IsFixed); // If this is an undefined enum, bail. if (TUK != TUK_Definition && !Invalid) return nullptr; @@ -13992,7 +13986,7 @@ // in which case we want the caller to bail out. if (CheckEnumRedeclaration(NameLoc.isValid() ? NameLoc : KWLoc, ScopedEnum, EnumUnderlyingTy, - EnumUnderlyingIsImplicit, PrevEnum)) + IsFixed, PrevEnum)) return TUK == TUK_Declaration ? PrevTagDecl : nullptr; } @@ -14208,7 +14202,7 @@ // enum X { A, B, C } D; D should chain to X. New = EnumDecl::Create(Context, SearchDC, KWLoc, Loc, Name, cast_or_null(PrevDecl), ScopedEnum, - ScopedEnumUsesClassTag, !EnumUnderlying.isNull()); + ScopedEnumUsesClassTag, IsFixed); if (isStdAlignValT && (!StdAlignValT || getStdAlignValT()->isImplicit())) StdAlignValT = cast(New); @@ -14216,8 +14210,7 @@ // If this is an undefined enum, warn. if (TUK != TUK_Definition && !Invalid) { TagDecl *Def; - if (!EnumUnderlyingIsImplicit && - (getLangOpts().CPlusPlus11 || getLangOpts().ObjC2) && + if (IsFixed && (getLangOpts().CPlusPlus11 || getLangOpts().ObjC2) && cast(New)->isFixed()) { // C++0x: 7.2p2: opaque-enum-declaration. // Conflicts are diagnosed above. Do nothing. @@ -14249,6 +14242,7 @@ else ED->setIntegerType(QualType(EnumUnderlying.get(), 0)); ED->setPromotionType(ED->getIntegerType()); + assert(ED->isComplete() && "enum with type should be complete"); } } else { // struct/union/class @@ -15707,7 +15701,7 @@ &EnumVal).get())) { // C99 6.7.2.2p2: Make sure we have an integer constant expression. } else { - if (Enum->isFixed()) { + if (Enum->isComplete()) { EltTy = Enum->getIntegerType(); // In Obj-C and Microsoft mode, require the enumeration value to be @@ -16218,7 +16212,9 @@ if (LangOpts.ShortEnums) Packed = true; - if (Enum->isFixed()) { + // If the enum already has a type because it is fixed or dictated by the + // target, promote that type instead of analyzing the enumerators. + if (Enum->isComplete()) { BestType = Enum->getIntegerType(); if (BestType->isPromotableIntegerType()) BestPromotionType = Context.getPromotedIntegerType(BestType); Index: lib/Sema/SemaTemplateInstantiateDecl.cpp =================================================================== --- lib/Sema/SemaTemplateInstantiateDecl.cpp +++ lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -1041,8 +1041,7 @@ SemaRef.SubstType(TI->getType(), TemplateArgs, UnderlyingLoc, DeclarationName()); SemaRef.CheckEnumRedeclaration(Def->getLocation(), Def->isScoped(), - DefnUnderlying, - /*EnumUnderlyingIsImplicit=*/false, Enum); + DefnUnderlying, /*IsFixed=*/true, Enum); } } Index: test/Sema/sign-compare-enum.c =================================================================== --- test/Sema/sign-compare-enum.c +++ test/Sema/sign-compare-enum.c @@ -1,24 +1,77 @@ -// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -verify -Wsign-compare %s -// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify -Wsign-compare %s -// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -DSILENCE -verify %s -// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -DSILENCE -verify %s - -int main() { - enum A { A_a = 0, A_b = 1 }; - static const int message[] = {0, 1}; - enum A a; +// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -verify -Wsign-compare %s +// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -verify -Wsign-compare %s +// RUN: %clang_cc1 -x c++ -triple=x86_64-pc-linux-gnu -fsyntax-only -verify -Wsign-compare %s +// RUN: %clang_cc1 -x c++ -triple=x86_64-pc-win32 -fsyntax-only -verify -Wsign-compare %s +// Check that -Wsign-compare is off by default. +// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -verify -DSILENCE %s + +#ifdef SILENCE +// expected-no-diagnostics +#endif + +enum PosEnum { + A_a = 0, + A_b = 1, + A_c = 10 +}; + +static const int message[] = {0, 1}; + +int test_pos(enum PosEnum a) { if (a < 2) return 0; -#if defined(SIGNED) && !defined(SILENCE) - if (a < sizeof(message)/sizeof(message[0])) // expected-warning {{comparison of integers of different signs: 'enum A' and 'unsigned long long'}} - return 0; -#else - // expected-no-diagnostics + // No warning, except in Windows C mode, where PosEnum is 'int' and it can + // take on any value according to the C standard. +#if !defined(SILENCE) && defined(_WIN32) && !defined(__cplusplus) + // expected-warning@+2 {{comparison of integers of different signs}} +#endif if (a < 2U) return 0; + + unsigned uv = 2; +#if !defined(SILENCE) && defined(_WIN32) && !defined(__cplusplus) + // expected-warning@+2 {{comparison of integers of different signs}} +#endif + if (a < uv) + return 1; + +#if !defined(SILENCE) && defined(_WIN32) && !defined(__cplusplus) + // expected-warning@+2 {{comparison of integers of different signs}} +#endif if (a < sizeof(message)/sizeof(message[0])) return 0; + return 4; +} + +enum NegEnum { + NE_a = -1, + NE_b = 0, + NE_c = 10 +}; + +int test_neg(enum NegEnum a) { + if (a < 2) + return 0; + +#ifndef SILENCE + // expected-warning@+2 {{comparison of integers of different signs}} +#endif + if (a < 2U) + return 0; + + unsigned uv = 2; +#ifndef SILENCE + // expected-warning@+2 {{comparison of integers of different signs}} +#endif + if (a < uv) + return 1; + +#ifndef SILENCE + // expected-warning@+2 {{comparison of integers of different signs}} #endif + if (a < sizeof(message)/sizeof(message[0])) + return 0; + return 4; }