diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index ece6013f67262..00ddf0b9656a3 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -161,6 +161,9 @@ Improvements to Clang's time-trace Bug Fixes in This Version ------------------------- +- Fixed missing warnings when comparing mismatched enumeration constants + in C (`#29217 `). + - Clang now accepts elaborated-type-specifiers that explicitly specialize a member class template for an implicit instantiation of a class template. diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index 3fc481a62a78a..bf0622bdeca30 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -153,6 +153,12 @@ class Expr : public ValueStmt { TR = t; } + /// If this expression is an enumeration constant, return the + /// enumeration type under which said constant was declared. + /// Otherwise return the expression's type. + /// Note this effectively circumvents the weak typing of C's enum constants + QualType getEnumCoercedType(const ASTContext &Ctx) const; + ExprDependence getDependence() const { return static_cast(ExprBits.Dependent); } @@ -471,6 +477,13 @@ class Expr : public ValueStmt { /// bit-fields, but it will return null for a conditional bit-field. FieldDecl *getSourceBitField(); + /// If this expression refers to an enum constant, retrieve its declaration + EnumConstantDecl *getEnumConstantDecl(); + + const EnumConstantDecl *getEnumConstantDecl() const { + return const_cast(this)->getEnumConstantDecl(); + } + const FieldDecl *getSourceBitField() const { return const_cast(this)->getSourceBitField(); } diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 8b10e28958326..ee9515e218cba 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -263,6 +263,14 @@ namespace { } } +QualType Expr::getEnumCoercedType(const ASTContext &Ctx) const { + if (isa(this->getType())) + return this->getType(); + else if (const auto *ECD = this->getEnumConstantDecl()) + return Ctx.getTypeDeclType(cast(ECD->getDeclContext())); + return this->getType(); +} + SourceLocation Expr::getExprLoc() const { switch (getStmtClass()) { case Stmt::NoStmtClass: llvm_unreachable("statement without class"); @@ -4097,6 +4105,13 @@ FieldDecl *Expr::getSourceBitField() { return nullptr; } +EnumConstantDecl *Expr::getEnumConstantDecl() { + Expr *E = this->IgnoreParenImpCasts(); + if (auto *DRE = dyn_cast(E)) + return dyn_cast(DRE->getDecl()); + return nullptr; +} + bool Expr::refersToVectorElement() const { // FIXME: Why do we not just look at the ObjectKind here? const Expr *E = this->IgnoreParens(); diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 71e6e7230fc45..1a869e16c987f 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -16050,15 +16050,8 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T, // Diagnose conversions between different enumeration types. // In C, we pretend that the type of an EnumConstantDecl is its enumeration // type, to give us better diagnostics. - QualType SourceType = E->getType(); - if (!S.getLangOpts().CPlusPlus) { - if (DeclRefExpr *DRE = dyn_cast(E)) - if (EnumConstantDecl *ECD = dyn_cast(DRE->getDecl())) { - EnumDecl *Enum = cast(ECD->getDeclContext()); - SourceType = S.Context.getTypeDeclType(Enum); - Source = S.Context.getCanonicalType(SourceType).getTypePtr(); - } - } + QualType SourceType = E->getEnumCoercedType(S.Context); + Source = S.Context.getCanonicalType(SourceType).getTypePtr(); if (const EnumType *SourceEnum = Source->getAs()) if (const EnumType *TargetEnum = Target->getAs()) diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 4049ab3bf6caf..2b6accfd4a4d5 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -1497,7 +1497,8 @@ static void checkEnumArithmeticConversions(Sema &S, Expr *LHS, Expr *RHS, // // Warn on this in all language modes. Produce a deprecation warning in C++20. // Eventually we will presumably reject these cases (in C++23 onwards?). - QualType L = LHS->getType(), R = RHS->getType(); + QualType L = LHS->getEnumCoercedType(S.Context), + R = RHS->getEnumCoercedType(S.Context); bool LEnum = L->isUnscopedEnumerationType(), REnum = R->isUnscopedEnumerationType(); bool IsCompAssign = ACK == Sema::ACK_CompAssign; diff --git a/clang/test/Sema/builtins-elementwise-math.c b/clang/test/Sema/builtins-elementwise-math.c index e7b36285fa7dc..2e05337273ee4 100644 --- a/clang/test/Sema/builtins-elementwise-math.c +++ b/clang/test/Sema/builtins-elementwise-math.c @@ -76,6 +76,7 @@ void test_builtin_elementwise_add_sat(int i, short s, double d, float4 v, int3 i enum f { three }; enum f x = __builtin_elementwise_add_sat(one, three); + // expected-warning@-1 {{comparison of different enumeration types ('enum e' and 'enum f')}} _BitInt(32) ext; // expected-warning {{'_BitInt' in C17 and earlier is a Clang extension}} ext = __builtin_elementwise_add_sat(ext, ext); @@ -134,6 +135,7 @@ void test_builtin_elementwise_sub_sat(int i, short s, double d, float4 v, int3 i enum f { three }; enum f x = __builtin_elementwise_sub_sat(one, three); + // expected-warning@-1 {{comparison of different enumeration types ('enum e' and 'enum f')}} _BitInt(32) ext; // expected-warning {{'_BitInt' in C17 and earlier is a Clang extension}} ext = __builtin_elementwise_sub_sat(ext, ext); @@ -189,6 +191,7 @@ void test_builtin_elementwise_max(int i, short s, double d, float4 v, int3 iv, u enum f { three }; enum f x = __builtin_elementwise_max(one, three); + // expected-warning@-1 {{comparison of different enumeration types ('enum e' and 'enum f')}} _BitInt(32) ext; // expected-warning {{'_BitInt' in C17 and earlier is a Clang extension}} ext = __builtin_elementwise_max(ext, ext); @@ -244,6 +247,7 @@ void test_builtin_elementwise_min(int i, short s, double d, float4 v, int3 iv, u enum f { three }; enum f x = __builtin_elementwise_min(one, three); + // expected-warning@-1 {{comparison of different enumeration types ('enum e' and 'enum f')}} _BitInt(32) ext; // expected-warning {{'_BitInt' in C17 and earlier is a Clang extension}} ext = __builtin_elementwise_min(ext, ext); diff --git a/clang/test/Sema/warn-compare-enum-types-mismatch.c b/clang/test/Sema/warn-compare-enum-types-mismatch.c new file mode 100644 index 0000000000000..2b72aae16b977 --- /dev/null +++ b/clang/test/Sema/warn-compare-enum-types-mismatch.c @@ -0,0 +1,42 @@ +// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wenum-compare -Wno-unused-comparison %s +// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wenum-compare -Wno-unused-comparison %s + +typedef enum EnumA { + A +} EnumA; + +enum EnumB { + B +}; + +enum { + C +}; + +void foo(void) { + enum EnumA a = A; + enum EnumB b = B; + A == B; + // expected-warning@-1 {{comparison of different enumeration types}} + a == (B); + // expected-warning@-1 {{comparison of different enumeration types}} + a == b; + // expected-warning@-1 {{comparison of different enumeration types}} + A > B; + // expected-warning@-1 {{comparison of different enumeration types}} + A >= b; + // expected-warning@-1 {{comparison of different enumeration types}} + a > b; + // expected-warning@-1 {{comparison of different enumeration types}} + (A) <= ((B)); + // expected-warning@-1 {{comparison of different enumeration types}} + a < B; + // expected-warning@-1 {{comparison of different enumeration types}} + a < b; + // expected-warning@-1 {{comparison of different enumeration types}} + + // In the following cases we purposefully differ from GCC and dont warn + a == C; + A < C; + b >= C; +} diff --git a/clang/test/Sema/warn-conditional-emum-types-mismatch.c b/clang/test/Sema/warn-conditional-enum-types-mismatch.c similarity index 88% rename from clang/test/Sema/warn-conditional-emum-types-mismatch.c rename to clang/test/Sema/warn-conditional-enum-types-mismatch.c index c9e2eddc7764b..f039245b6fabe 100644 --- a/clang/test/Sema/warn-conditional-emum-types-mismatch.c +++ b/clang/test/Sema/warn-conditional-enum-types-mismatch.c @@ -21,7 +21,7 @@ int get_flag(int cond) { #ifdef __cplusplus // expected-warning@-2 {{conditional expression between different enumeration types ('ro' and 'rw')}} #else - // expected-no-diagnostics + // expected-warning@-4 {{conditional expression between different enumeration types ('enum ro' and 'enum rw')}} #endif } diff --git a/clang/test/Sema/warn-overlap.c b/clang/test/Sema/warn-overlap.c index 1eddfd1077fd9..2db07ebcd17b8 100644 --- a/clang/test/Sema/warn-overlap.c +++ b/clang/test/Sema/warn-overlap.c @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-overlap-compare %s -// RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused -Wno-loop-analysis %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-overlap-compare -Wno-enum-compare %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused -Wno-loop-analysis -Wno-enum-compare %s #define mydefine 2