Constant expression evaluation refactoring:

 - Remodel Expr::EvaluateAsInt to behave like the other EvaluateAs* functions,
   and add Expr::EvaluateKnownConstInt to capture the current fold-or-assert
   behaviour.
 - Factor out evaluation of bitfield bit widths.
 - Fix a few places which would evaluate an expression twice: once to determine
   whether it is a constant expression, then again to get the value.


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

8 files changed, 57 insertions, 64 deletions

diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp
index 3377799e3d..b625655e45 100644
--- a/lib/AST/ASTContext.cpp
+++ b/lib/AST/ASTContext.cpp
@@ -608,33 +608,33 @@ void ASTContext::setInstantiatedFromUnnamedFieldDecl(FieldDecl *Inst,
 bool ASTContext::ZeroBitfieldFollowsNonBitfield(const FieldDecl *FD, 
                                     const FieldDecl *LastFD) const {
   return (FD->isBitField() && LastFD && !LastFD->isBitField() &&
-          FD->getBitWidth()->EvaluateAsInt(*this).getZExtValue() == 0);
+          FD->getBitWidthValue(*this) == 0);
 }
 
 bool ASTContext::ZeroBitfieldFollowsBitfield(const FieldDecl *FD,
                                              const FieldDecl *LastFD) const {
   return (FD->isBitField() && LastFD && LastFD->isBitField() &&
-          FD->getBitWidth()->EvaluateAsInt(*this).getZExtValue() == 0 &&
-          LastFD->getBitWidth()->EvaluateAsInt(*this).getZExtValue() != 0);
+          FD->getBitWidthValue(*this) == 0 &&
+          LastFD->getBitWidthValue(*this) != 0);
 }
 
 bool ASTContext::BitfieldFollowsBitfield(const FieldDecl *FD,
                                          const FieldDecl *LastFD) const {
   return (FD->isBitField() && LastFD && LastFD->isBitField() &&
-          FD->getBitWidth()->EvaluateAsInt(*this).getZExtValue() &&
-          LastFD->getBitWidth()->EvaluateAsInt(*this).getZExtValue());
+          FD->getBitWidthValue(*this) &&
+          LastFD->getBitWidthValue(*this));
 }
 
 bool ASTContext::NonBitfieldFollowsBitfield(const FieldDecl *FD,
                                          const FieldDecl *LastFD) const {
   return (!FD->isBitField() && LastFD && LastFD->isBitField() &&
-          LastFD->getBitWidth()->EvaluateAsInt(*this).getZExtValue());  
+          LastFD->getBitWidthValue(*this));
 }
 
 bool ASTContext::BitfieldFollowsNonBitfield(const FieldDecl *FD,
                                              const FieldDecl *LastFD) const {
   return (FD->isBitField() && LastFD && !LastFD->isBitField() &&
-          FD->getBitWidth()->EvaluateAsInt(*this).getZExtValue());  
+          FD->getBitWidthValue(*this));
 }
 
 ASTContext::overridden_cxx_method_iterator
@@ -3577,8 +3577,7 @@ QualType ASTContext::isPromotableBitField(Expr *E) const {
 
   QualType FT = Field->getType();
 
-  llvm::APSInt BitWidthAP = Field->getBitWidth()->EvaluateAsInt(*this);
-  uint64_t BitWidth = BitWidthAP.getZExtValue();
+  uint64_t BitWidth = Field->getBitWidthValue(*this);
   uint64_t IntSize = getTypeSize(IntTy);
   // GCC extension compatibility: if the bit-field size is less than or equal
   // to the size of int, it gets promoted no matter what its type is.
@@ -4255,8 +4254,7 @@ static char ObjCEncodingForEnumType(const ASTContext *C, const EnumType *ET) {
 
 static void EncodeBitField(const ASTContext *Ctx, std::string& S,
                            QualType T, const FieldDecl *FD) {
-  const Expr *E = FD->getBitWidth();
-  assert(E && "bitfield width not there - getObjCEncodingForTypeImpl");
+  assert(FD->isBitField() && "not a bitfield - getObjCEncodingForTypeImpl");
   S += 'b';
   // The NeXT runtime encodes bit fields as b followed by the number of bits.
   // The GNU runtime requires more information; bitfields are encoded as b,
@@ -4282,8 +4280,7 @@ static void EncodeBitField(const ASTContext *Ctx, std::string& S,
     else
       S += ObjCEncodingForPrimitiveKind(Ctx, T);
   }
-  unsigned N = E->EvaluateAsInt(*Ctx).getZExtValue();
-  S += llvm::utostr(N);
+  S += llvm::utostr(FD->getBitWidthValue(*Ctx));
 }
 
 // FIXME: Use SmallString for accumulating string.
@@ -4699,7 +4696,7 @@ void ASTContext::getObjCEncodingForStructureImpl(RecordDecl *RDecl,
 
       if (field->isBitField()) {
         EncodeBitField(this, S, field->getType(), field);
-        CurOffs += field->getBitWidth()->EvaluateAsInt(*this).getZExtValue();
+        CurOffs += field->getBitWidthValue(*this);
       } else {
         QualType qt = field->getType();
         getLegacyIntegralTypeEncoding(qt);
diff --git a/lib/AST/ASTImporter.cpp b/lib/AST/ASTImporter.cpp
index 050a9ab2c6..2f7497db66 100644
--- a/lib/AST/ASTImporter.cpp
+++ b/lib/AST/ASTImporter.cpp
@@ -956,43 +956,33 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
       Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
         << Context.C2.getTypeDeclType(D2);
       if (Field1->isBitField()) {
-        llvm::APSInt Bits;
-        Field1->getBitWidth()->isIntegerConstantExpr(Bits, Context.C1);
         Context.Diag1(Field1->getLocation(), diag::note_odr_bit_field)
           << Field1->getDeclName() << Field1->getType()
-          << Bits.toString(10, false);
+          << Field1->getBitWidthValue(Context.C1);
         Context.Diag2(Field2->getLocation(), diag::note_odr_not_bit_field)
           << Field2->getDeclName();
       } else {
-        llvm::APSInt Bits;
-        Field2->getBitWidth()->isIntegerConstantExpr(Bits, Context.C2);
         Context.Diag2(Field2->getLocation(), diag::note_odr_bit_field)
           << Field2->getDeclName() << Field2->getType()
-          << Bits.toString(10, false);
-        Context.Diag1(Field1->getLocation(), 
-                          diag::note_odr_not_bit_field)
-        << Field1->getDeclName();
+          << Field2->getBitWidthValue(Context.C2);
+        Context.Diag1(Field1->getLocation(), diag::note_odr_not_bit_field)
+          << Field1->getDeclName();
       }
       return false;
     }
     
     if (Field1->isBitField()) {
       // Make sure that the bit-fields are the same length.
-      llvm::APSInt Bits1, Bits2;
-      if (!Field1->getBitWidth()->isIntegerConstantExpr(Bits1, Context.C1))
-        return false;
-      if (!Field2->getBitWidth()->isIntegerConstantExpr(Bits2, Context.C2))
-        return false;
+      unsigned Bits1 = Field1->getBitWidthValue(Context.C1);
+      unsigned Bits2 = Field2->getBitWidthValue(Context.C2);
       
-      if (!IsSameValue(Bits1, Bits2)) {
+      if (Bits1 != Bits2) {
         Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
           << Context.C2.getTypeDeclType(D2);
         Context.Diag2(Field2->getLocation(), diag::note_odr_bit_field)
-          << Field2->getDeclName() << Field2->getType()
-          << Bits2.toString(10, false);
+          << Field2->getDeclName() << Field2->getType() << Bits2;
         Context.Diag1(Field1->getLocation(), diag::note_odr_bit_field)
-          << Field1->getDeclName() << Field1->getType()
-          << Bits1.toString(10, false);
+          << Field1->getDeclName() << Field1->getType() << Bits1;
         return false;
       }
     }
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index 7d3390f133..5cd98584bf 100644
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -2187,6 +2187,12 @@ bool FieldDecl::isAnonymousStructOrUnion() const {
   return false;
 }
 
+unsigned FieldDecl::getBitWidthValue(const ASTContext &Ctx) const {
+  assert(isBitField() && "not a bitfield");
+  Expr *BitWidth = InitializerOrBitWidth.getPointer();
+  return BitWidth->EvaluateKnownConstInt(Ctx).getZExtValue();
+}
+
 unsigned FieldDecl::getFieldIndex() const {
   if (CachedFieldIndex) return CachedFieldIndex - 1;
 
diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp
index 5dcecb9a11..8f61ea23cd 100644
--- a/lib/AST/DeclCXX.cpp
+++ b/lib/AST/DeclCXX.cpp
@@ -830,15 +830,11 @@ NotASpecialMember:;
 
     // If this is not a zero-length bit-field, then the class is not empty.
     if (data().Empty) {
-      if (!Field->getBitWidth())
+      if (!Field->isBitField() ||
+          (!Field->getBitWidth()->isTypeDependent() &&
+           !Field->getBitWidth()->isValueDependent() &&
+           Field->getBitWidthValue(Context) != 0))
         data().Empty = false;
-      else if (!Field->getBitWidth()->isTypeDependent() &&
-               !Field->getBitWidth()->isValueDependent()) {
-        llvm::APSInt Bits;
-        if (Field->getBitWidth()->isIntegerConstantExpr(Bits, Context))
-          if (!!Bits)
-            data().Empty = false;
-      } 
     }
   }
   
diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
index 7cfbd9e2fb..dc37ac9226 100644
--- a/lib/AST/Expr.cpp
+++ b/lib/AST/Expr.cpp
@@ -2955,7 +2955,7 @@ StringRef ObjCBridgedCastExpr::getBridgeKindName() const {
 }
 
 bool ChooseExpr::isConditionTrue(const ASTContext &C) const {
-  return getCond()->EvaluateAsInt(C) != 0;
+  return getCond()->EvaluateKnownConstInt(C) != 0;
 }
 
 ShuffleVectorExpr::ShuffleVectorExpr(ASTContext &C, Expr **args, unsigned nexpr,
diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp
index 742dcab564..3da744955a 100644
--- a/lib/AST/ExprConstant.cpp
+++ b/lib/AST/ExprConstant.cpp
@@ -1285,7 +1285,7 @@ bool IntExprEvaluator::VisitCallExpr(const CallExpr *E) {
     // If evaluating the argument has side-effects we can't determine
     // the size of the object and lower it to unknown now.
     if (E->getArg(0)->HasSideEffects(Info.Ctx)) {
-      if (E->getArg(1)->EvaluateAsInt(Info.Ctx).getZExtValue() <= 1)
+      if (E->getArg(1)->EvaluateKnownConstInt(Info.Ctx).getZExtValue() <= 1)
         return Success(-1ULL, E);
       return Success(0, E);
     }
@@ -1302,7 +1302,7 @@ bool IntExprEvaluator::VisitCallExpr(const CallExpr *E) {
     return Success(E->getArg(0)->isEvaluatable(Info.Ctx), E);
       
   case Builtin::BI__builtin_eh_return_data_regno: {
-    int Operand = E->getArg(0)->EvaluateAsInt(Info.Ctx).getZExtValue();
+    int Operand = E->getArg(0)->EvaluateKnownConstInt(Info.Ctx).getZExtValue();
     Operand = Info.Ctx.getTargetInfo().getEHDataRegisterNumber(Operand);
     return Success(Operand, E);
   }
@@ -2681,6 +2681,13 @@ bool Expr::EvaluateAsBooleanCondition(bool &Result,
   return HandleConversionToBool(this, Result, Info);
 }
 
+bool Expr::EvaluateAsInt(APSInt &Result, const ASTContext &Ctx) const {
+  EvalResult Scratch;
+  EvalInfo Info(Ctx, Scratch);
+
+  return EvaluateInteger(this, Result, Info) && !Scratch.HasSideEffects;
+}
+
 bool Expr::EvaluateAsLValue(EvalResult &Result, const ASTContext &Ctx) const {
   EvalInfo Info(Ctx, Result);
 
@@ -2719,7 +2726,7 @@ bool Expr::HasSideEffects(const ASTContext &Ctx) const {
   return HasSideEffect(Info).Visit(this);
 }
 
-APSInt Expr::EvaluateAsInt(const ASTContext &Ctx) const {
+APSInt Expr::EvaluateKnownConstInt(const ASTContext &Ctx) const {
   EvalResult EvalResult;
   bool Result = Evaluate(EvalResult, Ctx);
   (void)Result;
@@ -3021,11 +3028,11 @@ static ICEDiag CheckICE(const Expr* E, ASTContext &Ctx) {
         // Evaluate gives an error for undefined Div/Rem, so make sure
         // we don't evaluate one.
         if (LHSResult.Val == 0 && RHSResult.Val == 0) {
-          llvm::APSInt REval = Exp->getRHS()->EvaluateAsInt(Ctx);
+          llvm::APSInt REval = Exp->getRHS()->EvaluateKnownConstInt(Ctx);
           if (REval == 0)
             return ICEDiag(1, E->getLocStart());
           if (REval.isSigned() && REval.isAllOnesValue()) {
-            llvm::APSInt LEval = Exp->getLHS()->EvaluateAsInt(Ctx);
+            llvm::APSInt LEval = Exp->getLHS()->EvaluateKnownConstInt(Ctx);
             if (LEval.isMinSignedValue())
               return ICEDiag(1, E->getLocStart());
           }
@@ -3056,11 +3063,11 @@ static ICEDiag CheckICE(const Expr* E, ASTContext &Ctx) {
       //   evaluated are not considered.
       if (Ctx.getLangOptions().CPlusPlus0x && LHSResult.Val == 0) {
         if (Exp->getOpcode() == BO_LAnd && 
-            Exp->getLHS()->EvaluateAsInt(Ctx) == 0)
+            Exp->getLHS()->EvaluateKnownConstInt(Ctx) == 0)
           return LHSResult;
 
         if (Exp->getOpcode() == BO_LOr &&
-            Exp->getLHS()->EvaluateAsInt(Ctx) != 0)
+            Exp->getLHS()->EvaluateKnownConstInt(Ctx) != 0)
           return LHSResult;
       }
 
@@ -3070,7 +3077,7 @@ static ICEDiag CheckICE(const Expr* E, ASTContext &Ctx) {
         // to actually check the condition to see whether the side
         // with the comma is evaluated.
         if ((Exp->getOpcode() == BO_LAnd) !=
-            (Exp->getLHS()->EvaluateAsInt(Ctx) == 0))
+            (Exp->getLHS()->EvaluateKnownConstInt(Ctx) == 0))
           return RHSResult;
         return NoDiag();
       }
@@ -3109,7 +3116,7 @@ static ICEDiag CheckICE(const Expr* E, ASTContext &Ctx) {
     if (FalseResult.Val == 2) return FalseResult;
     if (CommonResult.Val == 1) return CommonResult;
     if (FalseResult.Val == 1 &&
-        Exp->getCommon()->EvaluateAsInt(Ctx) == 0) return NoDiag();
+        Exp->getCommon()->EvaluateKnownConstInt(Ctx) == 0) return NoDiag();
     return FalseResult;
   }
   case Expr::ConditionalOperatorClass: {
@@ -3136,7 +3143,7 @@ static ICEDiag CheckICE(const Expr* E, ASTContext &Ctx) {
     //   subexpressions of [...] conditional (5.16) operations that
     //   are not evaluated are not considered
     bool TrueBranch = Ctx.getLangOptions().CPlusPlus0x
-      ? Exp->getCond()->EvaluateAsInt(Ctx) != 0
+      ? Exp->getCond()->EvaluateKnownConstInt(Ctx) != 0
       : false;
     ICEDiag TrueResult = NoDiag();
     if (!Ctx.getLangOptions().CPlusPlus0x || TrueBranch)
@@ -3156,7 +3163,7 @@ static ICEDiag CheckICE(const Expr* E, ASTContext &Ctx) {
     // Rare case where the diagnostics depend on which side is evaluated
     // Note that if we get here, CondResult is 0, and at least one of
     // TrueResult and FalseResult is non-zero.
-    if (Exp->getCond()->EvaluateAsInt(Ctx) == 0) {
+    if (Exp->getCond()->EvaluateKnownConstInt(Ctx) == 0) {
       return FalseResult;
     }
     return TrueResult;
diff --git a/lib/AST/ItaniumMangle.cpp b/lib/AST/ItaniumMangle.cpp
index 41cfa6ad01..2b93250fad 100644
--- a/lib/AST/ItaniumMangle.cpp
+++ b/lib/AST/ItaniumMangle.cpp
@@ -2416,7 +2416,8 @@ recurse:
       QualType T = (ImplicitlyConvertedToType.isNull() || 
                     !ImplicitlyConvertedToType->isIntegerType())? SAE->getType()
                                                     : ImplicitlyConvertedToType;
-      mangleIntegerLiteral(T, SAE->EvaluateAsInt(Context.getASTContext()));
+      llvm::APSInt V = SAE->EvaluateKnownConstInt(Context.getASTContext());
+      mangleIntegerLiteral(T, V);
       break;
     }
     
diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp
index f351d76d76..a3ccbd0850 100644
--- a/lib/AST/RecordLayoutBuilder.cpp
+++ b/lib/AST/RecordLayoutBuilder.cpp
@@ -1362,7 +1362,7 @@ void RecordLayoutBuilder::LayoutFields(const RecordDecl *D) {
         if (TypeSizeLastFD != TypeSize) {
           if (RemainingInAlignment &&
               LastFD && LastFD->isBitField() &&
-              LastFD->getBitWidth()->EvaluateAsInt(Context).getZExtValue()) {
+              LastFD->getBitWidthValue(Context)) {
             // If previous field was a bitfield with some remaining unfilled
             // bits, pad the field so current field starts on its type boundary.
             uint64_t FieldOffset = 
@@ -1393,8 +1393,7 @@ void RecordLayoutBuilder::LayoutFields(const RecordDecl *D) {
           setSize(std::max(getSizeInBits(), getDataSizeInBits()));
         }
         if (FD->isBitField()) {
-          uint64_t FieldSize = 
-            FD->getBitWidth()->EvaluateAsInt(Context).getZExtValue();
+          uint64_t FieldSize = FD->getBitWidthValue(Context);
           assert (FieldSize > 0 && "LayoutFields - ms_struct layout");
           if (RemainingInAlignment < FieldSize)
             RemainingInAlignment = TypeSize - FieldSize;
@@ -1403,8 +1402,7 @@ void RecordLayoutBuilder::LayoutFields(const RecordDecl *D) {
         }
       }
       else if (FD->isBitField()) {
-        uint64_t FieldSize = 
-          FD->getBitWidth()->EvaluateAsInt(Context).getZExtValue();
+        uint64_t FieldSize = FD->getBitWidthValue(Context);
         std::pair<uint64_t, unsigned> FieldInfo = 
           Context.getTypeInfo(FD->getType());
         uint64_t TypeSize = FieldInfo.first;
@@ -1415,15 +1413,13 @@ void RecordLayoutBuilder::LayoutFields(const RecordDecl *D) {
     else if (!Context.getTargetInfo().useBitFieldTypeAlignment() &&
              Context.getTargetInfo().useZeroLengthBitfieldAlignment()) {             
       FieldDecl *FD =  (*Field);
-      if (FD->isBitField() && 
-          FD->getBitWidth()->EvaluateAsInt(Context).getZExtValue() == 0)
+      if (FD->isBitField() && FD->getBitWidthValue(Context) == 0)
         ZeroLengthBitfield = FD;
     }
     LayoutField(*Field);
   }
   if (IsMsStruct && RemainingInAlignment &&
-      LastFD && LastFD->isBitField() &&
-      LastFD->getBitWidth()->EvaluateAsInt(Context).getZExtValue()) {
+      LastFD && LastFD->isBitField() && LastFD->getBitWidthValue(Context)) {
     // If we ended a bitfield before the full length of the type then
     // pad the struct out to the full length of the last type.
     uint64_t FieldOffset = 
@@ -1504,7 +1500,7 @@ void RecordLayoutBuilder::LayoutBitField(const FieldDecl *D) {
   bool FieldPacked = Packed || D->hasAttr<PackedAttr>();
   uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastByte;
   uint64_t FieldOffset = IsUnion ? 0 : UnpaddedFieldOffset;
-  uint64_t FieldSize = D->getBitWidth()->EvaluateAsInt(Context).getZExtValue();
+  uint64_t FieldSize = D->getBitWidthValue(Context);
 
   std::pair<uint64_t, unsigned> FieldInfo = Context.getTypeInfo(D->getType());
   uint64_t TypeSize = FieldInfo.first;