https://github.com/44-2-Kupa-Martin created
https://github.com/llvm/llvm-project/pull/81418
…C mode
Factored logic from `CheckImplicitConversion` into new methods
`Expr::getEnumConstantDecl` and `Expr::getEnumCoercedType` for use in
`checkEnumArithmeticConversions`.
Fix #29217
>From a1748e5c7867a8a4dc4dbdfd5f19460733adc3f1 Mon Sep 17 00:00:00 2001
From: 44-2-Kupa-Martin <kupamartinclassr...@gmail.com>
Date: Sun, 11 Feb 2024 12:22:45 -0300
Subject: [PATCH] [Clang][Sema] Fix missing warning when comparing mismatched
enums in C mode
Factored logic from `CheckImplicitConversion` into new methods
`Expr::getEnumConstantDecl` and `Expr::getEnumCoercedType` for use in
`checkEnumArithmeticConversions`.
Fix #29217
---
clang/docs/ReleaseNotes.rst | 3 ++
clang/include/clang/AST/Expr.h | 13 ++++++
clang/lib/AST/Expr.cpp | 16 +++++++
clang/lib/Sema/SemaChecking.cpp | 11 +----
clang/lib/Sema/SemaExpr.cpp | 3 +-
clang/test/Sema/builtins-elementwise-math.c | 4 ++
.../Sema/warn-compare-enum-types-mismatch.c | 42 +++++++++++++++++++
...=> warn-conditional-enum-types-mismatch.c} | 2 +-
clang/test/Sema/warn-overlap.c | 4 +-
9 files changed, 85 insertions(+), 13 deletions(-)
create mode 100644 clang/test/Sema/warn-compare-enum-types-mismatch.c
rename clang/test/Sema/{warn-conditional-emum-types-mismatch.c =>
warn-conditional-enum-types-mismatch.c} (88%)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ece6013f672621..00ddf0b9656a31 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 <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 8b10e289583260..78a0aba5abc3f1 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 {
+ bool NotEnumType = dyn_cast<EnumType>(this->getType()) == nullptr;
+ if (NotEnumType)
+ if (const EnumConstantDecl *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");
@@ -4097,6 +4105,14 @@ FieldDecl *Expr::getSourceBitField() {
return nullptr;
}
+EnumConstantDecl *Expr::getEnumConstantDecl() {
+ Expr *E = this->IgnoreParenImpCasts();
+ if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E))
+ if (EnumConstantDecl *ECD = dyn_cast<EnumConstantDecl>(DRE->getDecl()))
+ return ECD;
+ 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 71e6e7230fc455..1a869e16c987f4 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<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 4049ab3bf6cafb..2b6accfd4a4d50 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@-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 00000000000000..1c50f35516353f
--- /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;
+}
\ No newline at end of file
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@-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 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
