[Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

[ahatanak]

Summary

The assertion fails when a function with a default argument that materializes a temporary is called more than once in an expression. The assertion fails in CallStackFrame::createTemporary when it searches map Temporaries using the default argument's expression (which is a MaterializeTemporaryExpr) as the key and it discovers that there is already an element with that key that has been initialized.

constexpr bool equals(const float& arg = 1.0f) {
  return arg == 1.0f;
}

constexpr bool test_default_arg() {
  return equals() && equals();
}

This patch removes the assertion and makes CallStackFrame::createTemporary reset the existing element and return it if the element is found in the map.

rdar://problem/36505742

[lebedev.ri]

test/SemaCXX/constexpr-default-arg.cpp

3 ↗

(On Diff #132300)

Down the line, it won't be obvious *what* this testcase is checking.
At the very least wrap it into namespace rdar_problem_36505742 {}

[erik.pilkington]

Hi Akira, thanks for working on this!

lib/AST/ExprConstant.cpp

1165–1173 ↗

(On Diff #132300)

I think that the problem is more subtle than this. This static assert errors (previously clang would assert) when it really it should be fine.

constexpr const int &x(const int &p = 0) { return p; }
static_assert(&x() != &x());

Because default arguments are allocated on the caller side, both the calls to x() call createTemporary for the same MaterializeTemporaryExpr in the same CallStackFrame, when really that MTE should correspond to two distinct values. This patch just hides that underlying problem by recycling the value created during the first call during the second call.

Maybe we could have a fancier key that incorporates a node on the caller side, such as the CXXDefaultArgExpr as well at the MTE, and store that fancy key in APValue::LValueBases? That would allow us generate distinct values for these MTEs, and also remember what expression originated it. What do you think about that?

There is small discussion about this problem here: https://bugs.llvm.org/show_bug.cgi?id=33140

[ahatanak]

Address Erik's and Roman's review comments.

lib/AST/ExprConstant.cpp

1165–1173 ↗

(On Diff #132300)

Thank you Erik for explaining the problem and pointing me to the PR.

I ended up using a version number that is updated every time VisitCXXDefaultArgExpr is called. I initially tried using CXXDefaultArgExpr and the void* pointer as the key, but discovered that that wouldn't fix the assertion failure when compiling the first test case in PR33140 since InitListExpr uses the same CXXConstructExpr (and hence the same CXXDefaultArgExpr) to initialize the array.

Let me know if there are other cases I haven't thought about.

test/SemaCXX/constexpr-default-arg.cpp

3 ↗

(On Diff #132300)

I added comments that explain what it's trying to check.

[rsmith]

lib/AST/APValue.cpp

27 ↗

(On Diff #133947)

Move this to the end so it can share space with IsNullPtr.

lib/AST/ExprConstant.cpp

589–598 ↗

(On Diff #133947)

This seems extremely similar to the purpose of the existing CallIndex parameter. Have you thought about ways the two might be unified?

If there's no reasonable path to unifying the two (which I suspect there may not be), it would make more sense to me to store a current version number on the CallStackFrame rather than globally in the EvalInfo, and to restart the numbering in each new call frame. That also emphasizes something else: this version number should be kept with the CallIndex. (We could achieve that by either moving the CallIndex into the LValueBase or moving the Version out of it.)

597 ↗

(On Diff #133947)

This is wrong: these scopes can nest, so you can't just reset the number to 0 when you're done. You should restore the prior number when you're done here, to divide up evaluation into CXXDefaultArgExpr scopes. (Technically, I think it would also be correct to leave the number alone when you leave one of these scopes, but only because a scope for a particular parameter's default argument can't nest within another scope for the same default argument expression -- and even that might not be true in the presence of template instantiation.)

4595 ↗

(On Diff #133947)

We need the same handling for CXXDefaultInitExpr, too.

[ahatanak]

lib/AST/ExprConstant.cpp

597 ↗

(On Diff #133947)

Do you mean VisitCXXDefaultArgExpr can be called the second time before the first call returns? Do you have an example code that would cause that to happen (which I can perhaps add as a test case)?

It seemed to me that that would happen only if you used a lambda expression for the default argument, but I thought the current standard doesn't allow using lambda expressions in constant expressions.

[ahatanak]

OK, I see. It's pretty easy to come up with an example.

constexpr int foo1(int a = 12) {
  return a * a;
}

constexpr int foo2(int a = foo1()) {
  return a - 12;
}

[ahatanak]

Address review comments.

lib/AST/ExprConstant.cpp

589–598 ↗

(On Diff #133947)

It couldn't come up with a way to unify CallIndex and Version. I think it's better to keep them separate because you need the CallIndex to get the frame that owns the temporary and you need Version to distinguish between two temporaries created by the same MTE.

I moved CallIndex into LValueBase as you suggested.

[ahatanak]

ping

[rsmith]

Thank you, this looks like a great direction.

As noted, there are a bunch of other cases that we should cover with this approach. I'm really happy about the number of related bugs we get to fix with this change.

lib/AST/ExprConstant.cpp

3186–3187 ↗

(On Diff #134560)

This should take the version into account.

5236 ↗

(On Diff #134560)

Hmm. We should be versioning local variables as well. Currently we'll accept invalid code such as:

constexpr int f() {
  int *p = nullptr;
  for (int k = 0; k != 2; ++k) {
    int local_var = 0;
    if (k == 0)
      p = &local_var;
    else
      return *p;
  }
}
static_assert(f() == 0);

5275–5278 ↗

(On Diff #134560)

Can you combine these, so we have a single function to create a temporary and produce both an LValue denoting it and an APValue* to hold its evaluated value?

5772 ↗

(On Diff #134560)

This should create a versioned temporary object.

6540 ↗

(On Diff #134560)

This should create a versioned temporary object.

8031 ↗

(On Diff #134560)

You already checked this above. It'd make sense to check the call index and version in the same place here, but we should only need one check for each :)

9974–9997 ↗

(On Diff #134560)

These temporaries should all be versioned.

[ahatanak]

Address review comments.

lib/AST/ExprConstant.cpp

5236 ↗

(On Diff #134560)

I made changes to the constructor and destructor of ScopeRAII so that a version is pushed at the start of a new iteration. This fixes the FIXME in test/SemaCXX/constant-expression-cxx1y.cpp.

[ahatanak]

Assign an uninitialized APValue to a temporary instead of removing it from the temporary map when the temporary's lifetime has ended.

[rsmith]

I think this can be cleaned up a bit further, but I'm fine with that happening after the patch lands. Thanks!

lib/AST/ExprConstant.cpp

455–456 ↗

(On Diff #139797)

Have you considered using a pair<const void*, unsigned> as the key rather than a map-of-maps? It'd be preferable to only allocate one heap node rather than two for the (hopefully common) case where there is only one version of an object.

466 ↗

(On Diff #139797)

If we're using this for variables in loops too, we should generalize the name and the description to not talk about temporaries.

[ahatanak]

Use pair<const void*, unsigned> as the key.

cfe/trunk/include/clang/AST/APValue.h

enum ValueKind {
     LValue,
     Vector,
     Array,
     Struct,
     Union,
     MemberPointer,
     AddrLabelDiff
   };
-  typedef llvm::PointerUnion<const ValueDecl *, const Expr *> LValueBase;
+
+  class LValueBase {
+  public:
+    typedef llvm::PointerUnion<const ValueDecl *, const Expr *> PtrTy;
+
+    LValueBase() : CallIndex(0), Version(0) {}
+
+    template <class T>
+    LValueBase(T P, unsigned I = 0, unsigned V = 0)
+        : Ptr(P), CallIndex(I), Version(V) {}
+
+    template <class T>
+    bool is() const { return Ptr.is<T>(); }
+
+    template <class T>
+    T get() const { return Ptr.get<T>(); }
+
+    template <class T>
+    T dyn_cast() const { return Ptr.dyn_cast<T>(); }
+
+    void *getOpaqueValue() const;
+
+    bool isNull() const;
+
+    explicit operator bool () const;
+
+    PtrTy getPointer() const {
+      return Ptr;
+    }
+
+    unsigned getCallIndex() const {
+      return CallIndex;
+    }
+
+    void setCallIndex(unsigned Index) {
+      CallIndex = Index;
+    }
+
+    unsigned getVersion() const {
+      return Version;
+    }
+
+    bool operator==(const LValueBase &Other) const {
+      return Ptr == Other.Ptr && CallIndex == Other.CallIndex &&
+             Version == Other.Version;
+    }
+
+  private:
+    PtrTy Ptr;
+    unsigned CallIndex, Version;
+  };
+
   typedef llvm::PointerIntPair<const Decl *, 1, bool> BaseOrMemberType;
   union LValuePathEntry {
     /// BaseOrMember - The FieldDecl or CXXRecordDecl indicating the next item
     /// in the path. An opaque value of type BaseOrMemberType.
     void *BaseOrMember;
     /// ArrayIndex - The array index of the next item in the path.
     uint64_t ArrayIndex;
   };
public:
   APValue(APSInt R, APSInt I) : Kind(Uninitialized) {
     MakeComplexInt(); setComplexInt(std::move(R), std::move(I));
   }
   APValue(APFloat R, APFloat I) : Kind(Uninitialized) {
     MakeComplexFloat(); setComplexFloat(std::move(R), std::move(I));
   }
   APValue(const APValue &RHS);
   APValue(APValue &&RHS) : Kind(Uninitialized) { swap(RHS); }
-  APValue(LValueBase B, const CharUnits &O, NoLValuePath N, unsigned CallIndex,
+  APValue(LValueBase B, const CharUnits &O, NoLValuePath N,
           bool IsNullPtr = false)
       : Kind(Uninitialized) {
-    MakeLValue(); setLValue(B, O, N, CallIndex, IsNullPtr);
+    MakeLValue(); setLValue(B, O, N, IsNullPtr);
   }
   APValue(LValueBase B, const CharUnits &O, ArrayRef<LValuePathEntry> Path,
-          bool OnePastTheEnd, unsigned CallIndex, bool IsNullPtr = false)
+          bool OnePastTheEnd, bool IsNullPtr = false)
       : Kind(Uninitialized) {
-    MakeLValue(); setLValue(B, O, Path, OnePastTheEnd, CallIndex, IsNullPtr);
+    MakeLValue(); setLValue(B, O, Path, OnePastTheEnd, IsNullPtr);
   }
   APValue(UninitArray, unsigned InitElts, unsigned Size) : Kind(Uninitialized) {
     MakeArray(InitElts, Size);
   }
   APValue(UninitStruct, unsigned B, unsigned M) : Kind(Uninitialized) {
     MakeStruct(B, M);
   }
   explicit APValue(const FieldDecl *D, const APValue &V = APValue())
public:
   CharUnits &getLValueOffset();
   const CharUnits &getLValueOffset() const {
     return const_cast<APValue*>(this)->getLValueOffset();
   }
   bool isLValueOnePastTheEnd() const;
   bool hasLValuePath() const;
   ArrayRef<LValuePathEntry> getLValuePath() const;
   unsigned getLValueCallIndex() const;
+  unsigned getLValueVersion() const;
   bool isNullPointer() const;
 
   APValue &getVectorElt(unsigned I) {
     assert(isVector() && "Invalid accessor");
     assert(I < getVectorLength() && "Index out of range");
     return ((Vec*)(char*)Data.buffer)->Elts[I];
   }
   const APValue &getVectorElt(unsigned I) const {
public:
   void setComplexFloat(APFloat R, APFloat I) {
     assert(&R.getSemantics() == &I.getSemantics() &&
            "Invalid complex float (type mismatch).");
     assert(isComplexFloat() && "Invalid accessor");
     ((ComplexAPFloat *)(char *)Data.buffer)->Real = std::move(R);
     ((ComplexAPFloat *)(char *)Data.buffer)->Imag = std::move(I);
   }
   void setLValue(LValueBase B, const CharUnits &O, NoLValuePath,
-                 unsigned CallIndex, bool IsNullPtr);
+                 bool IsNullPtr);
   void setLValue(LValueBase B, const CharUnits &O,
                  ArrayRef<LValuePathEntry> Path, bool OnePastTheEnd,
-                 unsigned CallIndex, bool IsNullPtr);
+                 bool IsNullPtr);
   void setUnion(const FieldDecl *Field, const APValue &Value) {
     assert(isUnion() && "Invalid accessor");
     ((UnionData*)(char*)Data.buffer)->Field = Field;
     *((UnionData*)(char*)Data.buffer)->Value = Value;
   }
   void setAddrLabelDiff(const AddrLabelExpr* LHSExpr,
                         const AddrLabelExpr* RHSExpr) {
     ((AddrLabelDiffData*)(char*)Data.buffer)->LHSExpr = LHSExpr;
void MakeAddrLabelDiff() {
     assert(isUninit() && "Bad state change");
     new ((void*)(char*)Data.buffer) AddrLabelDiffData();
     Kind = AddrLabelDiff;
   }
 };
 
 } // end namespace clang.
 
+namespace llvm {
+template<> struct DenseMapInfo<clang::APValue::LValueBase> {
+  static clang::APValue::LValueBase getEmptyKey();
+  static clang::APValue::LValueBase getTombstoneKey();
+  static unsigned getHashValue(const clang::APValue::LValueBase &Base);
+  static bool isEqual(const clang::APValue::LValueBase &LHS,
+                      const clang::APValue::LValueBase &RHS);
+};
+}
+
 #endif

cfe/trunk/lib/AST/APValue.cpp

 #include "clang/AST/Expr.h"
 #include "clang/AST/Type.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace clang;
 
 namespace {
   struct LVBase {
-    llvm::PointerIntPair<APValue::LValueBase, 1, bool> BaseAndIsOnePastTheEnd;
+    APValue::LValueBase Base;
     CharUnits Offset;
     unsigned PathLength;
-    unsigned CallIndex;
-    bool IsNullPtr;
+    bool IsNullPtr : 1;
+    bool IsOnePastTheEnd : 1;
   };
 }
 
+void *APValue::LValueBase::getOpaqueValue() const {
+  return Ptr.getOpaqueValue();
+}
+
+bool APValue::LValueBase::isNull() const {
+  return Ptr.isNull();
+}
+
+APValue::LValueBase::operator bool () const {
+  return static_cast<bool>(Ptr);
+}
+
+clang::APValue::LValueBase
+llvm::DenseMapInfo<clang::APValue::LValueBase>::getEmptyKey() {
+  return clang::APValue::LValueBase(
+      DenseMapInfo<clang::APValue::LValueBase::PtrTy>::getEmptyKey(),
+      DenseMapInfo<unsigned>::getEmptyKey(),
+      DenseMapInfo<unsigned>::getEmptyKey());
+}
+
+clang::APValue::LValueBase
+llvm::DenseMapInfo<clang::APValue::LValueBase>::getTombstoneKey() {
+  return clang::APValue::LValueBase(
+      DenseMapInfo<clang::APValue::LValueBase::PtrTy>::getTombstoneKey(),
+      DenseMapInfo<unsigned>::getTombstoneKey(),
+      DenseMapInfo<unsigned>::getTombstoneKey());
+}
+
+unsigned llvm::DenseMapInfo<clang::APValue::LValueBase>::getHashValue(
+    const clang::APValue::LValueBase &Base) {
+  llvm::FoldingSetNodeID ID;
+  ID.AddPointer(Base.getOpaqueValue());
+  ID.AddInteger(Base.getCallIndex());
+  ID.AddInteger(Base.getVersion());
+  return ID.ComputeHash();
+}
+
+bool llvm::DenseMapInfo<clang::APValue::LValueBase>::isEqual(
+    const clang::APValue::LValueBase &LHS,
+    const clang::APValue::LValueBase &RHS) {
+  return LHS == RHS;
+}
+
 struct APValue::LV : LVBase {
   static const unsigned InlinePathSpace =
       (DataSize - sizeof(LVBase)) / sizeof(LValuePathEntry);
 
   /// Path - The sequence of base classes, fields and array indices to follow to
   /// walk from Base to the subobject. When performing GCC-style folding, there
   /// may not be such a path.
   union {
APValue::APValue(const APValue &RHS) : Kind(Uninitialized) {
   case ComplexFloat:
     MakeComplexFloat();
     setComplexFloat(RHS.getComplexFloatReal(), RHS.getComplexFloatImag());
     break;
   case LValue:
     MakeLValue();
     if (RHS.hasLValuePath())
       setLValue(RHS.getLValueBase(), RHS.getLValueOffset(), RHS.getLValuePath(),
-                RHS.isLValueOnePastTheEnd(), RHS.getLValueCallIndex(),
-                RHS.isNullPointer());
+                RHS.isLValueOnePastTheEnd(), RHS.isNullPointer());
     else
       setLValue(RHS.getLValueBase(), RHS.getLValueOffset(), NoLValuePath(),
-                RHS.getLValueCallIndex(), RHS.isNullPointer());
+                RHS.isNullPointer());
     break;
   case Array:
     MakeArray(RHS.getArrayInitializedElts(), RHS.getArraySize());
     for (unsigned I = 0, N = RHS.getArrayInitializedElts(); I != N; ++I)
       getArrayInitializedElt(I) = RHS.getArrayInitializedElt(I);
     if (RHS.hasArrayFiller())
       getArrayFiller() = RHS.getArrayFiller();
     break;
std::string APValue::getAsString(ASTContext &Ctx, QualType Ty) const {
   llvm::raw_string_ostream Out(Result);
   printPretty(Out, Ctx, Ty);
   Out.flush();
   return Result;
 }
 
 const APValue::LValueBase APValue::getLValueBase() const {
   assert(isLValue() && "Invalid accessor");
-  return ((const LV*)(const void*)Data.buffer)->BaseAndIsOnePastTheEnd.getPointer();
+  return ((const LV*)(const void*)Data.buffer)->Base;
 }
 
 bool APValue::isLValueOnePastTheEnd() const {
   assert(isLValue() && "Invalid accessor");
-  return ((const LV*)(const void*)Data.buffer)->BaseAndIsOnePastTheEnd.getInt();
+  return ((const LV*)(const void*)Data.buffer)->IsOnePastTheEnd;
 }
 
 CharUnits &APValue::getLValueOffset() {
   assert(isLValue() && "Invalid accessor");
   return ((LV*)(void*)Data.buffer)->Offset;
 }
 
 bool APValue::hasLValuePath() const {
   assert(isLValue() && "Invalid accessor");
   return ((const LV*)(const char*)Data.buffer)->hasPath();
 }
 
 ArrayRef<APValue::LValuePathEntry> APValue::getLValuePath() const {
   assert(isLValue() && hasLValuePath() && "Invalid accessor");
   const LV &LVal = *((const LV*)(const char*)Data.buffer);
   return llvm::makeArrayRef(LVal.getPath(), LVal.PathLength);
 }
 
 unsigned APValue::getLValueCallIndex() const {
   assert(isLValue() && "Invalid accessor");
-  return ((const LV*)(const char*)Data.buffer)->CallIndex;
+  return ((const LV*)(const char*)Data.buffer)->Base.getCallIndex();
+}
+
+unsigned APValue::getLValueVersion() const {
+  assert(isLValue() && "Invalid accessor");
+  return ((const LV*)(const char*)Data.buffer)->Base.getVersion();
 }
 
 bool APValue::isNullPointer() const {
   assert(isLValue() && "Invalid usage");
   return ((const LV*)(const char*)Data.buffer)->IsNullPtr;
 }
 
 void APValue::setLValue(LValueBase B, const CharUnits &O, NoLValuePath,
-                        unsigned CallIndex, bool IsNullPtr) {
+                        bool IsNullPtr) {
   assert(isLValue() && "Invalid accessor");
   LV &LVal = *((LV*)(char*)Data.buffer);
-  LVal.BaseAndIsOnePastTheEnd.setPointer(B);
-  LVal.BaseAndIsOnePastTheEnd.setInt(false);
+  LVal.Base = B;
+  LVal.IsOnePastTheEnd = false;
   LVal.Offset = O;
-  LVal.CallIndex = CallIndex;
   LVal.resizePath((unsigned)-1);
   LVal.IsNullPtr = IsNullPtr;
 }
 
 void APValue::setLValue(LValueBase B, const CharUnits &O,
                         ArrayRef<LValuePathEntry> Path, bool IsOnePastTheEnd,
-                        unsigned CallIndex, bool IsNullPtr) {
+                        bool IsNullPtr) {
   assert(isLValue() && "Invalid accessor");
   LV &LVal = *((LV*)(char*)Data.buffer);
-  LVal.BaseAndIsOnePastTheEnd.setPointer(B);
-  LVal.BaseAndIsOnePastTheEnd.setInt(IsOnePastTheEnd);
+  LVal.Base = B;
+  LVal.IsOnePastTheEnd = IsOnePastTheEnd;
   LVal.Offset = O;
-  LVal.CallIndex = CallIndex;
   LVal.resizePath(Path.size());
   memcpy(LVal.getPath(), Path.data(), Path.size() * sizeof(LValuePathEntry));
   LVal.IsNullPtr = IsNullPtr;
 }
 
 const ValueDecl *APValue::getMemberPointerDecl() const {
   assert(isMemberPointer() && "Invalid accessor");
   const MemberPointerData &MPD =

cfe/trunk/lib/AST/ExprConstant.cpp

struct CallStackFrame {
     const LValue *This;
 
     /// Arguments - Parameter bindings for this function call, indexed by
     /// parameters' function scope indices.
     APValue *Arguments;
 
     // Note that we intentionally use std::map here so that references to
     // values are stable.
-    typedef std::map<const void*, APValue> MapTy;
-    typedef MapTy::const_iterator temp_iterator;
+    typedef std::pair<const void *, unsigned> MapKeyTy;
+    typedef std::map<MapKeyTy, APValue> MapTy;
     /// Temporaries - Temporary lvalues materialized within this stack frame.
     MapTy Temporaries;
 
     /// CallLoc - The location of the call expression for this call.
     SourceLocation CallLoc;
 
     /// Index - The call index of this call.
     unsigned Index;
 
+    /// The stack of integers for tracking version numbers for temporaries.
+    SmallVector<unsigned, 2> TempVersionStack = {1};
+    unsigned CurTempVersion = TempVersionStack.back();
+
+    unsigned getTempVersion() const { return TempVersionStack.back(); }
+
+    void pushTempVersion() {
+      TempVersionStack.push_back(++CurTempVersion);
+    }
+
+    void popTempVersion() {
+      TempVersionStack.pop_back();
+    }
+
     // FIXME: Adding this to every 'CallStackFrame' may have a nontrivial impact
     // on the overall stack usage of deeply-recursing constexpr evaluataions.
     // (We should cache this map rather than recomputing it repeatedly.)
     // But let's try this and see how it goes; we can look into caching the map
     // as a later change.
 
     /// LambdaCaptureFields - Mapping from captured variables/this to
     /// corresponding data members in the closure class.
     llvm::DenseMap<const VarDecl *, FieldDecl *> LambdaCaptureFields;
     FieldDecl *LambdaThisCaptureField;
 
     CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
                    const FunctionDecl *Callee, const LValue *This,
                    APValue *Arguments);
     ~CallStackFrame();
 
-    APValue *getTemporary(const void *Key) {
-      MapTy::iterator I = Temporaries.find(Key);
-      return I == Temporaries.end() ? nullptr : &I->second;
+    // Return the temporary for Key whose version number is Version.
+    APValue *getTemporary(const void *Key, unsigned Version) {
+      MapKeyTy KV(Key, Version);
+      auto LB = Temporaries.lower_bound(KV);
+      if (LB != Temporaries.end() && LB->first == KV)
+        return &LB->second;
+      // Pair (Key,Version) wasn't found in the map. Check that no elements
+      // in the map have 'Key' as their key.
+      assert((LB == Temporaries.end() || LB->first.first != Key) &&
+             (LB == Temporaries.begin() || std::prev(LB)->first.first != Key) &&
+             "Element with key 'Key' found in map");
+      return nullptr;
+    }
+
+    // Return the current temporary for Key in the map.
+    APValue *getCurrentTemporary(const void *Key) {
+      auto UB = Temporaries.upper_bound(MapKeyTy(Key, UINT_MAX));
+      if (UB != Temporaries.begin() && std::prev(UB)->first.first == Key)
+        return &std::prev(UB)->second;
+      return nullptr;
     }
+
+    // Return the version number of the current temporary for Key.
+    unsigned getCurrentTemporaryVersion(const void *Key) const {
+      auto UB = Temporaries.upper_bound(MapKeyTy(Key, UINT_MAX));
+      if (UB != Temporaries.begin() && std::prev(UB)->first.first == Key)
+        return std::prev(UB)->first.second;
+      return 0;
+    }
+
     APValue &createTemporary(const void *Key, bool IsLifetimeExtended);
   };
 
   /// Temporarily override 'this'.
   class ThisOverrideRAII {
   public:
     ThisOverrideRAII(CallStackFrame &Frame, const LValue *NewThis, bool Enable)
         : Frame(Frame), OldThis(Frame.This) {
struct EvalInfo {
     APValue::LValueBase EvaluatingDecl;
 
     /// EvaluatingDeclValue - This is the value being constructed for the
     /// declaration whose initializer is being evaluated, if any.
     APValue *EvaluatingDeclValue;
 
     /// EvaluatingObject - Pair of the AST node that an lvalue represents and
     /// the call index that that lvalue was allocated in.
-    typedef std::pair<APValue::LValueBase, unsigned> EvaluatingObject;
+    typedef std::pair<APValue::LValueBase, std::pair<unsigned, unsigned>>
+        EvaluatingObject;
 
     /// EvaluatingConstructors - Set of objects that are currently being
     /// constructed.
     llvm::DenseSet<EvaluatingObject> EvaluatingConstructors;
 
     struct EvaluatingConstructorRAII {
       EvalInfo &EI;
       EvaluatingObject Object;
       bool DidInsert;
       EvaluatingConstructorRAII(EvalInfo &EI, EvaluatingObject Object)
           : EI(EI), Object(Object) {
         DidInsert = EI.EvaluatingConstructors.insert(Object).second;
       }
       ~EvaluatingConstructorRAII() {
         if (DidInsert) EI.EvaluatingConstructors.erase(Object);
       }
     };
 
-    bool isEvaluatingConstructor(APValue::LValueBase Decl, unsigned CallIndex) {
-      return EvaluatingConstructors.count(EvaluatingObject(Decl, CallIndex));
+    bool isEvaluatingConstructor(APValue::LValueBase Decl, unsigned CallIndex,
+                                 unsigned Version) {
+      return EvaluatingConstructors.count(
+          EvaluatingObject(Decl, {CallIndex, Version}));
     }
 
     /// The current array initialization index, if we're performing array
     /// initialization.
     uint64_t ArrayInitIndex = -1;
 
     /// HasActiveDiagnostic - Was the previous diagnostic stored? If so, further
     /// notes attached to it will also be stored, otherwise they will not be.
EvalInfo(const ASTContext &C, Expr::EvalStatus &S, EvaluationMode Mode)
         EvaluatingDecl((const ValueDecl *)nullptr),
         EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
         HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
         EvalMode(Mode) {}
 
     void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value) {
       EvaluatingDecl = Base;
       EvaluatingDeclValue = &Value;
-      EvaluatingConstructors.insert({Base, 0});
+      EvaluatingConstructors.insert({Base, {0, 0}});
     }
 
     const LangOptions &getLangOpts() const { return Ctx.getLangOpts(); }
 
     bool CheckCallLimit(SourceLocation Loc) {
       // Don't perform any constexpr calls (other than the call we're checking)
       // when checking a potential constant expression.
       if (checkingPotentialConstantExpression() && CallStackDepth > 1)

   /// RAII object wrapping a full-expression or block scope, and handling
   /// the ending of the lifetime of temporaries created within it.
   template<bool IsFullExpression>
   class ScopeRAII {
     EvalInfo &Info;
     unsigned OldStackSize;
   public:
     ScopeRAII(EvalInfo &Info)
-        : Info(Info), OldStackSize(Info.CleanupStack.size()) {}
+        : Info(Info), OldStackSize(Info.CleanupStack.size()) {
+      // Push a new temporary version. This is needed to distinguish between
+      // temporaries created in different iterations of a loop.
+      Info.CurrentCall->pushTempVersion();
+    }
     ~ScopeRAII() {
       // Body moved to a static method to encourage the compiler to inline away
       // instances of this class.
       cleanup(Info, OldStackSize);
+      Info.CurrentCall->popTempVersion();
     }
   private:
     static void cleanup(EvalInfo &Info, unsigned OldStackSize) {
       unsigned NewEnd = OldStackSize;
       for (unsigned I = OldStackSize, N = Info.CleanupStack.size();
            I != N; ++I) {
         if (IsFullExpression && Info.CleanupStack[I].isLifetimeExtended()) {
           // Full-expression cleanup of a lifetime-extended temporary: nothing

 CallStackFrame::~CallStackFrame() {
   assert(Info.CurrentCall == this && "calls retired out of order");
   --Info.CallStackDepth;
   Info.CurrentCall = Caller;
 }
 
 APValue &CallStackFrame::createTemporary(const void *Key,
                                          bool IsLifetimeExtended) {
-  APValue &Result = Temporaries[Key];
+  unsigned Version = Info.CurrentCall->getTempVersion();
+  APValue &Result = Temporaries[MapKeyTy(Key, Version)];
   assert(Result.isUninit() && "temporary created multiple times");
   Info.CleanupStack.push_back(Cleanup(&Result, IsLifetimeExtended));
   return Result;
 }
 
 static void describeCall(CallStackFrame *Frame, raw_ostream &Out);
 
 void EvalInfo::addCallStack(unsigned Limit) {
void setFrom(const APValue &v) {
         IntImag = v.getComplexIntImag();
       }
     }
   };
 
   struct LValue {
     APValue::LValueBase Base;
     CharUnits Offset;
-    unsigned InvalidBase : 1;
-    unsigned CallIndex : 31;
     SubobjectDesignator Designator;
-    bool IsNullPtr;
+    bool IsNullPtr : 1;
+    bool InvalidBase : 1;
 
     const APValue::LValueBase getLValueBase() const { return Base; }
     CharUnits &getLValueOffset() { return Offset; }
     const CharUnits &getLValueOffset() const { return Offset; }
-    unsigned getLValueCallIndex() const { return CallIndex; }
     SubobjectDesignator &getLValueDesignator() { return Designator; }
     const SubobjectDesignator &getLValueDesignator() const { return Designator;}
     bool isNullPointer() const { return IsNullPtr;}
 
+    unsigned getLValueCallIndex() const { return Base.getCallIndex(); }
+    unsigned getLValueVersion() const { return Base.getVersion(); }
+
     void moveInto(APValue &V) const {
       if (Designator.Invalid)
-        V = APValue(Base, Offset, APValue::NoLValuePath(), CallIndex,
-                    IsNullPtr);
+        V = APValue(Base, Offset, APValue::NoLValuePath(), IsNullPtr);
       else {
         assert(!InvalidBase && "APValues can't handle invalid LValue bases");
         V = APValue(Base, Offset, Designator.Entries,
-                    Designator.IsOnePastTheEnd, CallIndex, IsNullPtr);
+                    Designator.IsOnePastTheEnd, IsNullPtr);
       }
     }
     void setFrom(ASTContext &Ctx, const APValue &V) {
       assert(V.isLValue() && "Setting LValue from a non-LValue?");
       Base = V.getLValueBase();
       Offset = V.getLValueOffset();
       InvalidBase = false;
-      CallIndex = V.getLValueCallIndex();
       Designator = SubobjectDesignator(Ctx, V);
       IsNullPtr = V.isNullPointer();
     }
 
-    void set(APValue::LValueBase B, unsigned I = 0, bool BInvalid = false) {
+    void set(APValue::LValueBase B, bool BInvalid = false) {
 #ifndef NDEBUG
       // We only allow a few types of invalid bases. Enforce that here.
       if (BInvalid) {
         const auto *E = B.get<const Expr *>();
         assert((isa<MemberExpr>(E) || tryUnwrapAllocSizeCall(E)) &&
                "Unexpected type of invalid base");
       }
 #endif
 
       Base = B;
       Offset = CharUnits::fromQuantity(0);
       InvalidBase = BInvalid;
-      CallIndex = I;
       Designator = SubobjectDesignator(getType(B));
       IsNullPtr = false;
     }
 
     void setNull(QualType PointerTy, uint64_t TargetVal) {
       Base = (Expr *)nullptr;
       Offset = CharUnits::fromQuantity(TargetVal);
       InvalidBase = false;
-      CallIndex = 0;
       Designator = SubobjectDesignator(PointerTy->getPointeeType());
       IsNullPtr = true;
     }
 
     void setInvalid(APValue::LValueBase B, unsigned I = 0) {
-      set(B, I, true);
+      set(B, true);
     }
 
     // Check that this LValue is not based on a null pointer. If it is, produce
     // a diagnostic and mark the designator as invalid.
     bool checkNullPointer(EvalInfo &Info, const Expr *E,
                           CheckSubobjectKind CSK) {
       if (Designator.Invalid)
         return false;

 static bool EvaluateAtomic(const Expr *E, const LValue *This, APValue &Result,
                            EvalInfo &Info);
 static bool EvaluateAsRValue(EvalInfo &Info, const Expr *E, APValue &Result);
 
 //===----------------------------------------------------------------------===//
 // Misc utilities
 //===----------------------------------------------------------------------===//
 
+/// A helper function to create a temporary and set an LValue.
+template <class KeyTy>
+static APValue &createTemporary(const KeyTy *Key, bool IsLifetimeExtended,
+                                LValue &LV, CallStackFrame &Frame) {
+  LV.set({Key, Frame.Info.CurrentCall->Index,
+          Frame.Info.CurrentCall->getTempVersion()});
+  return Frame.createTemporary(Key, IsLifetimeExtended);
+}
+
 /// Negate an APSInt in place, converting it to a signed form if necessary, and
 /// preserving its value (by extending by up to one bit as needed).
 static void negateAsSigned(APSInt &Int) {
   if (Int.isUnsigned() || Int.isMinSignedValue()) {
     Int = Int.extend(Int.getBitWidth() + 1);
     Int.setIsSigned(true);
   }
   Int = -Int;
static bool CheckConstantExpression(EvalInfo &Info, SourceLocation DiagLoc,
   return true;
 }
 
 static const ValueDecl *GetLValueBaseDecl(const LValue &LVal) {
   return LVal.Base.dyn_cast<const ValueDecl*>();
 }
 
 static bool IsLiteralLValue(const LValue &Value) {
-  if (Value.CallIndex)
+  if (Value.getLValueCallIndex())
     return false;
   const Expr *E = Value.Base.dyn_cast<const Expr*>();
   return E && !isa<MaterializeTemporaryExpr>(E);
 }
 
 static bool IsWeakLValue(const LValue &Value) {
   const ValueDecl *Decl = GetLValueBaseDecl(Value);
   return Decl && Decl->isWeak();

 /// \param Info   Information about the ongoing evaluation.
 /// \param E      An expression to be used when printing diagnostics.
 /// \param VD     The variable whose initializer should be obtained.
 /// \param Frame  The frame in which the variable was created. Must be null
 ///               if this variable is not local to the evaluation.
 /// \param Result Filled in with a pointer to the value of the variable.
 static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
                                 const VarDecl *VD, CallStackFrame *Frame,
-                                APValue *&Result) {
+                                APValue *&Result, const LValue *LVal) {
 
   // If this is a parameter to an active constexpr function call, perform
   // argument substitution.
   if (const ParmVarDecl *PVD = dyn_cast<ParmVarDecl>(VD)) {
     // Assume arguments of a potential constant expression are unknown
     // constant expressions.
     if (Info.checkingPotentialConstantExpression())
       return false;
     if (!Frame || !Frame->Arguments) {
       Info.FFDiag(E, diag::note_invalid_subexpr_in_const_expr);
       return false;
     }
     Result = &Frame->Arguments[PVD->getFunctionScopeIndex()];
     return true;
   }
 
   // If this is a local variable, dig out its value.
   if (Frame) {
-    Result = Frame->getTemporary(VD);
+    Result = LVal ? Frame->getTemporary(VD, LVal->getLValueVersion())
+                  : Frame->getCurrentTemporary(VD);
     if (!Result) {
       // Assume variables referenced within a lambda's call operator that were
       // not declared within the call operator are captures and during checking
       // of a potential constant expression, assume they are unknown constant
       // expressions.
       assert(isLambdaCallOperator(Frame->Callee) &&
              (VD->getDeclContext() != Frame->Callee || VD->isInitCapture()) &&
              "missing value for local variable");
static CompleteObject findCompleteObject(EvalInfo &Info, const Expr *E,
                                          AccessKinds AK, const LValue &LVal,
                                          QualType LValType) {
   if (!LVal.Base) {
     Info.FFDiag(E, diag::note_constexpr_access_null) << AK;
     return CompleteObject();
   }
 
   CallStackFrame *Frame = nullptr;
-  if (LVal.CallIndex) {
-    Frame = Info.getCallFrame(LVal.CallIndex);
+  if (LVal.getLValueCallIndex()) {
+    Frame = Info.getCallFrame(LVal.getLValueCallIndex());
     if (!Frame) {
       Info.FFDiag(E, diag::note_constexpr_lifetime_ended, 1)
         << AK << LVal.Base.is<const ValueDecl*>();
       NoteLValueLocation(Info, LVal.Base);
       return CompleteObject();
     }
   }
 
if (!Frame) {
           Info.Note(VD->getLocation(), diag::note_declared_at);
         } else {
           Info.FFDiag(E);
         }
         return CompleteObject();
       }
     }
 
-    if (!evaluateVarDeclInit(Info, E, VD, Frame, BaseVal))
+    if (!evaluateVarDeclInit(Info, E, VD, Frame, BaseVal, &LVal))
       return CompleteObject();
   } else {
     const Expr *Base = LVal.Base.dyn_cast<const Expr*>();
 
     if (!Frame) {
       if (const MaterializeTemporaryExpr *MTE =
               dyn_cast<MaterializeTemporaryExpr>(Base)) {
         assert(MTE->getStorageDuration() == SD_Static &&
if (!Frame) {
         BaseVal = Info.Ctx.getMaterializedTemporaryValue(MTE, false);
         assert(BaseVal && "got reference to unevaluated temporary");
         LifetimeStartedInEvaluation = true;
       } else {
         Info.FFDiag(E);
         return CompleteObject();
       }
     } else {
-      BaseVal = Frame->getTemporary(Base);
+      BaseVal = Frame->getTemporary(Base, LVal.Base.getVersion());
       assert(BaseVal && "missing value for temporary");
     }
 
     // Volatile temporary objects cannot be accessed in constant expressions.
     if (BaseType.isVolatileQualified()) {
       if (Info.getLangOpts().CPlusPlus) {
         Info.FFDiag(E, diag::note_constexpr_access_volatile_obj, 1)
           << AK << 0;
         Info.Note(Base->getExprLoc(), diag::note_constexpr_temporary_here);
       } else {
         Info.FFDiag(E);
       }
       return CompleteObject();
     }
   }
 
   // During the construction of an object, it is not yet 'const'.
   // FIXME: This doesn't do quite the right thing for const subobjects of the
   // object under construction.
-  if (Info.isEvaluatingConstructor(LVal.getLValueBase(), LVal.CallIndex)) {
+  if (Info.isEvaluatingConstructor(LVal.getLValueBase(),
+                                   LVal.getLValueCallIndex(),
+                                   LVal.getLValueVersion())) {
     BaseType = Info.Ctx.getCanonicalType(BaseType);
     BaseType.removeLocalConst();
     LifetimeStartedInEvaluation = true;
   }
 
   // In C++14, we can't safely access any mutable state when we might be
   // evaluating after an unmodeled side effect.
   //

 static bool handleLValueToRValueConversion(EvalInfo &Info, const Expr *Conv,
                                            QualType Type,
                                            const LValue &LVal, APValue &RVal) {
   if (LVal.Designator.Invalid)
     return false;
 
   // Check for special cases where there is no existing APValue to look at.
   const Expr *Base = LVal.Base.dyn_cast<const Expr*>();
-  if (Base && !LVal.CallIndex && !Type.isVolatileQualified()) {
+  if (Base && !LVal.getLValueCallIndex() && !Type.isVolatileQualified()) {
     if (const CompoundLiteralExpr *CLE = dyn_cast<CompoundLiteralExpr>(Base)) {
       // In C99, a CompoundLiteralExpr is an lvalue, and we defer evaluating the
       // initializer until now for such expressions. Such an expression can't be
       // an ICE in C, so this only matters for fold.
       if (Type.isVolatileQualified()) {
         Info.FFDiag(Conv);
         return false;
       }

 }
 
 static bool EvaluateVarDecl(EvalInfo &Info, const VarDecl *VD) {
   // We don't need to evaluate the initializer for a static local.
   if (!VD->hasLocalStorage())
     return true;
 
   LValue Result;
-  Result.set(VD, Info.CurrentCall->Index);
-  APValue &Val = Info.CurrentCall->createTemporary(VD, true);
+  APValue &Val = createTemporary(VD, true, Result, *Info.CurrentCall);
 
   const Expr *InitE = VD->getInit();
   if (!InitE) {
     Info.FFDiag(VD->getLocStart(), diag::note_constexpr_uninitialized)
       << false << VD->getType();
     Val = APValue();
     return false;
   }

 /// \brief A location where the result (returned value) of evaluating a
 /// statement should be stored.
 struct StmtResult {
   /// The APValue that should be filled in with the returned value.
   APValue &Value;
   /// The location containing the result, if any (used to support RVO).
   const LValue *Slot;
 };
+
+struct TempVersionRAII {
+  CallStackFrame &Frame;
+
+  TempVersionRAII(CallStackFrame &Frame) : Frame(Frame) {
+    Frame.pushTempVersion();
+  }
+
+  ~TempVersionRAII() {
+    Frame.popTempVersion();
+  }
+};
+
 }
 
 static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info,
                                    const Stmt *S,
                                    const SwitchCase *SC = nullptr);
 
 /// Evaluate the body of a loop, and translate the result as appropriate.
 static EvalStmtResult EvaluateLoopBody(StmtResult &Result, EvalInfo &Info,
static bool HandleConstructorCall(const Expr *E, const LValue &This,
 
   const CXXRecordDecl *RD = Definition->getParent();
   if (RD->getNumVBases()) {
     Info.FFDiag(CallLoc, diag::note_constexpr_virtual_base) << RD;
     return false;
   }
 
   EvalInfo::EvaluatingConstructorRAII EvalObj(
-      Info, {This.getLValueBase(), This.CallIndex});
+      Info, {This.getLValueBase(),
+             {This.getLValueCallIndex(), This.getLValueVersion()}});
   CallStackFrame Frame(Info, CallLoc, Definition, &This, ArgValues);
 
   // FIXME: Creating an APValue just to hold a nonexistent return value is
   // wasteful.
   APValue RetVal;
   StmtResult Ret = {RetVal, nullptr};
 
   // If it's a delegating constructor, delegate.
public:
   bool VisitUnaryPlus(const UnaryOperator *E)
     { return StmtVisitorTy::Visit(E->getSubExpr()); }
   bool VisitChooseExpr(const ChooseExpr *E)
     { return StmtVisitorTy::Visit(E->getChosenSubExpr()); }
   bool VisitGenericSelectionExpr(const GenericSelectionExpr *E)
     { return StmtVisitorTy::Visit(E->getResultExpr()); }
   bool VisitSubstNonTypeTemplateParmExpr(const SubstNonTypeTemplateParmExpr *E)
     { return StmtVisitorTy::Visit(E->getReplacement()); }
-  bool VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *E)
-    { return StmtVisitorTy::Visit(E->getExpr()); }
+  bool VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *E) {
+    TempVersionRAII RAII(*Info.CurrentCall);
+    return StmtVisitorTy::Visit(E->getExpr());
+  }
   bool VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *E) {
+    TempVersionRAII RAII(*Info.CurrentCall);
     // The initializer may not have been parsed yet, or might be erroneous.
     if (!E->getExpr())
       return Error(E);
     return StmtVisitorTy::Visit(E->getExpr());
   }
   // We cannot create any objects for which cleanups are required, so there is
   // nothing to do here; all cleanups must come from unevaluated subexpressions.
   bool VisitExprWithCleanups(const ExprWithCleanups *E)
if (!HandleConditionalOperator(E)) {
       Fold.keepDiagnostics();
       return false;
     }
 
     return true;
   }
 
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
-    if (APValue *Value = Info.CurrentCall->getTemporary(E))
+    if (APValue *Value = Info.CurrentCall->getCurrentTemporary(E))
       return DerivedSuccess(*Value, E);
 
     const Expr *Source = E->getSourceExpr();
     if (!Source)
       return Error(E);
     if (Source == E) { // sanity checking.
       assert(0 && "OpaqueValueExpr recursively refers to itself");
       return Error(E);
if (VD->hasLocalStorage() && Info.CurrentCall->Index > 1) {
     if (Info.CurrentCall->Callee &&
         Info.CurrentCall->Callee->Equals(VD->getDeclContext())) {
       Frame = Info.CurrentCall;
     }
   }
 
   if (!VD->getType()->isReferenceType()) {
     if (Frame) {
-      Result.set(VD, Frame->Index);
+      Result.set({VD, Frame->Index,
+                  Info.CurrentCall->getCurrentTemporaryVersion(VD)});
       return true;
     }
     return Success(VD);
   }
 
   APValue *V;
-  if (!evaluateVarDeclInit(Info, E, VD, Frame, V))
+  if (!evaluateVarDeclInit(Info, E, VD, Frame, V, nullptr))
     return false;
   if (V->isUninit()) {
     if (!Info.checkingPotentialConstantExpression())
       Info.FFDiag(E, diag::note_constexpr_use_uninit_reference);
     return false;
   }
   return Success(*V, E);
 }
bool LValueExprEvaluator::VisitMaterializeTemporaryExpr(
   // result of a constant expression evaluation, so we need to preserve its
   // value for use outside this evaluation.
   APValue *Value;
   if (E->getStorageDuration() == SD_Static) {
     Value = Info.Ctx.getMaterializedTemporaryValue(E, true);
     *Value = APValue();
     Result.set(E);
   } else {
-    Value = &Info.CurrentCall->
-        createTemporary(E, E->getStorageDuration() == SD_Automatic);
-    Result.set(E, Info.CurrentCall->Index);
+    Value = &createTemporary(E, E->getStorageDuration() == SD_Automatic, Result,
+                             *Info.CurrentCall);
   }
 
   QualType Type = Inner->getType();
 
   // Materialize the temporary itself.
   if (!EvaluateInPlace(*Value, Info, Result, Inner) ||
       (E->getStorageDuration() == SD_Static &&
        !CheckConstantExpression(Info, E->getExprLoc(), Type, *Value))) {
if (!EvaluateIntegerOrLValue(SubExpr, Value, Info))
       break;
 
     if (Value.isInt()) {
       unsigned Size = Info.Ctx.getTypeSize(E->getType());
       uint64_t N = Value.getInt().extOrTrunc(Size).getZExtValue();
       Result.Base = (Expr*)nullptr;
       Result.InvalidBase = false;
       Result.Offset = CharUnits::fromQuantity(N);
-      Result.CallIndex = 0;
       Result.Designator.setInvalid();
       Result.IsNullPtr = false;
       return true;
     } else {
       // Cast is of an lvalue, no need to change value.
       Result.setFrom(Info.Ctx, Value);
       return true;
     }
   }
 
   case CK_ArrayToPointerDecay: {
     if (SubExpr->isGLValue()) {
       if (!evaluateLValue(SubExpr, Result))
         return false;
     } else {
-      Result.set(SubExpr, Info.CurrentCall->Index);
-      if (!EvaluateInPlace(Info.CurrentCall->createTemporary(SubExpr, false),
-                           Info, Result, SubExpr))
+      APValue &Value = createTemporary(SubExpr, false, Result,
+                                       *Info.CurrentCall);
+      if (!EvaluateInPlace(Value, Info, Result, SubExpr))
         return false;
     }
     // The result is a pointer to the first element of the array.
     auto *AT = Info.Ctx.getAsArrayType(SubExpr->getType());
     if (auto *CAT = dyn_cast<ConstantArrayType>(AT))
       Result.addArray(Info, E, CAT);
     else
       Result.addUnsizedArray(Info, E, AT->getElementType());

 class TemporaryExprEvaluator
   : public LValueExprEvaluatorBase<TemporaryExprEvaluator> {
 public:
   TemporaryExprEvaluator(EvalInfo &Info, LValue &Result) :
     LValueExprEvaluatorBaseTy(Info, Result, false) {}
 
   /// Visit an expression which constructs the value of this temporary.
   bool VisitConstructExpr(const Expr *E) {
-    Result.set(E, Info.CurrentCall->Index);
-    return EvaluateInPlace(Info.CurrentCall->createTemporary(E, false),
-                           Info, Result, E);
+    APValue &Value = createTemporary(E, false, Result, *Info.CurrentCall);
+    return EvaluateInPlace(Value, Info, Result, E);
   }
 
   bool VisitCastExpr(const CastExpr *E) {
     switch (E->getCastKind()) {
     default:
       return LValueExprEvaluatorBaseTy::VisitCastExpr(E);
 
     case CK_ConstructorConversion:
if (A.getLValueBase().getOpaqueValue() !=
     if (!ADecl)
       return false;
     const Decl *BDecl = GetLValueBaseDecl(B);
     if (!BDecl || ADecl->getCanonicalDecl() != BDecl->getCanonicalDecl())
       return false;
   }
 
   return IsGlobalLValue(A.getLValueBase()) ||
-         A.getLValueCallIndex() == B.getLValueCallIndex();
+         (A.getLValueCallIndex() == B.getLValueCallIndex() &&
+          A.getLValueVersion() == B.getLValueVersion());
 }
 
 /// \brief Determine whether this is a pointer past the end of the complete
 /// object referred to by the lvalue.
 static bool isOnePastTheEndOfCompleteObject(const ASTContext &Ctx,
                                             const LValue &LV) {
   // A null pointer can be viewed as being "past the end" but we don't
   // choose to look at it that way here.
static bool Evaluate(APValue &Result, EvalInfo &Info, const Expr *E) {
   } else if (T->isMemberPointerType()) {
     MemberPtr P;
     if (!EvaluateMemberPointer(E, P, Info))
       return false;
     P.moveInto(Result);
     return true;
   } else if (T->isArrayType()) {
     LValue LV;
-    LV.set(E, Info.CurrentCall->Index);
-    APValue &Value = Info.CurrentCall->createTemporary(E, false);
+    APValue &Value = createTemporary(E, false, LV, *Info.CurrentCall);
     if (!EvaluateArray(E, LV, Value, Info))
       return false;
     Result = Value;
   } else if (T->isRecordType()) {
     LValue LV;
-    LV.set(E, Info.CurrentCall->Index);
-    APValue &Value = Info.CurrentCall->createTemporary(E, false);
+    APValue &Value = createTemporary(E, false, LV, *Info.CurrentCall);
     if (!EvaluateRecord(E, LV, Value, Info))
       return false;
     Result = Value;
   } else if (T->isVoidType()) {
     if (!Info.getLangOpts().CPlusPlus11)
       Info.CCEDiag(E, diag::note_constexpr_nonliteral)
         << E->getType();
     if (!EvaluateVoid(E, Info))
       return false;
   } else if (T->isAtomicType()) {
     QualType Unqual = T.getAtomicUnqualifiedType();
     if (Unqual->isArrayType() || Unqual->isRecordType()) {
       LValue LV;
-      LV.set(E, Info.CurrentCall->Index);
-      APValue &Value = Info.CurrentCall->createTemporary(E, false);
+      APValue &Value = createTemporary(E, false, LV, *Info.CurrentCall);
       if (!EvaluateAtomic(E, &LV, Value, Info))
         return false;
     } else {
       if (!EvaluateAtomic(E, nullptr, Result, Info))
         return false;
     }
   } else if (Info.getLangOpts().CPlusPlus11) {
     Info.FFDiag(E, diag::note_constexpr_nonliteral) << E->getType();
bool Expr::isPotentialConstantExpr(const FunctionDecl *FD,
 
   const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD);
   const CXXRecordDecl *RD = MD ? MD->getParent()->getCanonicalDecl() : nullptr;
 
   // Fabricate an arbitrary expression on the stack and pretend that it
   // is a temporary being used as the 'this' pointer.
   LValue This;
   ImplicitValueInitExpr VIE(RD ? Info.Ctx.getRecordType(RD) : Info.Ctx.IntTy);
-  This.set(&VIE, Info.CurrentCall->Index);
+  This.set({&VIE, Info.CurrentCall->Index});
 
   ArrayRef<const Expr*> Args;
 
   APValue Scratch;
   if (const CXXConstructorDecl *CD = dyn_cast<CXXConstructorDecl>(FD)) {
     // Evaluate the call as a constant initializer, to allow the construction
     // of objects of non-literal types.
     Info.setEvaluatingDecl(This.getLValueBase(), Scratch);

cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp

constexpr int h(int n) {
     *p[n] = 0; // expected-note 3{{assignment to object outside its lifetime}}
     return *p[n];
   }
   static_assert(h(0) == 0, ""); // ok, lifetime-extended
   static_assert(h(1) == 0, ""); // expected-error {{constant expression}} expected-note {{in call}}
   static_assert(h(2) == 0, ""); // expected-error {{constant expression}} expected-note {{in call}}
   static_assert(h(3) == 0, ""); // expected-error {{constant expression}} expected-note {{in call}}
 
-  // FIXME: This function should be treated as non-constant.
   constexpr void lifetime_versus_loops() {
     int *p = 0;
     for (int i = 0; i != 2; ++i) {
       int *q = p;
       int n = 0;
       p = &n;
       if (i)
         // This modifies the 'n' from the previous iteration of the loop outside
         // its lifetime.
-        ++*q;
+        ++*q; // expected-note {{increment of object outside its lifetime}}
     }
   }
-  static_assert((lifetime_versus_loops(), true), "");
+  static_assert((lifetime_versus_loops(), true), ""); // expected-error {{constant expression}} expected-note {{in call}}
 }
 
 namespace Bitfields {
   struct A {
     bool b : 1;
     int n : 4;
     unsigned u : 5;
   };

cfe/trunk/test/SemaCXX/constexpr-default-arg.cpp

+// RUN: %clang_cc1 -std=c++1y -S -o - -emit-llvm -verify %s
+
+namespace default_arg_temporary {
+
+constexpr bool equals(const float& arg = 1.0f) {
+  return arg == 1.0f;
+}
+
+constexpr const int &x(const int &p = 0) {
+  return p;
+}
+
+struct S {
+  constexpr S(const int &a = 0) {}
+};
+
+void test_default_arg2() {
+  // This piece of code used to cause an assertion failure in
+  // CallStackFrame::createTemporary because the same MTE is used to initilize
+  // both elements of the array (see PR33140).
+  constexpr S s[2] = {};
+
+  // This piece of code used to cause an assertion failure in
+  // CallStackFrame::createTemporary because multiple CXXDefaultArgExpr share
+  // the same MTE (see PR33140).
+  static_assert(equals() && equals(), "");
+
+  // Test that constant expression evaluation produces distinct lvalues for
+  // each call.
+  static_assert(&x() != &x(), "");
+}
+
+// Check that multiple CXXDefaultInitExprs don't cause an assertion failure.
+struct A { int &&r = 0; }; // expected-warning {{binding reference member}} // expected-note {{reference member declared here}}
+struct B { A x, y; };
+B b = {};
+
+}