[clang] [clang][Sema] Move computing enum width and type to a separate function (PR #120965)

Ilia Kuklin via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 23 10:58:48 PST 2024


https://github.com/kuilpd updated https://github.com/llvm/llvm-project/pull/120965

>From ab8dcaef120233d0145508aaa4bf45eec22cadf1 Mon Sep 17 00:00:00 2001
From: Ilia Kuklin <ikuklin at accesssoftek.com>
Date: Mon, 23 Dec 2024 17:49:32 +0500
Subject: [PATCH 1/2] [clang][Sema] Move calculating enum width and type to a
 separate function

---
 clang/include/clang/Sema/Sema.h |   7 ++
 clang/lib/Sema/SemaDecl.cpp     | 158 ++++++++++++++++++--------------
 2 files changed, 94 insertions(+), 71 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5ee7ea48cc983c..51a1721fb74f01 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3987,6 +3987,13 @@ class Sema final : public SemaBase {
                           SourceLocation IdLoc, IdentifierInfo *Id,
                           const ParsedAttributesView &Attrs,
                           SourceLocation EqualLoc, Expr *Val);
+
+  bool ComputeBestEnumProperties(ASTContext &Context, EnumDecl *Enum,
+                                 bool isCpp, bool isPacked,
+                                 unsigned NumNegativeBits,
+                                 unsigned NumPositiveBits, unsigned &BestWidth,
+                                 QualType &BestType,
+                                 QualType &BestPromotionType);
   void ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
                      Decl *EnumDecl, ArrayRef<Decl *> Elements, Scope *S,
                      const ParsedAttributesView &Attr);
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 4001c4d263f1d2..79cbfe3116b26b 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -20008,6 +20008,87 @@ bool Sema::IsValueInFlagEnum(const EnumDecl *ED, const llvm::APInt &Val,
   return !(FlagMask & Val) || (AllowMask && !(FlagMask & ~Val));
 }
 
+bool Sema::ComputeBestEnumProperties(ASTContext &Context, EnumDecl *Enum,
+                                     bool is_cpp, bool isPacked,
+                                     unsigned NumNegativeBits,
+                                     unsigned NumPositiveBits,
+                                     unsigned &BestWidth, QualType &BestType,
+                                     QualType &BestPromotionType) {
+  unsigned IntWidth = Context.getTargetInfo().getIntWidth();
+  unsigned CharWidth = Context.getTargetInfo().getCharWidth();
+  unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
+  bool enum_too_large = false;
+  if (NumNegativeBits) {
+    // If there is a negative value, figure out the smallest integer type (of
+    // int/long/longlong) that fits.
+    // If it's packed, check also if it fits a char or a short.
+    if (isPacked && NumNegativeBits <= CharWidth &&
+        NumPositiveBits < CharWidth) {
+      BestType = Context.SignedCharTy;
+      BestWidth = CharWidth;
+    } else if (isPacked && NumNegativeBits <= ShortWidth &&
+               NumPositiveBits < ShortWidth) {
+      BestType = Context.ShortTy;
+      BestWidth = ShortWidth;
+    } else if (NumNegativeBits <= IntWidth && NumPositiveBits < IntWidth) {
+      BestType = Context.IntTy;
+      BestWidth = IntWidth;
+    } else {
+      BestWidth = Context.getTargetInfo().getLongWidth();
+
+      if (NumNegativeBits <= BestWidth && NumPositiveBits < BestWidth) {
+        BestType = Context.LongTy;
+      } else {
+        BestWidth = Context.getTargetInfo().getLongLongWidth();
+
+        if (NumNegativeBits > BestWidth || NumPositiveBits >= BestWidth)
+          enum_too_large = true;
+        BestType = Context.LongLongTy;
+      }
+    }
+    BestPromotionType = (BestWidth <= IntWidth ? Context.IntTy : BestType);
+  } else {
+    // If there is no negative value, figure out the smallest type that fits
+    // all of the enumerator values.
+    // If it's packed, check also if it fits a char or a short.
+    if (isPacked && NumPositiveBits <= CharWidth) {
+      BestType = Context.UnsignedCharTy;
+      BestPromotionType = Context.IntTy;
+      BestWidth = CharWidth;
+    } else if (isPacked && NumPositiveBits <= ShortWidth) {
+      BestType = Context.UnsignedShortTy;
+      BestPromotionType = Context.IntTy;
+      BestWidth = ShortWidth;
+    } else if (NumPositiveBits <= IntWidth) {
+      BestType = Context.UnsignedIntTy;
+      BestWidth = IntWidth;
+      BestPromotionType = (NumPositiveBits == BestWidth || !is_cpp)
+                              ? Context.UnsignedIntTy
+                              : Context.IntTy;
+    } else if (NumPositiveBits <=
+               (BestWidth = Context.getTargetInfo().getLongWidth())) {
+      BestType = Context.UnsignedLongTy;
+      BestPromotionType = (NumPositiveBits == BestWidth || !is_cpp)
+                              ? Context.UnsignedLongTy
+                              : Context.LongTy;
+    } else {
+      BestWidth = Context.getTargetInfo().getLongLongWidth();
+      if (NumPositiveBits > BestWidth) {
+        // This can happen with bit-precise integer types, but those are not
+        // allowed as the type for an enumerator per C23 6.7.2.2p4 and p12.
+        // FIXME: GCC uses __int128_t and __uint128_t for cases that fit within
+        // a 128-bit integer, we should consider doing the same.
+        enum_too_large = true;
+      }
+      BestType = Context.UnsignedLongLongTy;
+      BestPromotionType = (NumPositiveBits == BestWidth || !is_cpp)
+                              ? Context.UnsignedLongLongTy
+                              : Context.LongLongTy;
+    }
+  }
+  return enum_too_large;
+}
+
 void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
                          Decl *EnumDeclX, ArrayRef<Decl *> Elements, Scope *S,
                          const ParsedAttributesView &Attrs) {
@@ -20030,10 +20111,6 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
     return;
   }
 
-  unsigned IntWidth = Context.getTargetInfo().getIntWidth();
-  unsigned CharWidth = Context.getTargetInfo().getCharWidth();
-  unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
-
   // Verify that all the values are okay, compute the size of the values, and
   // reverse the list.
   unsigned NumNegativeBits = 0;
@@ -20099,73 +20176,12 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
       BestPromotionType = BestType;
 
     BestWidth = Context.getIntWidth(BestType);
-  }
-  else if (NumNegativeBits) {
-    // If there is a negative value, figure out the smallest integer type (of
-    // int/long/longlong) that fits.
-    // If it's packed, check also if it fits a char or a short.
-    if (Packed && NumNegativeBits <= CharWidth && NumPositiveBits < CharWidth) {
-      BestType = Context.SignedCharTy;
-      BestWidth = CharWidth;
-    } else if (Packed && NumNegativeBits <= ShortWidth &&
-               NumPositiveBits < ShortWidth) {
-      BestType = Context.ShortTy;
-      BestWidth = ShortWidth;
-    } else if (NumNegativeBits <= IntWidth && NumPositiveBits < IntWidth) {
-      BestType = Context.IntTy;
-      BestWidth = IntWidth;
-    } else {
-      BestWidth = Context.getTargetInfo().getLongWidth();
-
-      if (NumNegativeBits <= BestWidth && NumPositiveBits < BestWidth) {
-        BestType = Context.LongTy;
-      } else {
-        BestWidth = Context.getTargetInfo().getLongLongWidth();
-
-        if (NumNegativeBits > BestWidth || NumPositiveBits >= BestWidth)
-          Diag(Enum->getLocation(), diag::ext_enum_too_large);
-        BestType = Context.LongLongTy;
-      }
-    }
-    BestPromotionType = (BestWidth <= IntWidth ? Context.IntTy : BestType);
   } else {
-    // If there is no negative value, figure out the smallest type that fits
-    // all of the enumerator values.
-    // If it's packed, check also if it fits a char or a short.
-    if (Packed && NumPositiveBits <= CharWidth) {
-      BestType = Context.UnsignedCharTy;
-      BestPromotionType = Context.IntTy;
-      BestWidth = CharWidth;
-    } else if (Packed && NumPositiveBits <= ShortWidth) {
-      BestType = Context.UnsignedShortTy;
-      BestPromotionType = Context.IntTy;
-      BestWidth = ShortWidth;
-    } else if (NumPositiveBits <= IntWidth) {
-      BestType = Context.UnsignedIntTy;
-      BestWidth = IntWidth;
-      BestPromotionType
-        = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
-                           ? Context.UnsignedIntTy : Context.IntTy;
-    } else if (NumPositiveBits <=
-               (BestWidth = Context.getTargetInfo().getLongWidth())) {
-      BestType = Context.UnsignedLongTy;
-      BestPromotionType
-        = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
-                           ? Context.UnsignedLongTy : Context.LongTy;
-    } else {
-      BestWidth = Context.getTargetInfo().getLongLongWidth();
-      if (NumPositiveBits > BestWidth) {
-        // This can happen with bit-precise integer types, but those are not
-        // allowed as the type for an enumerator per C23 6.7.2.2p4 and p12.
-        // FIXME: GCC uses __int128_t and __uint128_t for cases that fit within
-        // a 128-bit integer, we should consider doing the same.
-        Diag(Enum->getLocation(), diag::ext_enum_too_large);
-      }
-      BestType = Context.UnsignedLongLongTy;
-      BestPromotionType
-        = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
-                           ? Context.UnsignedLongLongTy : Context.LongLongTy;
-    }
+    bool enum_too_large = ComputeBestEnumProperties(
+        Context, Enum, getLangOpts().CPlusPlus, Packed, NumNegativeBits,
+        NumPositiveBits, BestWidth, BestType, BestPromotionType);
+    if (enum_too_large)
+      Diag(Enum->getLocation(), diag::ext_enum_too_large);
   }
 
   // Loop over all of the enumerator constants, changing their types to match
@@ -20197,7 +20213,7 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
       //  int; or,
       //  - the enumerated type
       NewTy = Context.IntTy;
-      NewWidth = IntWidth;
+      NewWidth = Context.getTargetInfo().getIntWidth();
       NewSign = true;
     } else if (ECD->getType() == BestType) {
       // Already the right type!

>From c41ddb9d9cd5d8d03a541e3a67e8f903af5160c3 Mon Sep 17 00:00:00 2001
From: Ilia Kuklin <ikuklin at accesssoftek.com>
Date: Mon, 23 Dec 2024 23:48:50 +0500
Subject: [PATCH 2/2] Remove unnecessary arguments and adjust formatting

---
 clang/include/clang/Sema/Sema.h |  6 ++---
 clang/lib/Sema/SemaDecl.cpp     | 44 ++++++++++++++++++---------------
 2 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 51a1721fb74f01..29310a45a521f2 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3988,11 +3988,9 @@ class Sema final : public SemaBase {
                           const ParsedAttributesView &Attrs,
                           SourceLocation EqualLoc, Expr *Val);
 
-  bool ComputeBestEnumProperties(ASTContext &Context, EnumDecl *Enum,
-                                 bool isCpp, bool isPacked,
+  bool ComputeBestEnumProperties(ASTContext &Context, bool isPacked,
                                  unsigned NumNegativeBits,
-                                 unsigned NumPositiveBits, unsigned &BestWidth,
-                                 QualType &BestType,
+                                 unsigned NumPositiveBits, QualType &BestType,
                                  QualType &BestPromotionType);
   void ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
                      Decl *EnumDecl, ArrayRef<Decl *> Elements, Scope *S,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 79cbfe3116b26b..16b074d77b8c78 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -20008,16 +20008,16 @@ bool Sema::IsValueInFlagEnum(const EnumDecl *ED, const llvm::APInt &Val,
   return !(FlagMask & Val) || (AllowMask && !(FlagMask & ~Val));
 }
 
-bool Sema::ComputeBestEnumProperties(ASTContext &Context, EnumDecl *Enum,
-                                     bool is_cpp, bool isPacked,
+bool Sema::ComputeBestEnumProperties(ASTContext &Context, bool isPacked,
                                      unsigned NumNegativeBits,
                                      unsigned NumPositiveBits,
-                                     unsigned &BestWidth, QualType &BestType,
+                                     QualType &BestType,
                                      QualType &BestPromotionType) {
   unsigned IntWidth = Context.getTargetInfo().getIntWidth();
   unsigned CharWidth = Context.getTargetInfo().getCharWidth();
   unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
-  bool enum_too_large = false;
+  bool EnumTooLarge = false;
+  unsigned BestWidth;
   if (NumNegativeBits) {
     // If there is a negative value, figure out the smallest integer type (of
     // int/long/longlong) that fits.
@@ -20042,7 +20042,7 @@ bool Sema::ComputeBestEnumProperties(ASTContext &Context, EnumDecl *Enum,
         BestWidth = Context.getTargetInfo().getLongLongWidth();
 
         if (NumNegativeBits > BestWidth || NumPositiveBits >= BestWidth)
-          enum_too_large = true;
+          EnumTooLarge = true;
         BestType = Context.LongLongTy;
       }
     }
@@ -20062,15 +20062,17 @@ bool Sema::ComputeBestEnumProperties(ASTContext &Context, EnumDecl *Enum,
     } else if (NumPositiveBits <= IntWidth) {
       BestType = Context.UnsignedIntTy;
       BestWidth = IntWidth;
-      BestPromotionType = (NumPositiveBits == BestWidth || !is_cpp)
-                              ? Context.UnsignedIntTy
-                              : Context.IntTy;
+      BestPromotionType =
+          (NumPositiveBits == BestWidth || !Context.getLangOpts().CPlusPlus)
+              ? Context.UnsignedIntTy
+              : Context.IntTy;
     } else if (NumPositiveBits <=
                (BestWidth = Context.getTargetInfo().getLongWidth())) {
       BestType = Context.UnsignedLongTy;
-      BestPromotionType = (NumPositiveBits == BestWidth || !is_cpp)
-                              ? Context.UnsignedLongTy
-                              : Context.LongTy;
+      BestPromotionType =
+          (NumPositiveBits == BestWidth || !Context.getLangOpts().CPlusPlus)
+              ? Context.UnsignedLongTy
+              : Context.LongTy;
     } else {
       BestWidth = Context.getTargetInfo().getLongLongWidth();
       if (NumPositiveBits > BestWidth) {
@@ -20078,15 +20080,16 @@ bool Sema::ComputeBestEnumProperties(ASTContext &Context, EnumDecl *Enum,
         // allowed as the type for an enumerator per C23 6.7.2.2p4 and p12.
         // FIXME: GCC uses __int128_t and __uint128_t for cases that fit within
         // a 128-bit integer, we should consider doing the same.
-        enum_too_large = true;
+        EnumTooLarge = true;
       }
       BestType = Context.UnsignedLongLongTy;
-      BestPromotionType = (NumPositiveBits == BestWidth || !is_cpp)
-                              ? Context.UnsignedLongLongTy
-                              : Context.LongLongTy;
+      BestPromotionType =
+          (NumPositiveBits == BestWidth || !Context.getLangOpts().CPlusPlus)
+              ? Context.UnsignedLongLongTy
+              : Context.LongLongTy;
     }
   }
-  return enum_too_large;
+  return EnumTooLarge;
 }
 
 void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
@@ -20177,10 +20180,11 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
 
     BestWidth = Context.getIntWidth(BestType);
   } else {
-    bool enum_too_large = ComputeBestEnumProperties(
-        Context, Enum, getLangOpts().CPlusPlus, Packed, NumNegativeBits,
-        NumPositiveBits, BestWidth, BestType, BestPromotionType);
-    if (enum_too_large)
+    bool EnumTooLarge =
+        ComputeBestEnumProperties(Context, Packed, NumNegativeBits,
+                                  NumPositiveBits, BestType, BestPromotionType);
+    BestWidth = Context.getIntWidth(BestType);
+    if (EnumTooLarge)
       Diag(Enum->getLocation(), diag::ext_enum_too_large);
   }
 



More information about the cfe-commits mailing list