[clang] 8c2ae42 - [Clang][Sema] Fix missing warning when comparing mismatched enums in … (#81418)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 27 06:59:04 PST 2024
Author: Kupa-Martin
Date: 2024-02-27T06:58:59-08:00
New Revision: 8c2ae42b3e1c6aa7c18f873edcebff7c0b45a37e
URL: https://github.com/llvm/llvm-project/commit/8c2ae42b3e1c6aa7c18f873edcebff7c0b45a37e
DIFF: https://github.com/llvm/llvm-project/commit/8c2ae42b3e1c6aa7c18f873edcebff7c0b45a37e.diff
LOG: [Clang][Sema] Fix missing warning when comparing mismatched enums in … (#81418)
…C mode
Factored logic from `CheckImplicitConversion` into new methods
`Expr::getEnumConstantDecl` and `Expr::getEnumCoercedType` for use in
`checkEnumArithmeticConversions`.
Fix #29217
Added:
clang/test/Sema/warn-compare-enum-types-mismatch.c
clang/test/Sema/warn-conditional-enum-types-mismatch.c
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/Expr.h
clang/lib/AST/Expr.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/Sema/builtins-elementwise-math.c
clang/test/Sema/warn-overlap.c
Removed:
clang/test/Sema/warn-conditional-emum-types-mismatch.c
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 515dffa28df186..c60c5682dbd826 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -197,6 +197,9 @@ Improvements to Clang's time-trace
Bug Fixes in This Version
-------------------------
+- Fixed missing warnings when comparing mismatched enumeration constants
+ in C (`#29217 <https://github.com/llvm/llvm-project/issues/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 3fc481a62a78a9..bf0622bdeca30e 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<ExprDependence>(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<Expr *>(this)->getEnumConstantDecl();
+ }
+
const FieldDecl *getSourceBitField() const {
return const_cast<Expr*>(this)->getSourceBitField();
}
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index cc0407131d7931..b4de2155adcebd 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<EnumType>(this->getType()))
+ return this->getType();
+ else if (const auto *ECD = this->getEnumConstantDecl())
+ return Ctx.getTypeDeclType(cast<EnumDecl>(ECD->getDeclContext()));
+ return this->getType();
+}
+
SourceLocation Expr::getExprLoc() const {
switch (getStmtClass()) {
case Stmt::NoStmtClass: llvm_unreachable("statement without class");
@@ -4098,6 +4106,13 @@ FieldDecl *Expr::getSourceBitField() {
return nullptr;
}
+EnumConstantDecl *Expr::getEnumConstantDecl() {
+ Expr *E = this->IgnoreParenImpCasts();
+ if (auto *DRE = dyn_cast<DeclRefExpr>(E))
+ return dyn_cast<EnumConstantDecl>(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 7d8a2cf280c336..0de76ee119cf81 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -16148,15 +16148,8 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
// Diagnose conversions between
diff erent 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<DeclRefExpr>(E))
- if (EnumConstantDecl *ECD = dyn_cast<EnumConstantDecl>(DRE->getDecl())) {
- EnumDecl *Enum = cast<EnumDecl>(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<EnumType>())
if (const EnumType *TargetEnum = Target->getAs<EnumType>())
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 403839f77a2b7c..2a0e86c37f1bfc 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 e7b36285fa7dcf..2e05337273ee41 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 at -1 {{comparison of
diff erent 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 at -1 {{comparison of
diff erent 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 at -1 {{comparison of
diff erent 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 at -1 {{comparison of
diff erent 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 00000000000000..2b72aae16b977a
--- /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 at -1 {{comparison of
diff erent enumeration types}}
+ a == (B);
+ // expected-warning at -1 {{comparison of
diff erent enumeration types}}
+ a == b;
+ // expected-warning at -1 {{comparison of
diff erent enumeration types}}
+ A > B;
+ // expected-warning at -1 {{comparison of
diff erent enumeration types}}
+ A >= b;
+ // expected-warning at -1 {{comparison of
diff erent enumeration types}}
+ a > b;
+ // expected-warning at -1 {{comparison of
diff erent enumeration types}}
+ (A) <= ((B));
+ // expected-warning at -1 {{comparison of
diff erent enumeration types}}
+ a < B;
+ // expected-warning at -1 {{comparison of
diff erent enumeration types}}
+ a < b;
+ // expected-warning at -1 {{comparison of
diff erent enumeration types}}
+
+ // In the following cases we purposefully
diff er 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 c9e2eddc7764bd..f039245b6fabe5 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 at -2 {{conditional expression between
diff erent enumeration types ('ro' and 'rw')}}
#else
- // expected-no-diagnostics
+ // expected-warning at -4 {{conditional expression between
diff erent 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 1eddfd1077fd92..2db07ebcd17b87 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
More information about the cfe-commits
mailing list