[clang] [clang][Sema] Cleanup and optimize DiagnoseAssignmentEnum (PR #141471)

[Timm Baeder via cfe-commits]

Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/141471 at github.com>


https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/141471

>From 7b0a2b83146623a1674967ff4353a9eea6efd25e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Mon, 26 May 2025 12:19:22 +0200
Subject: [PATCH 1/2] [clang][Sema] Cleanup and optimize DiagnoseAssignmentEnum

Reorder the precondition checks to move the costly onces last. Also,
only evaluate the RHS once to get the integral value.
---
 clang/lib/Sema/SemaStmt.cpp | 98 ++++++++++++++++++++-----------------
 1 file changed, 53 insertions(+), 45 deletions(-)

diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index ed070649dc1f9..08f77b2e246e9 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -1741,57 +1741,65 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
 void
 Sema::DiagnoseAssignmentEnum(QualType DstType, QualType SrcType,
                              Expr *SrcExpr) {
+
+  const auto *ET = DstType->getAs<EnumType>();
+  if (!ET)
+    return;
+
+  if (SrcType->isIntegerType() ||
+      Context.hasSameUnqualifiedType(SrcType, DstType))
+    return;
+
+  if (SrcExpr->isTypeDependent() || SrcExpr->isValueDependent())
+    return;
+
+  const EnumDecl *ED = ET->getDecl();
+  if (!ED->isClosed())
+    return;
+
   if (Diags.isIgnored(diag::warn_not_in_enum_assignment, SrcExpr->getExprLoc()))
     return;
 
-  if (const EnumType *ET = DstType->getAs<EnumType>())
-    if (!Context.hasSameUnqualifiedType(SrcType, DstType) &&
-        SrcType->isIntegerType()) {
-      if (!SrcExpr->isTypeDependent() && !SrcExpr->isValueDependent() &&
-          SrcExpr->isIntegerConstantExpr(Context)) {
-        // Get the bitwidth of the enum value before promotions.
-        unsigned DstWidth = Context.getIntWidth(DstType);
-        bool DstIsSigned = DstType->isSignedIntegerOrEnumerationType();
+  std::optional<llvm::APSInt> RHSVal = SrcExpr->getIntegerConstantExpr(Context);
+  if (!RHSVal)
+    return;
 
-        llvm::APSInt RhsVal = SrcExpr->EvaluateKnownConstInt(Context);
-        AdjustAPSInt(RhsVal, DstWidth, DstIsSigned);
-        const EnumDecl *ED = ET->getDecl();
+  // Get the bitwidth of the enum value before promotions.
+  unsigned DstWidth = Context.getIntWidth(DstType);
+  bool DstIsSigned = DstType->isSignedIntegerOrEnumerationType();
+  AdjustAPSInt(*RHSVal, DstWidth, DstIsSigned);
 
-        if (!ED->isClosed())
-          return;
+  if (ED->hasAttr<FlagEnumAttr>()) {
+    if (!IsValueInFlagEnum(ED, *RHSVal, /*AllowMask=*/true))
+      Diag(SrcExpr->getExprLoc(), diag::warn_not_in_enum_assignment)
+          << DstType.getUnqualifiedType();
+    return;
+  }
 
-        if (ED->hasAttr<FlagEnumAttr>()) {
-          if (!IsValueInFlagEnum(ED, RhsVal, true))
-            Diag(SrcExpr->getExprLoc(), diag::warn_not_in_enum_assignment)
-              << DstType.getUnqualifiedType();
-        } else {
-          typedef SmallVector<std::pair<llvm::APSInt, EnumConstantDecl *>, 64>
-              EnumValsTy;
-          EnumValsTy EnumVals;
-
-          // Gather all enum values, set their type and sort them,
-          // allowing easier comparison with rhs constant.
-          for (auto *EDI : ED->enumerators()) {
-            llvm::APSInt Val = EDI->getInitVal();
-            AdjustAPSInt(Val, DstWidth, DstIsSigned);
-            EnumVals.push_back(std::make_pair(Val, EDI));
-          }
-          if (EnumVals.empty())
-            return;
-          llvm::stable_sort(EnumVals, CmpEnumVals);
-          EnumValsTy::iterator EIend = llvm::unique(EnumVals, EqEnumVals);
-
-          // See which values aren't in the enum.
-          EnumValsTy::const_iterator EI = EnumVals.begin();
-          while (EI != EIend && EI->first < RhsVal)
-            EI++;
-          if (EI == EIend || EI->first != RhsVal) {
-            Diag(SrcExpr->getExprLoc(), diag::warn_not_in_enum_assignment)
-                << DstType.getUnqualifiedType();
-          }
-        }
-      }
-    }
+  typedef SmallVector<std::pair<llvm::APSInt, EnumConstantDecl *>, 64>
+      EnumValsTy;
+  EnumValsTy EnumVals;
+
+  // Gather all enum values, set their type and sort them,
+  // allowing easier comparison with rhs constant.
+  for (auto *EDI : ED->enumerators()) {
+    llvm::APSInt Val = EDI->getInitVal();
+    AdjustAPSInt(Val, DstWidth, DstIsSigned);
+    EnumVals.emplace_back(Val, EDI);
+  }
+  if (EnumVals.empty())
+    return;
+  llvm::stable_sort(EnumVals, CmpEnumVals);
+  EnumValsTy::iterator EIend = llvm::unique(EnumVals, EqEnumVals);
+
+  // See which values aren't in the enum.
+  EnumValsTy::const_iterator EI = EnumVals.begin();
+  while (EI != EIend && EI->first < *RHSVal)
+    EI++;
+  if (EI == EIend || EI->first != *RHSVal) {
+    Diag(SrcExpr->getExprLoc(), diag::warn_not_in_enum_assignment)
+        << DstType.getUnqualifiedType();
+  }
 }
 
 StmtResult Sema::ActOnWhileStmt(SourceLocation WhileLoc,

>From 02fb34099f99c1f22ecb5cb9d930b35c914eb1ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Mon, 26 May 2025 13:01:57 +0200
Subject: [PATCH 2/2] Whoops

---
 clang/lib/Sema/SemaStmt.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 08f77b2e246e9..85337acf5e48b 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -1746,7 +1746,7 @@ Sema::DiagnoseAssignmentEnum(QualType DstType, QualType SrcType,
   if (!ET)
     return;
 
-  if (SrcType->isIntegerType() ||
+  if (!SrcType->isIntegerType() ||
       Context.hasSameUnqualifiedType(SrcType, DstType))
     return;