[CodeGen] Emit destructor calls to destruct compound literals

Fix a bug in IRGen where it wasn't destructing compound literals in C
that are ObjC pointer arrays or non-trivial structs. Also diagnose jumps
that enter or exit the lifetime of the compound literals.

rdar://problem/51867864

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

Author: ahatanaka

clang/include/clang/AST/ASTImporter.h

@@ -16,6 +16,7 @@
 
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclarationName.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
@@ -349,6 +350,10 @@ class TypeSourceInfo;
       return ToOrErr.takeError();
     }
 
+    /// Import cleanup objects owned by ExprWithCleanup.
+    llvm::Expected<ExprWithCleanups::CleanupObject>
+    Import(ExprWithCleanups::CleanupObject From);
+
     /// Import the given type from the "from" context into the "to"
     /// context. A null type is imported as a null type (no error).
     ///

clang/include/clang/AST/ExprCXX.h

@@ -3321,13 +3321,15 @@ class DependentScopeDeclRefExpr final
 /// literal is the extent of the enclosing scope.
 class ExprWithCleanups final
     : public FullExpr,
-      private llvm::TrailingObjects<ExprWithCleanups, BlockDecl *> {
+      private llvm::TrailingObjects<
+          ExprWithCleanups,
+          llvm::PointerUnion<BlockDecl *, CompoundLiteralExpr *>> {
 public:
   /// The type of objects that are kept in the cleanup.
-  /// It's useful to remember the set of blocks;  we could also
-  /// remember the set of temporaries, but there's currently
-  /// no need.
-  using CleanupObject = BlockDecl *;
+  /// It's useful to remember the set of blocks and block-scoped compound
+  /// literals; we could also remember the set of temporaries, but there's
+  /// currently no need.
+  using CleanupObject = llvm::PointerUnion<BlockDecl *, CompoundLiteralExpr *>;
 
 private:
   friend class ASTStmtReader;

clang/include/clang/AST/TextNodeDumper.h

@@ -184,6 +184,7 @@ class TextNodeDumper
   void dumpBareDeclRef(const Decl *D);
   void dumpName(const NamedDecl *ND);
   void dumpAccessSpecifier(AccessSpecifier AS);
+  void dumpCleanupObject(const ExprWithCleanups::CleanupObject &C);
 
   void dumpDeclRef(const Decl *D, StringRef Label = {});
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -5543,6 +5543,8 @@ def note_enters_block_captures_weak : Note<
 def note_enters_block_captures_non_trivial_c_struct : Note<
   "jump enters lifetime of block which captures a C struct that is non-trivial "
   "to destroy">;
+def note_enters_compound_literal_scope : Note<
+  "jump enters lifetime of a compound literal that is non-trivial to destruct">;
 
 def note_exits_cleanup : Note<
   "jump exits scope of variable with __attribute__((cleanup))">;
@@ -5586,6 +5588,8 @@ def note_exits_block_captures_weak : Note<
 def note_exits_block_captures_non_trivial_c_struct : Note<
   "jump exits lifetime of block which captures a C struct that is non-trivial "
   "to destroy">;
+def note_exits_compound_literal_scope : Note<
+  "jump exits lifetime of a compound literal that is non-trivial to destruct">;
 
 def err_func_returning_qualified_void : ExtWarn<
   "function cannot return qualified void type %0">,

clang/include/clang/Sema/Sema.h

@@ -613,9 +613,8 @@ class Sema final {
   CleanupInfo Cleanup;
 
   /// ExprCleanupObjects - This is the stack of objects requiring
-  /// cleanup that are created by the current full expression.  The
-  /// element type here is ExprWithCleanups::Object.
-  SmallVector<BlockDecl*, 8> ExprCleanupObjects;
+  /// cleanup that are created by the current full expression.
+  SmallVector<ExprWithCleanups::CleanupObject, 8> ExprCleanupObjects;
 
   /// Store a set of either DeclRefExprs or MemberExprs that contain a reference
   /// to a variable (constant) that may or may not be odr-used in this Expr, and

clang/include/clang/Serialization/ASTBitCodes.h

@@ -1900,6 +1900,9 @@ namespace serialization {
       CTOR_INITIALIZER_INDIRECT_MEMBER
     };
 
+    /// Kinds of cleanup objects owned by ExprWithCleanups.
+    enum CleanupObjectKind { COK_Block, COK_CompoundLiteral };
+
     /// Describes the redeclarations of a declaration.
     struct LocalRedeclarationsInfo {
       // The ID of the first declaration

clang/lib/AST/ASTImporter.cpp

@@ -7910,6 +7910,18 @@ void ASTImporter::RegisterImportedDecl(Decl *FromD, Decl *ToD) {
   MapImported(FromD, ToD);
 }
 
+llvm::Expected<ExprWithCleanups::CleanupObject>
+ASTImporter::Import(ExprWithCleanups::CleanupObject From) {
+  if (auto *CLE = From.dyn_cast<CompoundLiteralExpr *>()) {
+    if (Expected<Expr *> R = Import(CLE))
+      return ExprWithCleanups::CleanupObject(cast<CompoundLiteralExpr>(*R));
+  }
+
+  // FIXME: Handle BlockDecl when we implement importing BlockExpr in
+  //        ASTNodeImporter.
+  return make_error<ImportError>(ImportError::UnsupportedConstruct);
+}
+
 Expected<QualType> ASTImporter::Import(QualType FromT) {
   if (FromT.isNull())
     return QualType{};

clang/lib/AST/JSONNodeDumper.cpp

@@ -1334,7 +1334,16 @@ void JSONNodeDumper::VisitExprWithCleanups(const ExprWithCleanups *EWC) {
   if (EWC->getNumObjects()) {
     JOS.attributeArray("cleanups", [this, EWC] {
       for (const ExprWithCleanups::CleanupObject &CO : EWC->getObjects())
-        JOS.value(createBareDeclRef(CO));
+        if (auto *BD = CO.dyn_cast<BlockDecl *>()) {
+          JOS.value(createBareDeclRef(BD));
+        } else if (auto *CLE = CO.dyn_cast<CompoundLiteralExpr *>()) {
+          llvm::json::Object Obj;
+          Obj["id"] = createPointerRepresentation(CLE);
+          Obj["kind"] = CLE->getStmtClassName();
+          JOS.value(std::move(Obj));
+        } else {
+          llvm_unreachable("unexpected cleanup object type");
+        }
     });
   }
 }

clang/lib/AST/TextNodeDumper.cpp

@@ -448,6 +448,23 @@ void TextNodeDumper::dumpAccessSpecifier(AccessSpecifier AS) {
   }
 }
 
+void TextNodeDumper::dumpCleanupObject(
+    const ExprWithCleanups::CleanupObject &C) {
+  if (auto *BD = C.dyn_cast<BlockDecl *>())
+    dumpDeclRef(BD, "cleanup");
+  else if (auto *CLE = C.dyn_cast<CompoundLiteralExpr *>())
+    AddChild([=] {
+      OS << "cleanup ";
+      {
+        ColorScope Color(OS, ShowColors, StmtColor);
+        OS << CLE->getStmtClassName();
+      }
+      dumpPointer(CLE);
+    });
+  else
+    llvm_unreachable("unexpected cleanup type");
+}
+
 void TextNodeDumper::dumpDeclRef(const Decl *D, StringRef Label) {
   if (!D)
     return;
@@ -950,7 +967,7 @@ void TextNodeDumper::VisitMaterializeTemporaryExpr(
 
 void TextNodeDumper::VisitExprWithCleanups(const ExprWithCleanups *Node) {
   for (unsigned i = 0, e = Node->getNumObjects(); i != e; ++i)
-    dumpDeclRef(Node->getObject(i), "cleanup");
+    dumpCleanupObject(Node->getObject(i));
 }
 
 void TextNodeDumper::VisitSizeOfPackExpr(const SizeOfPackExpr *Node) {

clang/lib/CodeGen/CGBlocks.cpp

@@ -860,13 +860,13 @@ static void enterBlockScope(CodeGenFunction &CGF, BlockDecl *block) {
 }
 
 /// Enter a full-expression with a non-trivial number of objects to
-/// clean up.  This is in this file because, at the moment, the only
-/// kind of cleanup object is a BlockDecl*.
+/// clean up.
 void CodeGenFunction::enterNonTrivialFullExpression(const FullExpr *E) {
   if (const auto EWC = dyn_cast<ExprWithCleanups>(E)) {
     assert(EWC->getNumObjects() != 0);
     for (const ExprWithCleanups::CleanupObject &C : EWC->getObjects())
-      enterBlockScope(*this, C);
+      if (auto *BD = C.dyn_cast<BlockDecl *>())
+        enterBlockScope(*this, BD);
   }
 }
 

clang/lib/CodeGen/CGExpr.cpp

@@ -4268,6 +4268,14 @@ LValue CodeGenFunction::EmitCompoundLiteralLValue(const CompoundLiteralExpr *E){
   EmitAnyExprToMem(InitExpr, DeclPtr, E->getType().getQualifiers(),
                    /*Init*/ true);
 
+  // Block-scope compound literals are destroyed at the end of the enclosing
+  // scope in C.
+  if (!getLangOpts().CPlusPlus)
+    if (QualType::DestructionKind DtorKind = E->getType().isDestructedType())
+      pushLifetimeExtendedDestroy(getCleanupKind(DtorKind), DeclPtr,
+                                  E->getType(), getDestroyer(DtorKind),
+                                  DtorKind & EHCleanup);
+
   return Result;
 }
 

clang/lib/CodeGen/CGExprAgg.cpp

@@ -659,7 +659,21 @@ AggExprEmitter::VisitCompoundLiteralExpr(CompoundLiteralExpr *E) {
   }
 
   AggValueSlot Slot = EnsureSlot(E->getType());
+
+  // Block-scope compound literals are destroyed at the end of the enclosing
+  // scope in C.
+  bool Destruct =
+      !CGF.getLangOpts().CPlusPlus && !Slot.isExternallyDestructed();
+  if (Destruct)
+    Slot.setExternallyDestructed();
+
   CGF.EmitAggExpr(E->getInitializer(), Slot);
+
+  if (Destruct)
+    if (QualType::DestructionKind DtorKind = E->getType().isDestructedType())
+      CGF.pushLifetimeExtendedDestroy(
+          CGF.getCleanupKind(DtorKind), Slot.getAddress(), E->getType(),
+          CGF.getDestroyer(DtorKind), DtorKind & EHCleanup);
 }
 
 /// Attempt to look through various unimportant expressions to find a

clang/lib/CodeGen/CGExprScalar.cpp

@@ -556,6 +556,11 @@ class ScalarExprEmitter
   Value *VisitMemberExpr(MemberExpr *E);
   Value *VisitExtVectorElementExpr(Expr *E) { return EmitLoadOfLValue(E); }
   Value *VisitCompoundLiteralExpr(CompoundLiteralExpr *E) {
+    // Strictly speaking, we shouldn't be calling EmitLoadOfLValue, which
+    // transitively calls EmitCompoundLiteralLValue, here in C++ since compound
+    // literals aren't l-values in C++. We do so simply because that's the
+    // cleanest way to handle compound literals in C++.
+    // See the discussion here: https://reviews.llvm.org/D64464
     return EmitLoadOfLValue(E);
   }
 

clang/lib/Sema/JumpDiagnostics.cpp

@@ -75,6 +75,7 @@ class JumpScopeChecker {
   void BuildScopeInformation(Decl *D, unsigned &ParentScope);
   void BuildScopeInformation(VarDecl *D, const BlockDecl *BDecl,
                              unsigned &ParentScope);
+  void BuildScopeInformation(CompoundLiteralExpr *CLE, unsigned &ParentScope);
   void BuildScopeInformation(Stmt *S, unsigned &origParentScope);
 
   void VerifyJumps();
@@ -276,6 +277,16 @@ void JumpScopeChecker::BuildScopeInformation(VarDecl *D,
   }
 }
 
+/// Build scope information for compound literals of C struct types that are
+/// non-trivial to destruct.
+void JumpScopeChecker::BuildScopeInformation(CompoundLiteralExpr *CLE,
+                                             unsigned &ParentScope) {
+  unsigned InDiag = diag::note_enters_compound_literal_scope;
+  unsigned OutDiag = diag::note_exits_compound_literal_scope;
+  Scopes.push_back(GotoScope(ParentScope, InDiag, OutDiag, CLE->getExprLoc()));
+  ParentScope = Scopes.size() - 1;
+}
+
 /// BuildScopeInformation - The statements from CI to CE are known to form a
 /// coherent VLA scope with a specified parent node.  Walk through the
 /// statements, adding any labels or gotos to LabelAndGotoScopes and recursively
@@ -529,11 +540,15 @@ void JumpScopeChecker::BuildScopeInformation(Stmt *S,
     // implementable but a lot of work which we haven't felt up to doing.
     ExprWithCleanups *EWC = cast<ExprWithCleanups>(S);
     for (unsigned i = 0, e = EWC->getNumObjects(); i != e; ++i) {
-      const BlockDecl *BDecl = EWC->getObject(i);
-      for (const auto &CI : BDecl->captures()) {
-        VarDecl *variable = CI.getVariable();
-        BuildScopeInformation(variable, BDecl, origParentScope);
-      }
+      if (auto *BDecl = EWC->getObject(i).dyn_cast<BlockDecl *>())
+        for (const auto &CI : BDecl->captures()) {
+          VarDecl *variable = CI.getVariable();
+          BuildScopeInformation(variable, BDecl, origParentScope);
+        }
+      else if (auto *CLE = EWC->getObject(i).dyn_cast<CompoundLiteralExpr *>())
+        BuildScopeInformation(CLE, origParentScope);
+      else
+        llvm_unreachable("unexpected cleanup object type");
     }
     break;
   }

clang/lib/Sema/SemaExpr.cpp

@@ -6251,14 +6251,24 @@ Sema::BuildCompoundLiteralExpr(SourceLocation LParenLoc, TypeSourceInfo *TInfo,
     return ExprError();
   }
 
-  // Compound literals that have automatic storage duration are destroyed at
-  // the end of the scope. Emit diagnostics if it is or contains a C union type
-  // that is non-trivial to destruct.
-  if (!isFileScope)
+  if (!isFileScope && !getLangOpts().CPlusPlus) {
+    // Compound literals that have automatic storage duration are destroyed at
+    // the end of the scope in C; in C++, they're just temporaries.
+
+    // Emit diagnostics if it is or contains a C union type that is non-trivial
+    // to destruct.
     if (E->getType().hasNonTrivialToPrimitiveDestructCUnion())
       checkNonTrivialCUnion(E->getType(), E->getExprLoc(),
                             NTCUC_CompoundLiteral, NTCUK_Destruct);
 
+    // Diagnose jumps that enter or exit the lifetime of the compound literal.
+    if (literalType.isDestructedType()) {
+      Cleanup.setExprNeedsCleanups(true);
+      ExprCleanupObjects.push_back(E);
+      getCurFunction()->setHasBranchProtectedScope();
+    }
+  }
+
   if (E->getType().hasNonTrivialToPrimitiveDefaultInitializeCUnion() ||
       E->getType().hasNonTrivialToPrimitiveCopyCUnion())
     checkNonTrivialCUnionInInitializer(E->getInitializer(),

clang/lib/Serialization/ASTReaderStmt.cpp

@@ -1819,9 +1819,17 @@ void ASTStmtReader::VisitExprWithCleanups(ExprWithCleanups *E) {
 
   unsigned NumObjects = Record.readInt();
   assert(NumObjects == E->getNumObjects());
-  for (unsigned i = 0; i != NumObjects; ++i)
-    E->getTrailingObjects<BlockDecl *>()[i] =
-        readDeclAs<BlockDecl>();
+  for (unsigned i = 0; i != NumObjects; ++i) {
+    unsigned CleanupKind = Record.readInt();
+    ExprWithCleanups::CleanupObject Obj;
+    if (CleanupKind == COK_Block)
+      Obj = readDeclAs<BlockDecl>();
+    else if (CleanupKind == COK_CompoundLiteral)
+      Obj = cast<CompoundLiteralExpr>(Record.readSubExpr());
+    else
+      llvm_unreachable("unexpected cleanup object type");
+    E->getTrailingObjects<ExprWithCleanups::CleanupObject>()[i] = Obj;
+  }
 
   E->ExprWithCleanupsBits.CleanupsHaveSideEffects = Record.readInt();
   E->SubExpr = Record.readSubExpr();

clang/lib/Serialization/ASTWriterStmt.cpp

@@ -1721,8 +1721,15 @@ void ASTStmtWriter::VisitCXXPseudoDestructorExpr(CXXPseudoDestructorExpr *E) {
 void ASTStmtWriter::VisitExprWithCleanups(ExprWithCleanups *E) {
   VisitExpr(E);
   Record.push_back(E->getNumObjects());
-  for (unsigned i = 0, e = E->getNumObjects(); i != e; ++i)
-    Record.AddDeclRef(E->getObject(i));
+  for (auto &Obj : E->getObjects()) {
+    if (auto *BD = Obj.dyn_cast<BlockDecl *>()) {
+      Record.push_back(serialization::COK_Block);
+      Record.AddDeclRef(BD);
+    } else if (auto *CLE = Obj.dyn_cast<CompoundLiteralExpr *>()) {
+      Record.push_back(serialization::COK_CompoundLiteral);
+      Record.AddStmt(CLE);
+    }
+  }
 
   Record.push_back(E->cleanupsHaveSideEffects());
   Record.AddStmt(E->getSubExpr());

clang/test/AST/ast-dump-objc-arc-json.m

@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -x objective-c -fobjc-arc -ast-dump=json -ast-dump-filter Test %s | FileCheck %s
+
+typedef struct {
+  id f;
+} S;
+
+id TestCompoundLiteral(id a) {
+  return ((S){ .f = a }).f;
+}
+
+// CHECK:  "kind": "ExprWithCleanups",
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:    "offset": 202,
+// CHECK-NEXT:    "col": 10,
+// CHECK-NEXT:    "tokLen": 1
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:    "offset": 218,
+// CHECK-NEXT:    "col": 26,
+// CHECK-NEXT:    "tokLen": 1
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "desugaredQualType": "id",
+// CHECK-NEXT:   "qualType": "id",
+// CHECK-NEXT:   "typeAliasDeclId": "0x{{.*}}"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "valueCategory": "rvalue",
+// CHECK-NEXT:  "cleanupsHaveSideEffects": true,
+// CHECK-NEXT:  "cleanups": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:    "id": "0x{{.*}}",
+// CHECK-NEXT:    "kind": "CompoundLiteralExpr"
+// CHECK-NEXT:   }
+// CHECK-NEXT:  ],

clang/test/AST/ast-dump-stmt.m

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -Wno-unused -fblocks -fobjc-exceptions -ast-dump -ast-dump-filter Test %s | FileCheck -strict-whitespace %s
+// RUN: %clang_cc1 -Wno-unused -fobjc-arc -fblocks -fobjc-exceptions -ast-dump -ast-dump-filter Test %s | FileCheck -strict-whitespace %s
 
 void TestBlockExpr(int x) {
   ^{ x; };
@@ -34,3 +34,16 @@ void TestObjCAtCatchStmt() {
 // CHECK-NEXT:   ObjCAtCatchStmt{{.*}} catch all
 // CHECK-NEXT:     CompoundStmt
 // CHECK-NEXT:   ObjCAtFinallyStmt
+
+typedef struct {
+  id f;
+} S;
+
+id TestCompoundLiteral(id a) {
+  return ((S){ .f = a }).f;
+}
+
+// CHECK:     FunctionDecl{{.*}}TestCompoundLiteral
+// CHECK:       ExprWithCleanups
+// CHECK-NEXT:    cleanup CompoundLiteralExpr
+// CHECK:           CompoundLiteralExpr{{.*}}'S':'S' lvalue