[PATCH] D63149: Added AST matcher for ignoring elidable constructors

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6477
+/// matches ``D1 = B(B(1))``
+AST_MATCHER_P(Expr, ignoringElidableMoveConstructorCall,
+              ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
----------------
Is the matcher only strict to **move** constructor? looks like the matcher implementation is for general constructors, just `ignoringElidableConstructorCall`? we may have elidable copy constructor.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6479
+              ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
+  if (const auto *cxx_construct_expr = dyn_cast<CXXConstructExpr>(&Node)) {
+    if (cxx_construct_expr->isElidable()) {
----------------
nit: [LLVM](https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly) uses `CamelCase` style. 


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6486
+      }
+      return InnerMatcher.matches(*cxx_construct_expr, Finder, Builder);
+    }
----------------
I think this return is unnecessary, since we will fall through at line 6489.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63149