[NFC] Refactor representation of materialized temporaries

Summary:
this patch refactor representation of materialized temporaries to prevent an issue raised by rsmith in https://reviews.llvm.org/D63640#inline-612718

Reviewers: rsmith, martong, shafik

Reviewed By: rsmith

Subscribers: thakis, sammccall, ilya-biryukov, rnkovacs, arphaman, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D69360

clang-tools-extra/clang-tidy/abseil/StrCatAppendCheck.cpp

@@ -24,7 +24,7 @@ AST_MATCHER_P(Stmt, IgnoringTemporaries, ast_matchers::internal::Matcher<Stmt>,
   const Stmt *E = &Node;
   while (true) {
     if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(E))
-      E = MTE->getTemporary();
+      E = MTE->getSubExpr();
     if (const auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E))
       E = BTE->getSubExpr();
     if (const auto *ICE = dyn_cast<ImplicitCastExpr>(E))

clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp

@@ -53,7 +53,7 @@ buildBindArguments(const MatchFinder::MatchResult &Result, const CallExpr *C) {
     const Expr *E = C->getArg(I);
     BindArgument B;
     if (const auto *M = dyn_cast<MaterializeTemporaryExpr>(E)) {
-      const auto *TE = M->GetTemporaryExpr();
+      const auto *TE = M->getSubExpr();
       B.Kind = isa<CallExpr>(TE) ? BK_CallExpr : BK_Temporary;
     }
 

clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp

@@ -177,7 +177,7 @@ const Expr *digThroughConstructors(const Expr *E) {
       return nullptr;
     E = ConstructExpr->getArg(0);
     if (const auto *Temp = dyn_cast<MaterializeTemporaryExpr>(E))
-      E = Temp->GetTemporaryExpr();
+      E = Temp->getSubExpr();
     return digThroughConstructors(E);
   }
   return E;

clang-tools-extra/clang-tidy/performance/ImplicitConversionInLoopCheck.cpp

@@ -78,7 +78,7 @@ void ImplicitConversionInLoopCheck::check(
   // iterator returns a value instead of a reference, and the loop variable
   // is a reference. This situation is fine (it probably produces the same
   // code at the end).
-  if (IsNonTrivialImplicitCast(Materialized->getTemporary()))
+  if (IsNonTrivialImplicitCast(Materialized->getSubExpr()))
     ReportAndFix(Result.Context, VD, OperatorCall);
 }
 

clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.cpp

@@ -202,7 +202,7 @@ void NonConstParameterCheck::markCanNotBeConst(const Expr *E,
   } else if (const auto *Constr = dyn_cast<CXXConstructExpr>(E)) {
     for (const auto *Arg : Constr->arguments()) {
       if (const auto *M = dyn_cast<MaterializeTemporaryExpr>(Arg))
-        markCanNotBeConst(cast<Expr>(M->getTemporary()), CanNotBeConst);
+        markCanNotBeConst(cast<Expr>(M->getSubExpr()), CanNotBeConst);
     }
   } else if (const auto *ILE = dyn_cast<InitListExpr>(E)) {
     for (unsigned I = 0U; I < ILE->getNumInits(); ++I)

clang/include/clang/AST/ASTContext.h

@@ -272,12 +272,6 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// Mapping from __block VarDecls to BlockVarCopyInit.
   llvm::DenseMap<const VarDecl *, BlockVarCopyInit> BlockVarCopyInits;
 
-  /// Mapping from materialized temporaries with static storage duration
-  /// that appear in constant initializers to their evaluated values.  These are
-  /// allocated in a std::map because their address must be stable.
-  llvm::DenseMap<const MaterializeTemporaryExpr *, APValue *>
-    MaterializedTemporaryValues;
-
   /// Used to cleanups APValues stored in the AST.
   mutable llvm::SmallVector<APValue *, 0> APValueCleanups;
 
@@ -2827,11 +2821,6 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// index of the parameter when it exceeds the size of the normal bitfield.
   unsigned getParameterIndex(const ParmVarDecl *D) const;
 
-  /// Get the storage for the constant value of a materialized temporary
-  /// of static storage duration.
-  APValue *getMaterializedTemporaryValue(const MaterializeTemporaryExpr *E,
-                                         bool MayCreate);
-
   /// Return a string representing the human readable name for the specified
   /// function declaration or file name. Used by SourceLocExpr and
   /// PredefinedExpr to cache evaluated results.

clang/include/clang/AST/DeclCXX.h

@@ -3052,6 +3052,80 @@ class NamespaceAliasDecl : public NamedDecl,
   static bool classofKind(Kind K) { return K == NamespaceAlias; }
 };
 
+/// Implicit declaration of a temporary that was materialized by
+/// a MaterializeTemporaryExpr and lifetime-extended by a declaration
+class LifetimeExtendedTemporaryDecl final : public Decl {
+  friend class MaterializeTemporaryExpr;
+  friend class ASTDeclReader;
+
+  Stmt *ExprWithTemporary = nullptr;
+
+  /// The declaration which lifetime-extended this reference, if any.
+  /// Either a VarDecl, or (for a ctor-initializer) a FieldDecl.
+  ValueDecl *ExtendingDecl = nullptr;
+  unsigned ManglingNumber;
+
+  mutable APValue *Value = nullptr;
+
+  virtual void anchor();
+
+  LifetimeExtendedTemporaryDecl(Expr *Temp, ValueDecl *EDecl, unsigned Mangling)
+      : Decl(Decl::LifetimeExtendedTemporary, EDecl->getDeclContext(),
+             EDecl->getLocation()),
+        ExprWithTemporary(Temp), ExtendingDecl(EDecl),
+        ManglingNumber(Mangling) {}
+
+  LifetimeExtendedTemporaryDecl(EmptyShell)
+      : Decl(Decl::LifetimeExtendedTemporary, EmptyShell{}) {}
+
+public:
+  static LifetimeExtendedTemporaryDecl *Create(Expr *Temp, ValueDecl *EDec,
+                                               unsigned Mangling) {
+    return new (EDec->getASTContext(), EDec->getDeclContext())
+        LifetimeExtendedTemporaryDecl(Temp, EDec, Mangling);
+  }
+  static LifetimeExtendedTemporaryDecl *CreateDeserialized(ASTContext &C,
+                                                           unsigned ID) {
+    return new (C, ID) LifetimeExtendedTemporaryDecl(EmptyShell{});
+  }
+
+  ValueDecl *getExtendingDecl() { return ExtendingDecl; }
+  const ValueDecl *getExtendingDecl() const { return ExtendingDecl; }
+
+  /// Retrieve the storage duration for the materialized temporary.
+  StorageDuration getStorageDuration() const;
+
+  /// Retrieve the expression to which the temporary materialization conversion
+  /// was applied. This isn't necessarily the initializer of the temporary due
+  /// to the C++98 delayed materialization rules, but
+  /// skipRValueSubobjectAdjustments can be used to find said initializer within
+  /// the subexpression.
+  Expr *getTemporaryExpr() { return cast<Expr>(ExprWithTemporary); }
+  const Expr *getTemporaryExpr() const { return cast<Expr>(ExprWithTemporary); }
+
+  unsigned getManglingNumber() const { return ManglingNumber; }
+
+  /// Get the storage for the constant value of a materialized temporary
+  /// of static storage duration.
+  APValue *getOrCreateValue(bool MayCreate) const;
+
+  APValue *getValue() const { return Value; }
+
+  // Iterators
+  Stmt::child_range childrenExpr() {
+    return Stmt::child_range(&ExprWithTemporary, &ExprWithTemporary + 1);
+  }
+
+  Stmt::const_child_range childrenExpr() const {
+    return Stmt::const_child_range(&ExprWithTemporary, &ExprWithTemporary + 1);
+  }
+
+  static bool classof(const Decl *D) { return classofKind(D->getKind()); }
+  static bool classofKind(Kind K) {
+    return K == Decl::LifetimeExtendedTemporary;
+  }
+};
+
 /// Represents a shadow declaration introduced into a scope by a
 /// (resolved) using declaration.
 ///

clang/include/clang/AST/ExprCXX.h

@@ -4421,70 +4421,66 @@ class MaterializeTemporaryExpr : public Expr {
   friend class ASTStmtReader;
   friend class ASTStmtWriter;
 
-  struct ExtraState {
-    /// The temporary-generating expression whose value will be
-    /// materialized.
-    Stmt *Temporary;
-
-    /// The declaration which lifetime-extended this reference, if any.
-    /// Either a VarDecl, or (for a ctor-initializer) a FieldDecl.
-    const ValueDecl *ExtendingDecl;
-
-    unsigned ManglingNumber;
-  };
-  llvm::PointerUnion<Stmt *, ExtraState *> State;
+  llvm::PointerUnion<Stmt *, LifetimeExtendedTemporaryDecl *> State;
 
 public:
   MaterializeTemporaryExpr(QualType T, Expr *Temporary,
-                           bool BoundToLvalueReference)
-      : Expr(MaterializeTemporaryExprClass, T,
-             BoundToLvalueReference? VK_LValue : VK_XValue, OK_Ordinary,
-             Temporary->isTypeDependent(), Temporary->isValueDependent(),
-             Temporary->isInstantiationDependent(),
-             Temporary->containsUnexpandedParameterPack()),
-        State(Temporary) {}
+                           bool BoundToLvalueReference,
+                           LifetimeExtendedTemporaryDecl *MTD = nullptr);
 
   MaterializeTemporaryExpr(EmptyShell Empty)
       : Expr(MaterializeTemporaryExprClass, Empty) {}
 
-  Stmt *getTemporary() const {
-    return State.is<Stmt *>() ? State.get<Stmt *>()
-                              : State.get<ExtraState *>()->Temporary;
-  }
-
   /// Retrieve the temporary-generating subexpression whose value will
   /// be materialized into a glvalue.
-  Expr *GetTemporaryExpr() const { return static_cast<Expr *>(getTemporary()); }
+  Expr *getSubExpr() const {
+    return cast<Expr>(
+        State.is<Stmt *>()
+            ? State.get<Stmt *>()
+            : State.get<LifetimeExtendedTemporaryDecl *>()->getTemporaryExpr());
+  }
 
   /// Retrieve the storage duration for the materialized temporary.
   StorageDuration getStorageDuration() const {
-    const ValueDecl *ExtendingDecl = getExtendingDecl();
-    if (!ExtendingDecl)
-      return SD_FullExpression;
-    // FIXME: This is not necessarily correct for a temporary materialized
-    // within a default initializer.
-    if (isa<FieldDecl>(ExtendingDecl))
-      return SD_Automatic;
-    // FIXME: This only works because storage class specifiers are not allowed
-    // on decomposition declarations.
-    if (isa<BindingDecl>(ExtendingDecl))
-      return ExtendingDecl->getDeclContext()->isFunctionOrMethod()
-                 ? SD_Automatic
-                 : SD_Static;
-    return cast<VarDecl>(ExtendingDecl)->getStorageDuration();
+    return State.is<Stmt *>() ? SD_FullExpression
+                              : State.get<LifetimeExtendedTemporaryDecl *>()
+                                    ->getStorageDuration();
+  }
+
+  /// Get the storage for the constant value of a materialized temporary
+  /// of static storage duration.
+  APValue *getOrCreateValue(bool MayCreate) const {
+    assert(State.is<LifetimeExtendedTemporaryDecl *>() &&
+           "the temporary has not been lifetime extended");
+    return State.get<LifetimeExtendedTemporaryDecl *>()->getOrCreateValue(
+        MayCreate);
+  }
+
+  LifetimeExtendedTemporaryDecl *getLifetimeExtendedTemporaryDecl() {
+    return State.dyn_cast<LifetimeExtendedTemporaryDecl *>();
+  }
+  const LifetimeExtendedTemporaryDecl *
+  getLifetimeExtendedTemporaryDecl() const {
+    return State.dyn_cast<LifetimeExtendedTemporaryDecl *>();
   }
 
   /// Get the declaration which triggered the lifetime-extension of this
   /// temporary, if any.
-  const ValueDecl *getExtendingDecl() const {
+  ValueDecl *getExtendingDecl() {
     return State.is<Stmt *>() ? nullptr
-                              : State.get<ExtraState *>()->ExtendingDecl;
+                              : State.get<LifetimeExtendedTemporaryDecl *>()
+                                    ->getExtendingDecl();
+  }
+  const ValueDecl *getExtendingDecl() const {
+    return const_cast<MaterializeTemporaryExpr *>(this)->getExtendingDecl();
   }
 
-  void setExtendingDecl(const ValueDecl *ExtendedBy, unsigned ManglingNumber);
+  void setExtendingDecl(ValueDecl *ExtendedBy, unsigned ManglingNumber);
 
   unsigned getManglingNumber() const {
-    return State.is<Stmt *>() ? 0 : State.get<ExtraState *>()->ManglingNumber;
+    return State.is<Stmt *>() ? 0
+                              : State.get<LifetimeExtendedTemporaryDecl *>()
+                                    ->getManglingNumber();
   }
 
   /// Determine whether this materialized temporary is bound to an
@@ -4494,11 +4490,11 @@ class MaterializeTemporaryExpr : public Expr {
   }
 
   SourceLocation getBeginLoc() const LLVM_READONLY {
-    return getTemporary()->getBeginLoc();
+    return getSubExpr()->getBeginLoc();
   }
 
   SourceLocation getEndLoc() const LLVM_READONLY {
-    return getTemporary()->getEndLoc();
+    return getSubExpr()->getEndLoc();
   }
 
   static bool classof(const Stmt *T) {
@@ -4507,20 +4503,18 @@ class MaterializeTemporaryExpr : public Expr {
 
   // Iterators
   child_range children() {
-    if (State.is<Stmt *>())
-      return child_range(State.getAddrOfPtr1(), State.getAddrOfPtr1() + 1);
-
-    auto ES = State.get<ExtraState *>();
-    return child_range(&ES->Temporary, &ES->Temporary + 1);
+    return State.is<Stmt *>()
+               ? child_range(State.getAddrOfPtr1(), State.getAddrOfPtr1() + 1)
+               : State.get<LifetimeExtendedTemporaryDecl *>()->childrenExpr();
   }
 
   const_child_range children() const {
-    if (State.is<Stmt *>())
-      return const_child_range(State.getAddrOfPtr1(),
-                               State.getAddrOfPtr1() + 1);
-
-    auto ES = State.get<ExtraState *>();
-    return const_child_range(&ES->Temporary, &ES->Temporary + 1);
+    return State.is<Stmt *>()
+               ? const_child_range(State.getAddrOfPtr1(),
+                                   State.getAddrOfPtr1() + 1)
+               : const_cast<const LifetimeExtendedTemporaryDecl *>(
+                     State.get<LifetimeExtendedTemporaryDecl *>())
+                     ->childrenExpr();
   }
 };
 

clang/include/clang/AST/RecursiveASTVisitor.h

@@ -1435,6 +1435,10 @@ DEF_TRAVERSE_DECL(CapturedDecl, {
 
 DEF_TRAVERSE_DECL(EmptyDecl, {})
 
+DEF_TRAVERSE_DECL(LifetimeExtendedTemporaryDecl, {
+  TRY_TO(TraverseStmt(D->getTemporaryExpr()));
+})
+
 DEF_TRAVERSE_DECL(FileScopeAsmDecl,
                   { TRY_TO(TraverseStmt(D->getAsmString())); })
 
@@ -2632,10 +2636,16 @@ DEF_TRAVERSE_STMT(SizeOfPackExpr, {})
 DEF_TRAVERSE_STMT(SubstNonTypeTemplateParmPackExpr, {})
 DEF_TRAVERSE_STMT(SubstNonTypeTemplateParmExpr, {})
 DEF_TRAVERSE_STMT(FunctionParmPackExpr, {})
-DEF_TRAVERSE_STMT(MaterializeTemporaryExpr, {})
 DEF_TRAVERSE_STMT(CXXFoldExpr, {})
 DEF_TRAVERSE_STMT(AtomicExpr, {})
 
+DEF_TRAVERSE_STMT(MaterializeTemporaryExpr, {
+  if (S->getLifetimeExtendedTemporaryDecl()) {
+    TRY_TO(TraverseLifetimeExtendedTemporaryDecl(
+        S->getLifetimeExtendedTemporaryDecl()));
+    ShouldVisitChildren = false;
+  }
+})
 // For coroutines expressions, traverse either the operand
 // as written or the implied calls, depending on what the
 // derived class requests.

clang/include/clang/ASTMatchers/ASTMatchers.h

@@ -6621,8 +6621,8 @@ AST_MATCHER_P(Expr, ignoringElidableConstructorCall,
     if (CtorExpr->isElidable()) {
       if (const auto *MaterializeTemp =
               dyn_cast<MaterializeTemporaryExpr>(CtorExpr->getArg(0))) {
-        return InnerMatcher.matches(*MaterializeTemp->GetTemporaryExpr(),
-                                    Finder, Builder);
+        return InnerMatcher.matches(*MaterializeTemp->getSubExpr(), Finder,
+                                    Builder);
       }
     }
   }

clang/include/clang/Basic/DeclNodes.td

@@ -100,4 +100,5 @@ def OMPThreadPrivate : DeclNode<Decl>;
 def OMPAllocate : DeclNode<Decl>;
 def OMPRequires : DeclNode<Decl>;
 def Empty : DeclNode<Decl>;
+def LifetimeExtendedTemporary : DeclNode<Decl>;
 

clang/include/clang/Sema/Template.h

@@ -464,8 +464,9 @@ class VarDecl;
 #define OBJCPROPERTY(DERIVED, BASE)
 #define OBJCPROPERTYIMPL(DERIVED, BASE)
 #define EMPTY(DERIVED, BASE)
+#define LIFETIMEEXTENDEDTEMPORARY(DERIVED, BASE)
 
-// Decls which use special-case instantiation code.
+    // Decls which use special-case instantiation code.
 #define BLOCK(DERIVED, BASE)
 #define CAPTURED(DERIVED, BASE)
 #define IMPLICITPARAM(DERIVED, BASE)

clang/include/clang/Serialization/ASTBitCodes.h

@@ -1537,6 +1537,9 @@ namespace serialization {
       /// An EmptyDecl record.
       DECL_EMPTY,
 
+      /// An LifetimeExtendedTemporaryDecl record.
+      DECL_LIFETIME_EXTENDED_TEMPORARY,
+
       /// An ObjCTypeParamDecl record.
       DECL_OBJC_TYPE_PARAM,