[clang] 076b120 - CFG: Destroy temporaries in (a, b) expression in the correct order.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-08-05T14:52:53-07:00
New Revision: 076b120bebfd727b502208601012a44ab2e1028e

URL: https://github.com/llvm/llvm-project/commit/076b120bebfd727b502208601012a44ab2e1028e
DIFF: https://github.com/llvm/llvm-project/commit/076b120bebfd727b502208601012a44ab2e1028e.diff

LOG: CFG: Destroy temporaries in (a,b) expression in the correct order.

Added: 
    

Modified: 
    clang/lib/Analysis/CFG.cpp
    clang/test/Analysis/cfg.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index fc74226951a4..cde0bf448ecb 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -4773,11 +4773,11 @@ CFGBlock *CFGBuilder::VisitChildrenForTemporaryDtors(Stmt *E,
 CFGBlock *CFGBuilder::VisitBinaryOperatorForTemporaryDtors(
     BinaryOperator *E, bool ExternallyDestructed, TempDtorContext &Context) {
   if (E->isCommaOp()) {
-    // For comma operator LHS expression is visited
-    // before RHS expression. For destructors visit them in reverse order.
-    CFGBlock *RHSBlock = VisitForTemporaryDtors(E->getRHS(), ExternallyDestructed, Context);
+    // For the comma operator, the LHS expression is evaluated before the RHS
+    // expression, so prepend temporary destructors for the LHS first.
     CFGBlock *LHSBlock = VisitForTemporaryDtors(E->getLHS(), false, Context);
-    return LHSBlock ? LHSBlock : RHSBlock;
+    CFGBlock *RHSBlock = VisitForTemporaryDtors(E->getRHS(), ExternallyDestructed, Context);
+    return RHSBlock ? RHSBlock : LHSBlock;
   }
 
   if (E->isLogicalOp()) {
@@ -4798,19 +4798,15 @@ CFGBlock *CFGBuilder::VisitBinaryOperatorForTemporaryDtors(
   }
 
   if (E->isAssignmentOp()) {
-    // For assignment operator (=) LHS expression is visited
-    // before RHS expression. For destructors visit them in reverse order.
+    // For assignment operators, the RHS expression is evaluated before the LHS
+    // expression, so prepend temporary destructors for the RHS first.
     CFGBlock *RHSBlock = VisitForTemporaryDtors(E->getRHS(), false, Context);
     CFGBlock *LHSBlock = VisitForTemporaryDtors(E->getLHS(), false, Context);
     return LHSBlock ? LHSBlock : RHSBlock;
   }
 
-  // For any other binary operator RHS expression is visited before
-  // LHS expression (order of children). For destructors visit them in reverse
-  // order.
-  CFGBlock *LHSBlock = VisitForTemporaryDtors(E->getLHS(), false, Context);
-  CFGBlock *RHSBlock = VisitForTemporaryDtors(E->getRHS(), false, Context);
-  return RHSBlock ? RHSBlock : LHSBlock;
+  // Any other operator is visited normally.
+  return VisitChildrenForTemporaryDtors(E, ExternallyDestructed, Context);
 }
 
 CFGBlock *CFGBuilder::VisitCXXBindTemporaryExprForTemporaryDtors(

diff  --git a/clang/test/Analysis/cfg.cpp b/clang/test/Analysis/cfg.cpp
index 0159a3c0488c..5f2b365eeb4e 100644
--- a/clang/test/Analysis/cfg.cpp
+++ b/clang/test/Analysis/cfg.cpp
@@ -575,6 +575,24 @@ int vla_evaluate(int x) {
   return x;
 }
 
+// CHECK-LABEL: void CommaTemp::f()
+// CHECK:       [B1]
+// CHECK-NEXT:    1: CommaTemp::A() (CXXConstructExpr,
+// CHECK-NEXT:    2: [B1.1] (BindTemporary)
+// CHECK-NEXT:    3: CommaTemp::B() (CXXConstructExpr,
+// CHECK-NEXT:    4: [B1.3] (BindTemporary)
+// CHECK-NEXT:    5: ... , [B1.4]
+// CHECK-NEXT:    6: ~CommaTemp::B() (Temporary object destructor)
+// CHECK-NEXT:    7: ~CommaTemp::A() (Temporary object destructor)
+namespace CommaTemp {
+  struct A { ~A(); };
+  struct B { ~B(); };
+  void f();
+}
+void CommaTemp::f() {
+  A(), B();
+}
+
 // CHECK-LABEL: template<> int *PR18472<int>()
 // CHECK: [B2 (ENTRY)]
 // CHECK-NEXT:   Succs (1): B1