[PATCH] D49215: [analyzer] Admit that some copy/move constructors have more than one argument.

2018-07-16 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337229: [CFG] [analyzer] Allow elidable copies to have more 
than one arguments. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49215?vs=155105=155800#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49215

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


Index: cfe/trunk/lib/Analysis/CFG.cpp
===
--- cfe/trunk/lib/Analysis/CFG.cpp
+++ cfe/trunk/lib/Analysis/CFG.cpp
@@ -1263,7 +1263,6 @@
 // Support pre-C++17 copy elision AST.
 auto *CE = cast(Child);
 if (BuildOpts.MarkElidedCXXConstructors && CE->isElidable()) {
-  assert(CE->getNumArgs() == 1);
   findConstructionContexts(withExtraLayer(CE), CE->getArg(0));
 }
 
Index: cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
===
--- cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
+++ cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
@@ -878,3 +878,26 @@
   useDByReference(D());
 }
 } // end namespace argument_constructors
+
+namespace copy_elision_with_extra_arguments {
+class C {
+public:
+  C();
+  C(const C , int x = 0);
+};
+
+// CHECK: void testCopyElisionWhenCopyConstructorHasExtraArguments()
+// CHECK:[B1]
+// CXX11-ELIDE-NEXT: 1: copy_elision_with_extra_arguments::C() 
(CXXConstructExpr, [B1.3], [B1.5], class copy_elision_with_extra_arguments::C)
+// CXX11-NOELIDE-NEXT: 1: copy_elision_with_extra_arguments::C() 
(CXXConstructExpr, [B1.3], class copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 2: [B1.1] (ImplicitCastExpr, NoOp, const class 
copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 3: [B1.2]
+// CXX11-NEXT: 4:
+// CXX11-NEXT: 5: [B1.3] (CXXConstructExpr, [B1.6], class 
copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 6: copy_elision_with_extra_arguments::C c = 
copy_elision_with_extra_arguments::C();
+// CXX17-NEXT: 1: copy_elision_with_extra_arguments::C() 
(CXXConstructExpr, [B1.2], class copy_elision_with_extra_arguments::C)
+// CXX17-NEXT: 2: copy_elision_with_extra_arguments::C c = 
copy_elision_with_extra_arguments::C();
+void testCopyElisionWhenCopyConstructorHasExtraArguments() {
+  C c = C();
+}
+} // namespace copy_elision_with_extra_arguments


Index: cfe/trunk/lib/Analysis/CFG.cpp
===
--- cfe/trunk/lib/Analysis/CFG.cpp
+++ cfe/trunk/lib/Analysis/CFG.cpp
@@ -1263,7 +1263,6 @@
 // Support pre-C++17 copy elision AST.
 auto *CE = cast(Child);
 if (BuildOpts.MarkElidedCXXConstructors && CE->isElidable()) {
-  assert(CE->getNumArgs() == 1);
   findConstructionContexts(withExtraLayer(CE), CE->getArg(0));
 }
 
Index: cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
===
--- cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
+++ cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
@@ -878,3 +878,26 @@
   useDByReference(D());
 }
 } // end namespace argument_constructors
+
+namespace copy_elision_with_extra_arguments {
+class C {
+public:
+  C();
+  C(const C , int x = 0);
+};
+
+// CHECK: void testCopyElisionWhenCopyConstructorHasExtraArguments()
+// CHECK:[B1]
+// CXX11-ELIDE-NEXT: 1: copy_elision_with_extra_arguments::C() (CXXConstructExpr, [B1.3], [B1.5], class copy_elision_with_extra_arguments::C)
+// CXX11-NOELIDE-NEXT: 1: copy_elision_with_extra_arguments::C() (CXXConstructExpr, [B1.3], class copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 2: [B1.1] (ImplicitCastExpr, NoOp, const class copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 3: [B1.2]
+// CXX11-NEXT: 4:
+// CXX11-NEXT: 5: [B1.3] (CXXConstructExpr, [B1.6], class copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 6: copy_elision_with_extra_arguments::C c = copy_elision_with_extra_arguments::C();
+// CXX17-NEXT: 1: copy_elision_with_extra_arguments::C() (CXXConstructExpr, [B1.2], class copy_elision_with_extra_arguments::C)
+// CXX17-NEXT: 2: copy_elision_with_extra_arguments::C c = copy_elision_with_extra_arguments::C();
+void testCopyElisionWhenCopyConstructorHasExtraArguments() {
+  C c = C();
+}
+} // namespace copy_elision_with_extra_arguments
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49215: [analyzer] Admit that some copy/move constructors have more than one argument.

2018-07-11 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ updated this revision to Diff 155105.
NoQ added a comment.

Actually verify the CFG in the test.


https://reviews.llvm.org/D49215

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
@@ -937,3 +937,26 @@
   E e = E(D());
 }
 } // end namespace argument_constructors
+
+namespace copy_elision_with_extra_arguments {
+class C {
+public:
+  C();
+  C(const C , int x = 0);
+};
+
+// CHECK: void testCopyElisionWhenCopyConstructorHasExtraArguments()
+// CHECK:[B1]
+// CXX11-ELIDE-NEXT: 1: copy_elision_with_extra_arguments::C() 
(CXXConstructExpr, [B1.3], [B1.5], class copy_elision_with_extra_arguments::C)
+// CXX11-NOELIDE-NEXT: 1: copy_elision_with_extra_arguments::C() 
(CXXConstructExpr, [B1.3], class copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 2: [B1.1] (ImplicitCastExpr, NoOp, const class 
copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 3: [B1.2]
+// CXX11-NEXT: 4:
+// CXX11-NEXT: 5: [B1.3] (CXXConstructExpr, [B1.6], class 
copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 6: copy_elision_with_extra_arguments::C c = 
copy_elision_with_extra_arguments::C();
+// CXX17-NEXT: 1: copy_elision_with_extra_arguments::C() 
(CXXConstructExpr, [B1.2], class copy_elision_with_extra_arguments::C)
+// CXX17-NEXT: 2: copy_elision_with_extra_arguments::C c = 
copy_elision_with_extra_arguments::C();
+void testCopyElisionWhenCopyConstructorHasExtraArguments() {
+  C c = C();
+}
+} // namespace copy_elision_with_extra_arguments
Index: lib/Analysis/CFG.cpp
===
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -1298,7 +1298,6 @@
 // Support pre-C++17 copy elision AST.
 auto *CE = cast(Child);
 if (BuildOpts.MarkElidedCXXConstructors && CE->isElidable()) {
-  assert(CE->getNumArgs() == 1);
   findConstructionContexts(withExtraLayer(CE), CE->getArg(0));
 }
 


Index: test/Analysis/cfg-rich-constructors.cpp
===
--- test/Analysis/cfg-rich-constructors.cpp
+++ test/Analysis/cfg-rich-constructors.cpp
@@ -937,3 +937,26 @@
   E e = E(D());
 }
 } // end namespace argument_constructors
+
+namespace copy_elision_with_extra_arguments {
+class C {
+public:
+  C();
+  C(const C , int x = 0);
+};
+
+// CHECK: void testCopyElisionWhenCopyConstructorHasExtraArguments()
+// CHECK:[B1]
+// CXX11-ELIDE-NEXT: 1: copy_elision_with_extra_arguments::C() (CXXConstructExpr, [B1.3], [B1.5], class copy_elision_with_extra_arguments::C)
+// CXX11-NOELIDE-NEXT: 1: copy_elision_with_extra_arguments::C() (CXXConstructExpr, [B1.3], class copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 2: [B1.1] (ImplicitCastExpr, NoOp, const class copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 3: [B1.2]
+// CXX11-NEXT: 4:
+// CXX11-NEXT: 5: [B1.3] (CXXConstructExpr, [B1.6], class copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 6: copy_elision_with_extra_arguments::C c = copy_elision_with_extra_arguments::C();
+// CXX17-NEXT: 1: copy_elision_with_extra_arguments::C() (CXXConstructExpr, [B1.2], class copy_elision_with_extra_arguments::C)
+// CXX17-NEXT: 2: copy_elision_with_extra_arguments::C c = copy_elision_with_extra_arguments::C();
+void testCopyElisionWhenCopyConstructorHasExtraArguments() {
+  C c = C();
+}
+} // namespace copy_elision_with_extra_arguments
Index: lib/Analysis/CFG.cpp
===
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -1298,7 +1298,6 @@
 // Support pre-C++17 copy elision AST.
 auto *CE = cast(Child);
 if (BuildOpts.MarkElidedCXXConstructors && CE->isElidable()) {
-  assert(CE->getNumArgs() == 1);
   findConstructionContexts(withExtraLayer(CE), CE->getArg(0));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49215: [analyzer] Admit that some copy/move constructors have more than one argument.

2018-07-11 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, 
rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware.

Copy/move constructors may have additional default arguments and still be 
treated as normal copy/move constructors. Copy elision code expected this to 
not be a thing, for no particular reason. Remove the premature assertion.

Hopefully addresses the crash posted by @alexfh in 
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20180702/233796.html


Repository:
  rC Clang

https://reviews.llvm.org/D49215

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
@@ -937,3 +937,15 @@
   E e = E(D());
 }
 } // end namespace argument_constructors
+
+namespace copy_elision_with_extra_arguments {
+class C {
+public:
+  C();
+  C(const C , int x = 0);
+};
+
+void testCopyElisionWhenCopyConstructorHasExtraArguments() {
+  C c = C();
+}
+} // namespace copy_elision_with_extra_arguments
Index: lib/Analysis/CFG.cpp
===
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -1298,7 +1298,6 @@
 // Support pre-C++17 copy elision AST.
 auto *CE = cast(Child);
 if (BuildOpts.MarkElidedCXXConstructors && CE->isElidable()) {
-  assert(CE->getNumArgs() == 1);
   findConstructionContexts(withExtraLayer(CE), CE->getArg(0));
 }
 


Index: test/Analysis/cfg-rich-constructors.cpp
===
--- test/Analysis/cfg-rich-constructors.cpp
+++ test/Analysis/cfg-rich-constructors.cpp
@@ -937,3 +937,15 @@
   E e = E(D());
 }
 } // end namespace argument_constructors
+
+namespace copy_elision_with_extra_arguments {
+class C {
+public:
+  C();
+  C(const C , int x = 0);
+};
+
+void testCopyElisionWhenCopyConstructorHasExtraArguments() {
+  C c = C();
+}
+} // namespace copy_elision_with_extra_arguments
Index: lib/Analysis/CFG.cpp
===
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -1298,7 +1298,6 @@
 // Support pre-C++17 copy elision AST.
 auto *CE = cast(Child);
 if (BuildOpts.MarkElidedCXXConstructors && CE->isElidable()) {
-  assert(CE->getNumArgs() == 1);
   findConstructionContexts(withExtraLayer(CE), CE->getArg(0));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits