[PATCH] D63261: [clang-tidy] Fixed abseil-time-subtraction to work on C++17

2019-06-13 Thread Dmitri Gribenko via Phabricator via cfe-commits 
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363272: [clang-tidy] Fixed abseil-time-subtraction to work 
on C++17 (authored by gribozavr, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63261?vs=204538&id=204544#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63261

Files:
  clang-tools-extra/trunk/clang-tidy/abseil/TimeSubtractionCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/abseil-time-subtraction.cpp


Index: clang-tools-extra/trunk/clang-tidy/abseil/TimeSubtractionCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/abseil/TimeSubtractionCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/abseil/TimeSubtractionCheck.cpp
@@ -29,33 +29,52 @@
 
 static bool isConstructorAssignment(const MatchFinder::MatchResult &Result,
 const Expr *Node) {
+  // For C++14 and earlier there are elidable constructors that must be matched
+  // in hasParent. The elidable constructors do not exist in C++17 and later 
and
+  // therefore an additional check that does not match against the elidable
+  // constructors are needed for this case.
   return selectFirst(
- "e", match(expr(hasParent(materializeTemporaryExpr(hasParent(
- cxxConstructExpr(hasParent(exprWithCleanups(
- hasParent(varDecl()
-.bind("e"),
-*Node, *Result.Context)) != nullptr;
+ "e",
+ match(expr(anyOf(
+   callExpr(hasParent(materializeTemporaryExpr(hasParent(
+
cxxConstructExpr(hasParent(exprWithCleanups(
+hasParent(varDecl()
+   .bind("e"),
+   callExpr(hasParent(varDecl())).bind("e"))),
+   *Node, *Result.Context)) != nullptr;
 }
 
 static bool isArgument(const MatchFinder::MatchResult &Result,
const Expr *Node) {
+  // For the same reason as in isConstructorAssignment two AST shapes need to 
be
+  // matched here.
   return selectFirst(
  "e",
- match(expr(hasParent(
-
materializeTemporaryExpr(hasParent(cxxConstructExpr(
-hasParent(callExpr()),
-unless(hasParent(cxxOperatorCallExpr(
-   .bind("e"),
-   *Node, *Result.Context)) != nullptr;
+ match(
+ expr(anyOf(
+ expr(hasParent(materializeTemporaryExpr(
+  hasParent(cxxConstructExpr(
+  hasParent(callExpr()),
+  unless(hasParent(cxxOperatorCallExpr(
+ .bind("e"),
+ expr(hasParent(callExpr()),
+  unless(hasParent(cxxOperatorCallExpr(
+ .bind("e"))),
+ *Node, *Result.Context)) != nullptr;
 }
 
 static bool isReturn(const MatchFinder::MatchResult &Result, const Expr *Node) 
{
+  // For the same reason as in isConstructorAssignment two AST shapes need to 
be
+  // matched here.
   return selectFirst(
- "e", match(expr(hasParent(materializeTemporaryExpr(hasParent(
- cxxConstructExpr(hasParent(exprWithCleanups(
- hasParent(returnStmt()
-.bind("e"),
-*Node, *Result.Context)) != nullptr;
+ "e",
+ match(expr(anyOf(
+   expr(hasParent(materializeTemporaryExpr(hasParent(
+cxxConstructExpr(hasParent(exprWithCleanups(
+hasParent(returnStmt()
+   .bind("e"),
+   expr(hasParent(returnStmt())).bind("e"))),
+   *Node, *Result.Context)) != nullptr;
 }
 
 static bool parensRequired(const MatchFinder::MatchResult &Result,
Index: clang-tools-extra/trunk/test/clang-tidy/abseil-time-subtraction.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/abseil-time-subtraction.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/abseil-time-subtraction.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s abseil-time-subtraction %t -- -- 
-I %S/Inputs
+// RUN: %check_clang_tidy -std=c++11-or-later %s abseil-time-subtraction %t -- 
-- -I %S/Inputs
 // FIXME: Fix the checker to work in C++17 mode.
 
 #i

[PATCH] D63261: [clang-tidy] Fixed abseil-time-subtraction to work on C++17

2019-06-13 Thread Johan Vikström via Phabricator via cfe-commits 
jvikstrom updated this revision to Diff 204538.
jvikstrom marked an inline comment as done.
jvikstrom added a comment.

Using anyOf instead of multiple selectFirsts


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63261

Files:
  clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
  clang-tools-extra/test/clang-tidy/abseil-time-subtraction.cpp


Index: clang-tools-extra/test/clang-tidy/abseil-time-subtraction.cpp
===
--- clang-tools-extra/test/clang-tidy/abseil-time-subtraction.cpp
+++ clang-tools-extra/test/clang-tidy/abseil-time-subtraction.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s abseil-time-subtraction %t -- -- 
-I %S/Inputs
+// RUN: %check_clang_tidy -std=c++11-or-later %s abseil-time-subtraction %t -- 
-- -I %S/Inputs
 // FIXME: Fix the checker to work in C++17 mode.
 
 #include "absl/time/time.h"
Index: clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
===
--- clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
+++ clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
@@ -29,33 +29,52 @@
 
 static bool isConstructorAssignment(const MatchFinder::MatchResult &Result,
 const Expr *Node) {
+  // For C++14 and earlier there are elidable constructors that must be matched
+  // in hasParent. The elidable constructors do not exist in C++17 and later 
and
+  // therefore an additional check that does not match against the elidable
+  // constructors are needed for this case.
   return selectFirst(
- "e", match(expr(hasParent(materializeTemporaryExpr(hasParent(
- cxxConstructExpr(hasParent(exprWithCleanups(
- hasParent(varDecl()
-.bind("e"),
-*Node, *Result.Context)) != nullptr;
+ "e",
+ match(expr(anyOf(
+   callExpr(hasParent(materializeTemporaryExpr(hasParent(
+
cxxConstructExpr(hasParent(exprWithCleanups(
+hasParent(varDecl()
+   .bind("e"),
+   callExpr(hasParent(varDecl())).bind("e"))),
+   *Node, *Result.Context)) != nullptr;
 }
 
 static bool isArgument(const MatchFinder::MatchResult &Result,
const Expr *Node) {
+  // For the same reason as in isConstructorAssignment two separate 
selectFirsts
+  // need to be checked here
   return selectFirst(
  "e",
- match(expr(hasParent(
-
materializeTemporaryExpr(hasParent(cxxConstructExpr(
-hasParent(callExpr()),
-unless(hasParent(cxxOperatorCallExpr(
-   .bind("e"),
-   *Node, *Result.Context)) != nullptr;
+ match(
+ expr(anyOf(
+ expr(hasParent(materializeTemporaryExpr(
+  hasParent(cxxConstructExpr(
+  hasParent(callExpr()),
+  unless(hasParent(cxxOperatorCallExpr(
+ .bind("e"),
+ expr(hasParent(callExpr()),
+  unless(hasParent(cxxOperatorCallExpr(
+ .bind("e"))),
+ *Node, *Result.Context)) != nullptr;
 }
 
 static bool isReturn(const MatchFinder::MatchResult &Result, const Expr *Node) 
{
+  // For the same reason as in isConstructorAssignment two separate 
selectFirsts
+  // need to be checked here
   return selectFirst(
- "e", match(expr(hasParent(materializeTemporaryExpr(hasParent(
- cxxConstructExpr(hasParent(exprWithCleanups(
- hasParent(returnStmt()
-.bind("e"),
-*Node, *Result.Context)) != nullptr;
+ "e",
+ match(expr(anyOf(
+   expr(hasParent(materializeTemporaryExpr(hasParent(
+cxxConstructExpr(hasParent(exprWithCleanups(
+hasParent(returnStmt()
+   .bind("e"),
+   expr(hasParent(returnStmt())).bind("e"))),
+   *Node, *Result.Context)) != nullptr;
 }
 
 static bool parensRequired(const MatchFinder::MatchResult &Result,


Index: clang-tools-extra/test/clang-tidy/abseil-time-subtraction.cpp
===
--- clang-tools-extra/test/clang-tidy/abseil-time-subtraction.cpp
+++ clang-tools-extra/test/clang-tidy/abseil-time-subtr

[PATCH] D63261: [clang-tidy] Fixed abseil-time-subtraction to work on C++17

2019-06-13 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr added inline comments.



Comment at: clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp:45
+ 
match(callExpr(hasParent(varDecl())).bind("e"),
+   *Node, *Result.Context)) != nullptr;
 }

Use anyOf() instead two selectFirsts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63261



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

[PATCH] D63261: [clang-tidy] Fixed abseil-time-subtraction to work on C++17

2019-06-13 Thread Johan Vikström via Phabricator via cfe-commits 
jvikstrom created this revision.
jvikstrom added reviewers: hokein, gribozavr.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

Fixed abseil-time-subtraction to work on C++17


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63261

Files:
  clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
  clang-tools-extra/test/clang-tidy/abseil-time-subtraction.cpp


Index: clang-tools-extra/test/clang-tidy/abseil-time-subtraction.cpp
===
--- clang-tools-extra/test/clang-tidy/abseil-time-subtraction.cpp
+++ clang-tools-extra/test/clang-tidy/abseil-time-subtraction.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s abseil-time-subtraction %t -- -- 
-I %S/Inputs
+// RUN: %check_clang_tidy -std=c++11-or-later %s abseil-time-subtraction %t -- 
-- -I %S/Inputs
 // FIXME: Fix the checker to work in C++17 mode.
 
 #include "absl/time/time.h"
Index: clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
===
--- clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
+++ clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
@@ -29,16 +29,26 @@
 
 static bool isConstructorAssignment(const MatchFinder::MatchResult &Result,
 const Expr *Node) {
+  // For C++14 and earlier there are elidable constructors that must be matched
+  // in hasParent. The elidable constructors do not exist in C++17 and later 
and
+  // therefore an additional check that does not match against the elidable
+  // constructors are needed for this case.
   return selectFirst(
- "e", match(expr(hasParent(materializeTemporaryExpr(hasParent(
- cxxConstructExpr(hasParent(exprWithCleanups(
- hasParent(varDecl()
-.bind("e"),
-*Node, *Result.Context)) != nullptr;
+ "e",
+ match(callExpr(hasParent(materializeTemporaryExpr(
+hasParent(cxxConstructExpr(hasParent(
+
exprWithCleanups(hasParent(varDecl()
+   .bind("e"),
+   *Node, *Result.Context)) != nullptr ||
+ selectFirst("e",
+ 
match(callExpr(hasParent(varDecl())).bind("e"),
+   *Node, *Result.Context)) != nullptr;
 }
 
 static bool isArgument(const MatchFinder::MatchResult &Result,
const Expr *Node) {
+  // For the same reason as in isConstructorAssignment two separate 
selectFirsts
+  // need to be checked here
   return selectFirst(
  "e",
  match(expr(hasParent(
@@ -46,16 +56,26 @@
 hasParent(callExpr()),
 unless(hasParent(cxxOperatorCallExpr(
.bind("e"),
-   *Node, *Result.Context)) != nullptr;
+   *Node, *Result.Context)) != nullptr ||
+ selectFirst(
+ "e", match(expr(hasParent(callExpr()),
+ unless(hasParent(cxxOperatorCallExpr(
+.bind("e"),
+*Node, *Result.Context)) != nullptr;
 }
 
 static bool isReturn(const MatchFinder::MatchResult &Result, const Expr *Node) 
{
+  // For the same reason as in isConstructorAssignment two separate 
selectFirsts
+  // need to be checked here
   return selectFirst(
  "e", match(expr(hasParent(materializeTemporaryExpr(hasParent(
  cxxConstructExpr(hasParent(exprWithCleanups(
  hasParent(returnStmt()
 .bind("e"),
-*Node, *Result.Context)) != nullptr;
+*Node, *Result.Context)) != nullptr ||
+ selectFirst("e",
+ match(expr(hasParent(returnStmt())).bind("e"),
+   *Node, *Result.Context)) != nullptr;
 }
 
 static bool parensRequired(const MatchFinder::MatchResult &Result,


Index: clang-tools-extra/test/clang-tidy/abseil-time-subtraction.cpp
===
--- clang-tools-extra/test/clang-tidy/abseil-time-subtraction.cpp
+++ clang-tools-extra/test/clang-tidy/abseil-time-subtraction.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s abseil-time-subtraction %t -- -- -I %S/Inputs
+// RUN: %check_clang_tidy -std=c++11-or-later %s abseil-time-subtraction %t -- -- -I %S/Inputs
 // FIXME: Fix the checker to work in C++17 mode.
 
 #include "absl/time/time.h"
Index: clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
===
--- clang-tools-extra