NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.

While landing https://reviews.llvm.org/D43428 as 
https://reviews.llvm.org/rC325966, i caused an internal compiler error of an 
MSVC 2015 compiler on a buildbot. Then i tried to hotfix it in 
https://reviews.llvm.org/rC325969 (which didn't help) and in 
https://reviews.llvm.org/rC325978 (which suddenly helped, i.e. simply moving 
the existing code around has suppressed the crash). Then i committed 
https://reviews.llvm.org/D43481 as https://reviews.llvm.org/rC326016 and MSVC 
crashed again. Then i committed https://reviews.llvm.org/rC326021 as a hotfix, 
and it seemed to help again.

The problem is, i lost some code in https://reviews.llvm.org/rC326021. Namely i 
forgot to check that the cast that's mentioned in 
https://reviews.llvm.org/D43481 / https://reviews.llvm.org/rC326016 is a 
NoOp-kind cast.

Bring back the check for the cast kind, add a test for it.

The test exposes an interesting edge case that we don't support yet: implicit 
constructor conversion. For now we only support explicit constructor conversion 
(i.e. functional cast as in https://reviews.llvm.org/D43480). For whatever 
reason, in this case the AST doesn't contain a `CXXBindTemporaryExpr` even if 
the object requires destruction, so the assertion in 
`ConstructionContext::createFromLayers` that says that any construction context 
with an MTE but without the respective BTE should be of an object with a 
trivial destructor (which isn't a problem here because the object is 
lifetime-extended anyway).


Repository:
  rC Clang

https://reviews.llvm.org/D43840

Files:
  lib/Analysis/CFG.cpp
  test/Analysis/cfg-rich-constructors.cpp


Index: test/Analysis/cfg-rich-constructors.cpp
===================================================================
--- test/Analysis/cfg-rich-constructors.cpp
+++ test/Analysis/cfg-rich-constructors.cpp
@@ -484,4 +484,35 @@
 void constructorInTernaryCondition() {
   const D &d = D(1) ? D(2) : D(3);
 }
+
 } // end namespace temporary_object_expr_with_dtors
+
+namespace implicit_constructor_conversion {
+
+class A {};
+A get();
+
+class B {
+public:
+  B(const A &);
+  ~B() {}
+};
+
+// FIXME: Find construction context for the implicit constructor conversion.
+// CHECK: void implicitConstructionConversionFromFunctionValue()
+// CHECK:          1: get
+// CHECK-NEXT:     2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, class 
implicit_constructor_conver
+// CHECK-NEXT:     3: [B1.2]()
+// CHECK-NEXT:     4: [B1.3] (ImplicitCastExpr, NoOp, const class 
implicit_constructor_conversion::A)
+// CHECK-NEXT:     5: [B1.4]
+// CHECK-NEXT:     6: [B1.5] (CXXConstructExpr, class 
implicit_constructor_conversion::B)
+// CHECK-NEXT:     7: [B1.6] (ImplicitCastExpr, ConstructorConversion, class 
implicit_constructor_convers
+// CHECK-NEXT:     8: [B1.7] (ImplicitCastExpr, NoOp, const class 
implicit_constructor_conversion::B)
+// CHECK-NEXT:     9: [B1.8]
+// CHECK-NEXT:    10: const implicit_constructor_conversion::B &b = get();
+// CHECK-NEXT:    11: [B1.10].~B() (Implicit destructor)
+void implicitConstructionConversionFromFunctionValue() {
+  const B &b = get(); // no-crash
+}
+
+} // end namespace implicit_constructor_conversion
Index: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -1200,7 +1200,8 @@
   }
   case Stmt::ImplicitCastExprClass: {
     auto *Cast = cast<ImplicitCastExpr>(Child);
-    findConstructionContexts(Layer, Cast->getSubExpr());
+    if (Cast->getCastKind() == CK_NoOp)
+      findConstructionContexts(Layer, Cast->getSubExpr());
     break;
   }
   case Stmt::CXXBindTemporaryExprClass: {


Index: test/Analysis/cfg-rich-constructors.cpp
===================================================================
--- test/Analysis/cfg-rich-constructors.cpp
+++ test/Analysis/cfg-rich-constructors.cpp
@@ -484,4 +484,35 @@
 void constructorInTernaryCondition() {
   const D &d = D(1) ? D(2) : D(3);
 }
+
 } // end namespace temporary_object_expr_with_dtors
+
+namespace implicit_constructor_conversion {
+
+class A {};
+A get();
+
+class B {
+public:
+  B(const A &);
+  ~B() {}
+};
+
+// FIXME: Find construction context for the implicit constructor conversion.
+// CHECK: void implicitConstructionConversionFromFunctionValue()
+// CHECK:          1: get
+// CHECK-NEXT:     2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, class implicit_constructor_conver
+// CHECK-NEXT:     3: [B1.2]()
+// CHECK-NEXT:     4: [B1.3] (ImplicitCastExpr, NoOp, const class implicit_constructor_conversion::A)
+// CHECK-NEXT:     5: [B1.4]
+// CHECK-NEXT:     6: [B1.5] (CXXConstructExpr, class implicit_constructor_conversion::B)
+// CHECK-NEXT:     7: [B1.6] (ImplicitCastExpr, ConstructorConversion, class implicit_constructor_convers
+// CHECK-NEXT:     8: [B1.7] (ImplicitCastExpr, NoOp, const class implicit_constructor_conversion::B)
+// CHECK-NEXT:     9: [B1.8]
+// CHECK-NEXT:    10: const implicit_constructor_conversion::B &b = get();
+// CHECK-NEXT:    11: [B1.10].~B() (Implicit destructor)
+void implicitConstructionConversionFromFunctionValue() {
+  const B &b = get(); // no-crash
+}
+
+} // end namespace implicit_constructor_conversion
Index: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -1200,7 +1200,8 @@
   }
   case Stmt::ImplicitCastExprClass: {
     auto *Cast = cast<ImplicitCastExpr>(Child);
-    findConstructionContexts(Layer, Cast->getSubExpr());
+    if (Cast->getCastKind() == CK_NoOp)
+      findConstructionContexts(Layer, Cast->getSubExpr());
     break;
   }
   case Stmt::CXXBindTemporaryExprClass: {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to