Implement promotion for enumeration types.

WHAT!?!

It turns out that Type::isPromotableIntegerType() was not considering
enumeration types to be promotable, so we would never do the
promotion despite having properly computed the promotion type when the
enum was defined. Various operations on values of enum type just
"worked" because we could still compute the integer rank of an enum
type; the oddity, however, is that operations such as "add an enum and
an unsigned" would often have an enum result type (!). The bug
actually showed up as a spurious -Wformat diagnostic
(<rdar://problem/7595366>), but in theory it could cause miscompiles.

In this commit:
  - Enum types with a promotion type of "int" or "unsigned int" are
  promotable.
  - Tweaked the computation of promotable types for enums
  - For all of the ABIs, treat enum types the same way as their
  underlying types (*not* their promotion types) for argument passing
  and return values
  - Extend the ABI tester with support for enumeration types

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@95117 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp
index edfb580cc35f154725d79d40d012c146143eb511..504ffc5e4e95f8ec4db65e2324a12f73164888ff 100644 (file)
--- a/lib/AST/Type.cpp
+++ b/lib/AST/Type.cpp
@@ -720,6 +720,19 @@ bool Type::isPromotableIntegerType() const {
     default:
       return false;
     }
+
+  // Enumerated types are promotable to their compatible integer types
+  // (C99 6.3.1.1) a.k.a. its underlying type (C++ [conv.prom]p2).
+  if (const EnumType *ET = getAs<EnumType>()){
+    if (this->isDependentType() || ET->getDecl()->getPromotionType().isNull())
+      return false;
+    
+    const BuiltinType *BT
+      = ET->getDecl()->getPromotionType()->getAs<BuiltinType>();
+    return BT->getKind() == BuiltinType::Int
+           || BT->getKind() == BuiltinType::UInt;
+  }
+  
   return false;
 }
 

diff --git a/lib/CodeGen/TargetInfo.cpp b/lib/CodeGen/TargetInfo.cpp
index 07f116a0e0b1461c5974b941b69c06370ca10501..6f650fc2ef70db6af1f548a952c6b45dec2f4d28 100644 (file)
--- a/lib/CodeGen/TargetInfo.cpp
+++ b/lib/CodeGen/TargetInfo.cpp
@@ -271,6 +271,10 @@ ABIArgInfo DefaultABIInfo::classifyArgumentType(QualType Ty,
   if (CodeGenFunction::hasAggregateLLVMType(Ty)) {
     return ABIArgInfo::getIndirect(0);
   } else {
+    // Treat an enum type as its underlying type.
+    if (const EnumType *EnumTy = Ty->getAs<EnumType>())
+      Ty = EnumTy->getDecl()->getIntegerType();
+
     return (Ty->isPromotableIntegerType() ?
             ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
   }
@@ -465,6 +469,10 @@ ABIArgInfo X86_32ABIInfo::classifyReturnType(QualType RetTy,
 
     return ABIArgInfo::getIndirect(0);
   } else {
+    // Treat an enum type as its underlying type.
+    if (const EnumType *EnumTy = RetTy->getAs<EnumType>())
+      RetTy = EnumTy->getDecl()->getIntegerType();
+
     return (RetTy->isPromotableIntegerType() ?
             ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
   }
@@ -511,6 +519,9 @@ ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty,
 
     return ABIArgInfo::getIndirect(getIndirectArgumentAlignment(Ty, Context));
   } else {
+    if (const EnumType *EnumTy = Ty->getAs<EnumType>())
+      Ty = EnumTy->getDecl()->getIntegerType();
+
     return (Ty->isPromotableIntegerType() ?
             ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
   }
@@ -935,6 +946,11 @@ ABIArgInfo X86_64ABIInfo::getCoerceResult(QualType Ty,
   if (CoerceTo == llvm::Type::getInt64Ty(CoerceTo->getContext())) {
     // Integer and pointer types will end up in a general purpose
     // register.
+
+    // Treat an enum type as its underlying type.
+    if (const EnumType *EnumTy = Ty->getAs<EnumType>())
+      Ty = EnumTy->getDecl()->getIntegerType();
+
     if (Ty->isIntegralType() || Ty->hasPointerRepresentation())
       return (Ty->isPromotableIntegerType() ?
               ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
@@ -956,9 +972,14 @@ ABIArgInfo X86_64ABIInfo::getIndirectResult(QualType Ty,
                                             ASTContext &Context) const {
   // If this is a scalar LLVM value then assume LLVM will pass it in the right
   // place naturally.
-  if (!CodeGenFunction::hasAggregateLLVMType(Ty))
+  if (!CodeGenFunction::hasAggregateLLVMType(Ty)) {
+    // Treat an enum type as its underlying type.
+    if (const EnumType *EnumTy = Ty->getAs<EnumType>())
+      Ty = EnumTy->getDecl()->getIntegerType();
+
     return (Ty->isPromotableIntegerType() ?
             ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
+  }
 
   bool ByVal = !isRecordWithNonTrivialDestructorOrCopyConstructor(Ty);
 
@@ -1534,9 +1555,14 @@ void ARMABIInfo::computeInfo(CGFunctionInfo &FI, ASTContext &Context,
 ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty,
                                             ASTContext &Context,
                                           llvm::LLVMContext &VMContext) const {
-  if (!CodeGenFunction::hasAggregateLLVMType(Ty))
+  if (!CodeGenFunction::hasAggregateLLVMType(Ty)) {
+    // Treat an enum type as its underlying type.
+    if (const EnumType *EnumTy = Ty->getAs<EnumType>())
+      Ty = EnumTy->getDecl()->getIntegerType();
+
     return (Ty->isPromotableIntegerType() ?
             ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
+  }
 
   // Ignore empty records.
   if (isEmptyRecord(Context, Ty, true))
@@ -1652,9 +1678,14 @@ ABIArgInfo ARMABIInfo::classifyReturnType(QualType RetTy,
   if (RetTy->isVoidType())
     return ABIArgInfo::getIgnore();
 
-  if (!CodeGenFunction::hasAggregateLLVMType(RetTy))
+  if (!CodeGenFunction::hasAggregateLLVMType(RetTy)) {
+    // Treat an enum type as its underlying type.
+    if (const EnumType *EnumTy = RetTy->getAs<EnumType>())
+      RetTy = EnumTy->getDecl()->getIntegerType();
+
     return (RetTy->isPromotableIntegerType() ?
             ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
+  }
 
   // Are we following APCS?
   if (getABIKind() == APCS) {
@@ -1737,6 +1768,10 @@ ABIArgInfo DefaultABIInfo::classifyReturnType(QualType RetTy,
   } else if (CodeGenFunction::hasAggregateLLVMType(RetTy)) {
     return ABIArgInfo::getIndirect(0);
   } else {
+    // Treat an enum type as its underlying type.
+    if (const EnumType *EnumTy = RetTy->getAs<EnumType>())
+      RetTy = EnumTy->getDecl()->getIntegerType();
+
     return (RetTy->isPromotableIntegerType() ?
             ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
   }

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 8f756da70da9ef98d7894eb1e23a4e1a82f6e89f..c604f6a4eef44c50d8f68e8dfcb0b0a05ab2b784 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -5971,8 +5971,8 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceLocation LBraceLoc,
     }
     BestPromotionType = (BestWidth <= IntWidth ? Context.IntTy : BestType);
   } else {
-    // If there is no negative value, figure out which of uint, ulong, ulonglong
-    // fits.
+    // 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;
@@ -5985,30 +5985,26 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceLocation LBraceLoc,
     } else if (NumPositiveBits <= IntWidth) {
       BestType = Context.UnsignedIntTy;
       BestWidth = IntWidth;
-      BestPromotionType = (NumPositiveBits == BestWidth
-                           ? Context.UnsignedIntTy : Context.IntTy);
+      BestPromotionType
+        = (NumPositiveBits == BestWidth || !getLangOptions().CPlusPlus)
+                           ? Context.UnsignedIntTy : Context.IntTy;
     } else if (NumPositiveBits <=
                (BestWidth = Context.Target.getLongWidth())) {
       BestType = Context.UnsignedLongTy;
-      BestPromotionType = (NumPositiveBits == BestWidth
-                           ? Context.UnsignedLongTy : Context.LongTy);
+      BestPromotionType
+        = (NumPositiveBits == BestWidth || !getLangOptions().CPlusPlus)
+                           ? Context.UnsignedLongTy : Context.LongTy;
     } else {
       BestWidth = Context.Target.getLongLongWidth();
       assert(NumPositiveBits <= BestWidth &&
              "How could an initializer get larger than ULL?");
       BestType = Context.UnsignedLongLongTy;
-      BestPromotionType = (NumPositiveBits == BestWidth
-                           ? Context.UnsignedLongLongTy : Context.LongLongTy);
+      BestPromotionType
+        = (NumPositiveBits == BestWidth || !getLangOptions().CPlusPlus)
+                           ? Context.UnsignedLongLongTy : Context.LongLongTy;
     }
   }
 
-  // If we're in C and the promotion type is larger than an int, just
-  // use the underlying type, which is generally the unsigned integer
-  // type of the same rank as the promotion type.  This is how the gcc
-  // extension works.
-  if (!getLangOptions().CPlusPlus && BestPromotionType != Context.IntTy)
-    BestPromotionType = BestType;
-
   // Loop over all of the enumerator constants, changing their types to match
   // the type of the enum if needed.
   for (unsigned i = 0; i != NumElements; ++i) {

diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp
index 033cbfd459f02a4421d4c5b44f801fee661168ec..d1d9bda93e552cc82c3a930a2112c45c7b1ad246 100644 (file)
--- a/lib/Sema/SemaOverload.cpp
+++ b/lib/Sema/SemaOverload.cpp
@@ -768,7 +768,8 @@ bool Sema::IsIntegralPromotion(Expr *From, QualType FromType, QualType ToType) {
   // int can represent all the values of the source type; otherwise,
   // the source rvalue can be converted to an rvalue of type unsigned
   // int (C++ 4.5p1).
-  if (FromType->isPromotableIntegerType() && !FromType->isBooleanType()) {
+  if (FromType->isPromotableIntegerType() && !FromType->isBooleanType() &&
+      !FromType->isEnumeralType()) {
     if (// We can promote any signed, promotable integer type to an int
         (FromType->isSignedIntegerType() ||
          // We can promote any unsigned integer type whose size is

diff --git a/test/CodeGen/enum.c b/test/CodeGen/enum.c
index 771fc6b182e3451612ce82a3835cb364fdca5961..eab32c1025186f38eddfa3b6891ae537c1e4fdb1 100644 (file)
--- a/test/CodeGen/enum.c
+++ b/test/CodeGen/enum.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple i386-unknown-unknown %s -emit-llvm-bc -o - | opt -std-compile-opts | llvm-dis | grep 'ret i32 6'
+// RUN: %clang_cc1 -triple i386-unknown-unknown -x c++ %s -emit-llvm-bc -o - | opt -std-compile-opts | llvm-dis | grep 'ret i32 7'
 
 static enum { foo, bar = 1U } z;
 

diff --git a/test/Sema/format-strings.c b/test/Sema/format-strings.c
index d8f7e4f6931c7def1a5d0fb6f53aa45d9f396f58..5ce3eb036ebfa7514690158eb9ad7753e7172808 100644 (file)
--- a/test/Sema/format-strings.c
+++ b/test/Sema/format-strings.c
@@ -179,3 +179,7 @@ void test_asl(aslclient asl) {
   asl_log(asl, 0, 3, "Error: %m"); // no-warning
   asl_log(asl, 0, 3, "Error: %W"); // expected-warning{{invalid conversion specifier 'W'}}
 }
+
+// <rdar://problem/7595366>
+typedef enum { A } int_t;
+void f0(int_t x) { printf("%d\n", x); }

diff --git a/utils/ABITest/ABITestGen.py b/utils/ABITest/ABITestGen.py
index a31f01043c35d439fe1d36909988f07d931f6c6a..c45a0c343270b5866ef068f19cf6040315f79304 100755 (executable)
--- a/utils/ABITest/ABITestGen.py
+++ b/utils/ABITest/ABITestGen.py
@@ -207,6 +207,9 @@ class TypePrinter:
                 yield '(%s) 0'%(t.name,)
                 yield '(%s) -1'%(t.name,)
                 yield '(%s) 1'%(t.name,)
+        elif isinstance(t, EnumType):
+            for i in range(0, len(t.enumerators)):
+                yield 'enum%dval%d' % (t.index, i)
         elif isinstance(t, RecordType):
             nonPadding = [f for f in t.fields 
                           if not f.isPaddingBitField()]
@@ -273,6 +276,8 @@ class TypePrinter:
             else:
                 code = 'p'
             print >>output, '%*sprintf("%s: %s = %%%s\\n", %s);'%(indent, '', prefix, name, code, name) 
+        elif isinstance(t, EnumType):
+            print >>output, '%*sprintf("%s: %s = %%d\\n", %s);'%(indent, '', prefix, name, name)
         elif isinstance(t, RecordType):
             if not t.fields:
                 print >>output, '%*sprintf("%s: %s (empty)\\n");'%(indent, '', prefix, name) 
@@ -301,6 +306,8 @@ class TypePrinter:
             output = self.output
         if isinstance(t, BuiltinType):
             print >>output, '%*sassert(%s == %s);' % (indent, '', nameLHS, nameRHS)
+        elif isinstance(t, EnumType):
+            print >>output, '%*sassert(%s == %s);' % (indent, '', nameLHS, nameRHS)
         elif isinstance(t, RecordType):
             for i,f in enumerate(t.fields):
                 if f.isPaddingBitField():
@@ -403,6 +410,11 @@ def main():
                      help="do not generate void* types",
                      action="store_false", default=True)
 
+    # Enumerations
+    group.add_option("", "--no-enums", dest="useEnum",
+                     help="do not generate enum types",
+                     action="store_false", default=True)
+
     # Derived types
     group.add_option("", "--no-array", dest="useArray",
                      help="do not generate record types",
@@ -530,6 +542,8 @@ def main():
                 vTypes.append(ArrayType(i, True, type, count * type.size))
                 
             atg.addGenerator(FixedTypeGenerator(vTypes))
+        if opts.useEnum:
+            atg.addGenerator(EnumTypeGenerator([None, '-1', '1', '1u'], 1, 4))
 
     if opts.recordMaxDepth is None: 
         # Fully recursive, just avoid top-level arrays.

diff --git a/utils/ABITest/TypeGen.py b/utils/ABITest/TypeGen.py
index d5678db6a0e8017c4ff617e54a2fdc66b958334f..40ea791eb5157bbf53e9d4df3a2a249f733ce602 100644 (file)
--- a/utils/ABITest/TypeGen.py
+++ b/utils/ABITest/TypeGen.py
@@ -46,6 +46,28 @@ class BuiltinType(Type):
     def __str__(self):
         return self.name
 
+class EnumType(Type):
+    def __init__(self, index, enumerators):
+        self.index = index
+        self.enumerators = enumerators
+
+    def getEnumerators(self):
+        result = ''
+        for i, init in enumerate(self.enumerators):
+            if i > 0:
+                result = result + ', '
+            result = result + 'enum%dval%d' % (self.index, i)
+            if init:
+                result = result + ' = %s' % (init)
+
+        return result
+
+    def __str__(self):
+        return 'enum { %s }' % (self.getEnumerators())
+
+    def getTypedefDef(self, name, printer):
+        return 'typedef enum %s { %s } %s;'%(name, self.getEnumerators(), name)
+
 class RecordType(Type):
     def __init__(self, index, isUnion, fields):
         self.index = index
@@ -188,6 +210,63 @@ class FixedTypeGenerator(TypeGenerator):
     def generateType(self, N):
         return self.types[N]
 
+# Factorial
+def fact(n):
+    result = 1
+    while n > 0:
+        result = result * n
+        n = n - 1
+    return result
+
+# Compute the number of combinations (n choose k)
+def num_combinations(n, k): 
+    return fact(n) / (fact(k) * fact(n - k))
+
+# Enumerate the combinations choosing k elements from the list of values
+def combinations(values, k):
+    # From ActiveState Recipe 190465: Generator for permutations,
+    # combinations, selections of a sequence
+    if k==0: yield []
+    else:
+        for i in xrange(len(values)-k+1):
+            for cc in combinations(values[i+1:],k-1):
+                yield [values[i]]+cc
+
+class EnumTypeGenerator(TypeGenerator):
+    def __init__(self, values, minEnumerators, maxEnumerators):
+        TypeGenerator.__init__(self)
+        self.values = values
+        self.minEnumerators = minEnumerators
+        self.maxEnumerators = maxEnumerators
+        self.setCardinality()
+
+    def setCardinality(self):
+        self.cardinality = 0
+        for num in range(self.minEnumerators, self.maxEnumerators + 1):
+            self.cardinality += num_combinations(len(self.values), num)
+
+    def generateType(self, n):
+        # Figure out the number of enumerators in this type
+        numEnumerators = self.minEnumerators
+        valuesCovered = 0
+        while numEnumerators < self.maxEnumerators:
+            comb = num_combinations(len(self.values), numEnumerators)
+            if valuesCovered + comb > n:
+                break
+            numEnumerators = numEnumerators + 1
+            valuesCovered += comb
+
+        # Find the requested combination of enumerators and build a
+        # type from it.
+        i = 0
+        for enumerators in combinations(self.values, numEnumerators):
+            if i == n - valuesCovered:
+                return EnumType(n, enumerators)
+                
+            i = i + 1
+
+        assert False
+
 class ComplexTypeGenerator(TypeGenerator):
     def __init__(self, typeGen):
         TypeGenerator.__init__(self)
@@ -363,10 +442,12 @@ def test():
 
     btg = FixedTypeGenerator([BuiltinType('char', 4),
                               BuiltinType('int',  4)])
-    
+    etg = EnumTypeGenerator([None, '-1', '1', '1u'], 0, 3)
+
     atg = AnyTypeGenerator()
     atg.addGenerator( btg )
     atg.addGenerator( RecordTypeGenerator(fields0, False, 4) )
+    atg.addGenerator( etg )
     print 'Cardinality:',atg.cardinality
     for i in range(100):
         if i == atg.cardinality: