[PATCH] D73562: [ASTMatchers] Add isPlacement traversal matcher for CXXNewExpr

2020-01-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6763
+///   matches the expression 'new (Storage) MyClass()'.
+AST_MATCHER(CXXNewExpr, isPlacement) { return Node.getNumPlacementArgs() > 0; }
+

I think a better design would be something like `hasPlacementExpr()` as a 
traversal matcher. Then `isPlacement()` can be trivially done in the project as 
`hasPlacementExpr(anything())` (we could still consider adding it as a 
dedicated matcher, but it doesn't seem critical to me), but this would also 
allow people to check for specific placement new argument expressions.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73562/new/

https://reviews.llvm.org/D73562



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D73562: [ASTMatchers] Add isPlacement traversal matcher for CXXNewExpr

2020-01-28 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62272 tests passed, 1 failed 
and 827 were skipped.

  failed: Clang.CodeGenOpenCL/amdgpu-features.cl

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73562/new/

https://reviews.llvm.org/D73562



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D73562: [ASTMatchers] Add isPlacement traversal matcher for CXXNewExpr

2020-01-28 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision.
Herald added subscribers: cfe-commits, dexonsmith, steven_wu, hiraditya, 
mehdi_amini.
Herald added a project: clang.
njames93 added a reviewer: aaron.ballman.

Adds a traversal matcher called `isPlacement` that matches on `placement new` 
operators.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73562

Files:
  clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -3124,5 +3124,37 @@
   EXPECT_TRUE(notMatches("template class A {};", Matcher));
 }
 
+TEST(CXXNewExpr, Array) {
+  StatementMatcher NewArray = cxxNewExpr(isArray());
+
+  EXPECT_TRUE(matches("void foo() { int *Ptr = new int[10]; }", NewArray));
+  EXPECT_TRUE(notMatches("void foo() { int *Ptr = new int; }", NewArray));
+
+  StatementMatcher NewArraySize10 =
+  cxxNewExpr(hasArraySize(integerLiteral(equals(10;
+  EXPECT_TRUE(
+  matches("void foo() { int *Ptr = new int[10]; }", NewArraySize10));
+  EXPECT_TRUE(
+  notMatches("void foo() { int *Ptr = new int[20]; }", NewArraySize10));
+}
+
+TEST(CXXNewExpr, IsPlacement) {
+  StatementMatcher PlacementNew = cxxNewExpr(isPlacement());
+
+  EXPECT_TRUE(matches(R"(
+void* operator new(decltype(sizeof(void*)), void*); 
+int *foo(void* Storage) {
+  return new (Storage) int; 
+})",
+  PlacementNew));
+
+  EXPECT_TRUE(notMatches(R"(
+void* operator new(decltype(sizeof(void*)), void*); 
+int *foo(void* Storage) {
+  return new int; 
+})",
+ PlacementNew));
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -394,6 +394,7 @@
   REGISTER_MATCHER(isNoneKind);
   REGISTER_MATCHER(isOMPStructuredBlock);
   REGISTER_MATCHER(isOverride);
+  REGISTER_MATCHER(isPlacement)
   REGISTER_MATCHER(isPrivate);
   REGISTER_MATCHER(isProtected);
   REGISTER_MATCHER(isPublic);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6752,6 +6752,16 @@
   return Node.isArray();
 }
 
+/// Matches placement new expressions.
+///
+/// Given:
+/// \code
+///   MyClass *p1 = new (Storage) MyClass();
+/// \endcode
+/// cxxNewExpr(isPlacement())
+///   matches the expression 'new (Storage) MyClass()'.
+AST_MATCHER(CXXNewExpr, isPlacement) { return Node.getNumPlacementArgs() > 0; }
+
 /// Matches array new expressions with a given array size.
 ///
 /// Given:
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -2547,6 +2547,16 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1CXXNewExpr.html;>CXXNewExprisPlacement
+Matches placement new expressions.
+
+Given:
+  MyClass *p1 = new (Storage) MyClass();
+cxxNewExpr(isPlacement())
+  matches the expression 'new (Storage) MyClass()'.
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1CXXOperatorCallExpr.html;>CXXOperatorCallExprhasOverloadedOperatorNameStringRef Name
 Matches overloaded operator names.
 
Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -91,7 +91,7 @@
   hasArgument(0,
   cxxNewExpr(hasType(pointsTo(qualType(hasCanonicalType(
  equalsBoundNode(PointerType),
- CanCallCtor)
+ CanCallCtor, unless(isPlacement()))
   .bind(NewExpression)),
   unless(isInTemplateInstantiation()))
   .bind(ConstructorCall,
@@ -101,7 +101,8 @@
   cxxMemberCallExpr(
   thisPointerType(getSmartPointerTypeMatcher()),
   callee(cxxMethodDecl(hasName("reset"))),
-  hasArgument(0, cxxNewExpr(CanCallCtor).bind(NewExpression)),
+  hasArgument(0, cxxNewExpr(CanCallCtor, unless(isPlacement()))
+