[PATCH] D97577: [clang-tidy] performance-for-range-copy: Don't trigger on implicit type conversions.

[Felix Berger via Phabricator via cfe-commits]

flx created this revision.
flx added reviewers: aaron.ballman, alexfh, sbenza.
Herald added a subscriber: xazax.hun.
flx requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This disables the check for false positive cases where implicit type conversion
through either an implicit single argument constructor or a member conversion
operator is triggered when constructing the loop variable.

Fix the test cases that meant to cover these cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97577

Files:
  clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
@@ -60,13 +60,13 @@
 
 void negativeUserDefinedConversion() {
   Convertible C[0];
-  for (const S &S1 : C) {
+  for (const S S1 : C) {
   }
 }
 
 void negativeImplicitConstructorConversion() {
   ConstructorConvertible C[0];
-  for (const S &S1 : C) {
+  for (const S S1 : C) {
   }
 }
 
Index: clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -12,6 +12,7 @@
 #include "../utils/Matchers.h"
 #include "../utils/OptionsUtils.h"
 #include "../utils/TypeTraits.h"
+#include 
"third_party/llvm/llvm-project/clang/include/clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 #include "clang/Basic/Diagnostic.h"
 
@@ -45,10 +46,14 @@
       hasOverloadedOperatorName("*"),
       callee(
           cxxMethodDecl(returns(unless(hasCanonicalType(referenceType()))))));
+  auto NotConstructedByCopy = cxxConstructExpr(
+      hasDeclaration(cxxConstructorDecl(unless(isCopyConstructor()))));
+  auto ConstructedByConversion = 
cxxMemberCallExpr(callee(cxxConversionDecl()));
   auto LoopVar =
       varDecl(HasReferenceOrPointerTypeOrIsAllowed,
-              unless(hasInitializer(expr(hasDescendant(expr(anyOf(
-                  materializeTemporaryExpr(), IteratorReturnsValueType)))))));
+              unless(hasInitializer(expr(hasDescendant(expr(
+                  anyOf(materializeTemporaryExpr(), IteratorReturnsValueType,
+                        NotConstructedByCopy, ConstructedByConversion)))))));
   Finder->addMatcher(
       traverse(TK_AsIs,
                cxxForRangeStmt(hasLoopVariable(LoopVar.bind("loopVar")))



Index: clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
@@ -60,13 +60,13 @@
 
 void negativeUserDefinedConversion() {
   Convertible C[0];
-  for (const S &S1 : C) {
+  for (const S S1 : C) {
   }
 }
 
 void negativeImplicitConstructorConversion() {
   ConstructorConvertible C[0];
-  for (const S &S1 : C) {
+  for (const S S1 : C) {
   }
 }
 
Index: clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -12,6 +12,7 @@
 #include "../utils/Matchers.h"
 #include "../utils/OptionsUtils.h"
 #include "../utils/TypeTraits.h"
+#include "third_party/llvm/llvm-project/clang/include/clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 #include "clang/Basic/Diagnostic.h"
 
@@ -45,10 +46,14 @@
       hasOverloadedOperatorName("*"),
       callee(
           cxxMethodDecl(returns(unless(hasCanonicalType(referenceType()))))));
+  auto NotConstructedByCopy = cxxConstructExpr(
+      hasDeclaration(cxxConstructorDecl(unless(isCopyConstructor()))));
+  auto ConstructedByConversion = cxxMemberCallExpr(callee(cxxConversionDecl()));
   auto LoopVar =
       varDecl(HasReferenceOrPointerTypeOrIsAllowed,
-              unless(hasInitializer(expr(hasDescendant(expr(anyOf(
-                  materializeTemporaryExpr(), IteratorReturnsValueType)))))));
+              unless(hasInitializer(expr(hasDescendant(expr(
+                  anyOf(materializeTemporaryExpr(), IteratorReturnsValueType,
+                        NotConstructedByCopy, ConstructedByConversion)))))));
   Finder->addMatcher(
       traverse(TK_AsIs,
                cxxForRangeStmt(hasLoopVariable(LoopVar.bind("loopVar")))

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