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

[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<const Expr>(
-             "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<const Expr>(
              "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<const Expr>(
-             "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.
 
 #include "absl/time/time.h"


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D63261.204544.patch
Type: text/x-patch
Size: 4342 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190613/07aa6189/attachment.bin>