maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[Sema] Don't mark plain MS enums as fixed 

Summary:
This fixes a flaw in our AST: PR27098

MSVC always gives plain enums the underlying type 'int'. Clang does this
as well, but we claim the enum is "fixed", as if the user actually wrote
': int'. It means we end up emitting spurious -Wsign-compare warnings on
code like this:

  enum Vals { E1, E2, E3 };
  bool f(unsigned v1, Vals v2) {
    return v1 == v2;
  }

We think 'v2' can take on negative values because we think 'Vals' is
fixed. This fixes that.

Reviewers: rsmith

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D43110

llvm-svn: 324913
This commit is contained in:
Reid Kleckner 2018-02-12 17:37:06 +00:00
parent e72d99261f
commit b0a17edff7
6 changed files with 93 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
clang/include/clang/AST/Decl.h
View File

@ -3449,7 +3449,9 @@ public:
/// \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

3
clang/include/clang/Sema/Sema.h
View File

@ -2315,8 +2315,7 @@ public:
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.

7
clang/lib/AST/Type.cpp
View File

@ -1997,12 +1997,7 @@ bool Type::isIncompleteType(NamedDecl **Def) const {
EnumDecl *EnumD = cast<EnumType>(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

44
clang/lib/Sema/SemaDecl.cpp
View File

@ -13236,11 +13236,9 @@ bool Sema::CheckEnumUnderlyingType(TypeSourceInfo *TI) {
/// 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 @@ bool Sema::CheckEnumRedeclaration(
<< 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 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
// this early, because it's needed to detect if this is an incompatible
// redeclaration.
llvm::PointerUnion<const Type*, TypeSourceInfo*> 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 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
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 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
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 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
// 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 @@ CreateNewDecl:
// enum X { A, B, C } D; D should chain to X.
New = EnumDecl::Create(Context, SearchDC, KWLoc, Loc, Name,
cast_or_null<EnumDecl>(PrevDecl), ScopedEnum,
ScopedEnumUsesClassTag, !EnumUnderlying.isNull());
ScopedEnumUsesClassTag, IsFixed);
if (isStdAlignValT && (!StdAlignValT || getStdAlignValT()->isImplicit()))
StdAlignValT = cast<EnumDecl>(New);
@ -14216,8 +14210,7 @@ CreateNewDecl:
// 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<EnumDecl>(New)->isFixed()) {
// C++0x: 7.2p2: opaque-enum-declaration.
// Conflicts are diagnosed above. Do nothing.
@ -14249,6 +14242,7 @@ CreateNewDecl:
else
ED->setIntegerType(QualType(EnumUnderlying.get<const Type*>(), 0));
ED->setPromotionType(ED->getIntegerType());
assert(ED->isComplete() && "enum with type should be complete");
}
} else {
// struct/union/class
@ -15707,7 +15701,7 @@ EnumConstantDecl *Sema::CheckEnumConstant(EnumDecl *Enum,
&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 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
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);

3
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
View File

@ -1041,8 +1041,7 @@ Decl *TemplateDeclInstantiator::VisitEnumDecl(EnumDecl *D) {
SemaRef.SubstType(TI->getType(), TemplateArgs,
UnderlyingLoc, DeclarationName());
SemaRef.CheckEnumRedeclaration(Def->getLocation(), Def->isScoped(),
DefnUnderlying,
/*EnumUnderlyingIsImplicit=*/false, Enum);
DefnUnderlying, /*IsFixed=*/true, Enum);
}
}

81
clang/test/Sema/sign-compare-enum.c
View File

@ -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
// 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
int main() {
enum A { A_a = 0, A_b = 1 };
static const int message[] = {0, 1};
enum A a;
// 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;
#endif
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;
}