[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Erich Keane via cfe-commits]

erichkeane wrote:

> @stbergmann is right. @erichkeane How should I proceed? Should I open a 
> revert PR? Or should I open a separate issue?

For now, unless someone insists, I think we're likely fine to do a follow-up PR 
to fix that.  Make sure to put the same reviewers on, and we'll see if we can 
review this quickly.



https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[via cfe-commits]

Kupa-Martin wrote:

@stbergmann is right. @erichkeane How should I proceed? Should I open a revert 
PR? Or should I open a separate issue?

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Stephan Bergmann via cfe-commits]

stbergmann wrote:

This change started to break the following C++26 code:
```
$ cat test.cc
enum E1 { E11 };
enum E2 {
E21 = E11,
E22 = 1,
E23 = E21 + E22
};
```
```
clang++ -std=c++26 -fsyntax-only test.cc
test.cc:5:15: error: invalid arithmetic between different enumeration types 
('E1' and 'E2')
5 | E23 = E21 + E22
  |   ~~~ ^ ~~~
1 error generated.
```
as within the definition of enum `E2` with unfixed underlying type, while the 
type of `E21` is indeed `E1`, the type of `E22` there is `int` rather than `E2` 
(see [dcl.enum]/5.1 "If an initializer is specified for an enumerator, the 
constant-expression shall be an integral constant expression (7.7). If the 
expression has unscoped enumeration type, the enumerator has the underlying 
type of that enumeration type, otherwise it has the same type as the 
expression.")

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Erich Keane via cfe-commits]

erichkeane wrote:

> For what it's worth, this change adds several instances of 
> `-Wenum-enum-conversion` for the Linux kernel: 
> [ClangBuiltLinux/linux#2002](https://github.com/ClangBuiltLinux/linux/issues/2002).
>  I assume this is intentional given the nature of the change as a whole but 
> neither the tests nor the release notes seem to really reflect that. In case 
> it is a bug that this change affected that, consider this a report :)

You're correct, those are intentional.  
`https://github.com/llvm/llvm-project/pull/81418/files#diff-f240d794093a226da48919a27929148597edd1460a1d38b0decbced3fc153ddc`
  specifically covers 'conversions' but arithmetic operations are also covered 
by that warning. 

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Nathan Chancellor via cfe-commits]

nathanchance wrote:

For what it's worth, this change adds several instances of 
`-Wenum-enum-conversion` for the Linux kernel: 
https://github.com/ClangBuiltLinux/linux/issues/2002. I assume this is 
intentional given the nature of the change as a whole but neither the tests nor 
the release notes seem to really reflect that. In case it is a bug that this 
change affected that, consider this a report :)

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[via cfe-commits]

github-actions[bot] wrote:



@Kupa-Martin Congratulations on having your first Pull Request (PR) merged into 
the LLVM Project!

Your changes will be combined with recent changes from other authors, then 
tested
by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with 
a build, you may recieve a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail 
[here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr).

If your change does cause a problem, it may be reverted, or you can revert it 
yourself.
This is a normal part of [LLVM 
development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy).
 You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are 
working as expected, well done!


https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Erich Keane via cfe-commits]

https://github.com/erichkeane closed 
https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.


https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[via cfe-commits]

Kupa-Martin wrote:

If everything looks good could someone please merge this pr? I dont have commit 
access. 

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[via cfe-commits]

@@ -263,6 +263,14 @@ namespace {
   }
 }
 
+QualType Expr::getEnumCoercedType(const ASTContext &Ctx) const {
+  if (isa(this->getType()))
+return this->getType();
+  else if (const EnumConstantDecl *ECD = this->getEnumConstantDecl())

Kupa-Martin wrote:

done

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[via cfe-commits]

https://github.com/Kupa-Martin updated 
https://github.com/llvm/llvm-project/pull/81418

>From 706ad23821800a624792dd00b7b38b3f7cfe89ad Mon Sep 17 00:00:00 2001
From: 44-2-Kupa-Martin 
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| 15 +++
 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, 84 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 `).
+
 - 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(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 8b10e289583260..ee9515e218cba0 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 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(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

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Erich Keane via cfe-commits]

https://github.com/erichkeane commented:

A couple more coding standard edits, else LGTM.  

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Erich Keane via cfe-commits]

@@ -4097,6 +4105,13 @@ FieldDecl *Expr::getSourceBitField() {
   return nullptr;
 }
 
+EnumConstantDecl *Expr::getEnumConstantDecl() {
+  Expr *E = this->IgnoreParenImpCasts();
+  if (DeclRefExpr *DRE = dyn_cast(E))

erichkeane wrote:

```suggestion
  if (auto *DRE = dyn_cast(E))
```

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Erich Keane via cfe-commits]

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Erich Keane via cfe-commits]

@@ -263,6 +263,14 @@ namespace {
   }
 }
 
+QualType Expr::getEnumCoercedType(const ASTContext &Ctx) const {
+  if (isa(this->getType()))
+return this->getType();
+  else if (const EnumConstantDecl *ECD = this->getEnumConstantDecl())

erichkeane wrote:

```suggestion
  else if (const auto *ECD = this->getEnumConstantDecl())
```

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[via cfe-commits]

@@ -263,6 +263,14 @@ namespace {
   }
 }
 
+QualType Expr::getEnumCoercedType(const ASTContext &Ctx) const {
+  bool NotEnumType = dyn_cast(this->getType()) == nullptr;
+  if (NotEnumType)

Kupa-Martin wrote:

done

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[via cfe-commits]

@@ -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; 
+}

Kupa-Martin wrote:

done

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[via cfe-commits]

https://github.com/Kupa-Martin updated 
https://github.com/llvm/llvm-project/pull/81418

>From f08540e18abc078623ea1273f246003b76c24562 Mon Sep 17 00:00:00 2001
From: 44-2-Kupa-Martin 
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| 15 +++
 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, 84 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 `).
+
 - 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(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 8b10e289583260..210c19edba040f 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 EnumConstantDecl *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 (DeclRefExpr *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 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(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->getEnumCo

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[via cfe-commits]

https://github.com/Kupa-Martin updated 
https://github.com/llvm/llvm-project/pull/81418

>From ca5879d97c3573ea26a3b431c3c18656a67370f2 Mon Sep 17 00:00:00 2001
From: 44-2-Kupa-Martin 
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 `).
+
 - 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(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 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(this->getType()) == nullptr;
+  if (NotEnumType)
+if (const EnumConstantDecl *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,14 @@ FieldDecl *Expr::getSourceBitField() {
   return nullptr;
 }
 
+EnumConstantDecl *Expr::getEnumConstantDecl() {
+  Expr *E = this->IgnoreParenImpCasts();
+  if (DeclRefExpr *DRE = dyn_cast(E))
+if (EnumConstantDecl *ECD = dyn_cast(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(E))
-  if (EnumConstantDecl *ECD = dyn_cast(DRE->getDecl())) {
-EnumDecl *Enum = cast(ECD->getDeclContext());
-SourceType = S.Context.getTypeDeclType(Enum);
-Source = S.Context.getCanonicalType(SourceType).getT

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[via cfe-commits]

@@ -263,6 +263,14 @@ namespace {
   }
 }
 
+QualType Expr::getEnumCoercedType(const ASTContext &Ctx) const {
+  bool NotEnumType = dyn_cast(this->getType()) == nullptr;
+  if (NotEnumType)

Kupa-Martin wrote:

In C mode enumeration constants have integral type, so `getEnumConstantDecl()` 
might not return `nullptr` even if the expression doesnt have enum type.

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Shafik Yaghmour via cfe-commits]

@@ -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; 
+}

shafik wrote:

Please add a newline at the end of the file

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Shafik Yaghmour via cfe-commits]

@@ -263,6 +263,14 @@ namespace {
   }
 }
 
+QualType Expr::getEnumCoercedType(const ASTContext &Ctx) const {
+  bool NotEnumType = dyn_cast(this->getType()) == nullptr;
+  if (NotEnumType)

shafik wrote:

This look redundant since `getEnumConstantDecl()` will just return `nullptr`

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Erich Keane via cfe-commits]

@@ -4097,6 +4105,14 @@ FieldDecl *Expr::getSourceBitField() {
   return nullptr;
 }
 
+EnumConstantDecl *Expr::getEnumConstantDecl() {
+  Expr *E = this->IgnoreParenImpCasts();
+  if (DeclRefExpr *DRE = dyn_cast(E))
+if (EnumConstantDecl *ECD = dyn_cast(DRE->getDecl()))

erichkeane wrote:

```suggestion
return dyn_cast(DRE->getDecl());
```

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Erich Keane via cfe-commits]

@@ -263,6 +263,14 @@ namespace {
   }
 }
 
+QualType Expr::getEnumCoercedType(const ASTContext &Ctx) const {
+  bool NotEnumType = dyn_cast(this->getType()) == nullptr;
+  if (NotEnumType)

erichkeane wrote:

```suggestion
  if (!isa(this->getType()))
```

Though, would probably suggest just something like:
```
if (isa(getType())) return getType();

else if (const EnumConstantDecl *ECD = getEnumConstantDecl())
  return Ctx.getTypeDeclType...
return getType();

```

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Erich Keane via cfe-commits]

https://github.com/erichkeane commented:

Not the best one to review this, but some drive-bys

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[Erich Keane via cfe-commits]

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[via cfe-commits]

Kupa-Martin wrote:

ping

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[via cfe-commits]

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: None (44-2-Kupa-Martin)


Changes

…C mode

Factored logic from `CheckImplicitConversion` into new methods 
`Expr::getEnumConstantDecl` and `Expr::getEnumCoercedType` for use in 
`checkEnumArithmeticConversions`.

Fix #29217

---
Full diff: https://github.com/llvm/llvm-project/pull/81418.diff


9 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+3) 
- (modified) clang/include/clang/AST/Expr.h (+13) 
- (modified) clang/lib/AST/Expr.cpp (+16) 
- (modified) clang/lib/Sema/SemaChecking.cpp (+2-9) 
- (modified) clang/lib/Sema/SemaExpr.cpp (+2-1) 
- (modified) clang/test/Sema/builtins-elementwise-math.c (+4) 
- (added) clang/test/Sema/warn-compare-enum-types-mismatch.c (+42) 
- (renamed) clang/test/Sema/warn-conditional-enum-types-mismatch.c (+1-1) 
- (modified) clang/test/Sema/warn-overlap.c (+2-2) 


``diff
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 `).
+
 - 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(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 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(this->getType()) == nullptr;
+  if (NotEnumType)
+if (const EnumConstantDecl *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,14 @@ FieldDecl *Expr::getSourceBitField() {
   return nullptr;
 }
 
+EnumConstantDecl *Expr::getEnumConstantDecl() {
+  Expr *E = this->IgnoreParenImpCasts();
+  if (DeclRefExpr *DRE = dyn_cast(E))
+if (EnumConstantDecl *ECD = dyn_cast(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(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 4049ab3bf6cafb..2b6accfd4a4d50 100644
--

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[via cfe-commits]

github-actions[bot] wrote:



Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this 
page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using `@` followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from 
other developers.

If you have further questions, they may be answered by the [LLVM GitHub User 
Guide](https://llvm.org/docs/GitHub.html).

You can also ask questions in a comment on this PR, on the [LLVM 
Discord](https://discord.com/invite/xS7Z362) or on the 
[forums](https://discourse.llvm.org/).

https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)

[via cfe-commits]

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 
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 `).
+
 - 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(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 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(this->getType()) == nullptr;
+  if (NotEnumType)
+if (const EnumConstantDecl *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,14 @@ FieldDecl *Expr::getSourceBitField() {
   return nullptr;
 }
 
+EnumConstantDecl *Expr::getEnumConstantDecl() {
+  Expr *E = this->IgnoreParenImpCasts();
+  if (DeclRefExpr *DRE = dyn_cast(E))
+if (EnumConstantDecl *ECD = dyn_cast(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(E))
-  if (EnumConstantDecl *ECD = dyn