[PATCH] D42812: [clang-tidy] ObjC ARC objects should not trigger performance-unnecessary-value-param

2018-02-02 Thread Ben Hamilton via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL324097: [clang-tidy] ObjC ARC objects should not trigger 
performance-unnecessary-value… (authored by benhamilton, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D42812

Files:
  clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp
  
clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.m
  
clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.mm


Index: 
clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.m
===
--- 
clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.m
+++ 
clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.m
@@ -0,0 +1,16 @@
+// RUN: clang-tidy %s -checks=-*,performance-unnecessary-value-param -- \
+// RUN:   -xobjective-c -fobjc-abi-version=2 -fobjc-arc | count 0
+
+#if !__has_feature(objc_arc)
+#error Objective-C ARC not enabled as expected
+#endif
+
+// Passing an Objective-C ARC-managed object to a C function should
+// not raise performance-unnecessary-value-param.
+void foo(id object) { }
+
+// Same for explcitly non-ARC-managed Objective-C objects.
+void bar(__unsafe_unretained id object) { }
+
+// Same for Objective-c classes.
+void baz(Class c) { }
Index: 
clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.mm
===
--- 
clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.mm
+++ 
clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.mm
@@ -0,0 +1,16 @@
+// RUN: clang-tidy %s -checks=-*,performance-unnecessary-value-param -- \
+// RUN:   -xobjective-c++ -fobjc-abi-version=2 -fobjc-arc | count 0
+
+#if !__has_feature(objc_arc)
+#error Objective-C ARC not enabled as expected
+#endif
+
+// Passing an Objective-C ARC-managed object to a C function should
+// not raise performance-unnecessary-value-param.
+void foo(id object) { }
+
+// Same for explcitly non-ARC-managed Objective-C objects.
+void bar(__unsafe_unretained id object) { }
+
+// Same for Objective-c classes.
+void baz(Class c) { }
Index: 
clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===
--- 
clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ 
clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -79,6 +79,10 @@
   Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
 
 void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
+  // This check is specific to C++ and doesn't apply to languages like
+  // Objective-C.
+  if (!getLangOpts().CPlusPlus)
+return;
   const auto ExpensiveValueParamDecl =
   parmVarDecl(hasType(hasCanonicalType(allOf(
   unless(referenceType()), 
matchers::isExpensiveToCopy(,
Index: clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp
@@ -45,7 +45,8 @@
 return llvm::None;
   return !Type.isTriviallyCopyableType(Context) &&
  !classHasTrivialCopyAndDestroy(Type) &&
- !hasDeletedCopyConstructor(Type);
+ !hasDeletedCopyConstructor(Type) &&
+ !Type->isObjCLifetimeType();
 }
 
 bool recordIsTriviallyDefaultConstructible(const RecordDecl &RecordDecl,


Index: clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.m
===
--- clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.m
+++ clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.m
@@ -0,0 +1,16 @@
+// RUN: clang-tidy %s -checks=-*,performance-unnecessary-value-param -- \
+// RUN:   -xobjective-c -fobjc-abi-version=2 -fobjc-arc | count 0
+
+#if !__has_feature(objc_arc)
+#error Objective-C ARC not enabled as expected
+#endif
+
+// Passing an Objective-C ARC-managed object to a C function should
+// not raise performance-unnecessary-value-param.
+void foo(id object) { }
+
+// Same for explcitly non-ARC-managed Objective-C objects.
+void bar(__unsafe_unretained id object) { }
+
+// Same for Objective-c classes.
+void baz(Class c) { }
Index: clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.mm
===
--- clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.mm
+++ clang-tools-extra/trunk/test/clang-t

[PATCH] D42812: [clang-tidy] ObjC ARC objects should not trigger performance-unnecessary-value-param

2018-02-02 Thread Haojian Wu via Phabricator via cfe-commits 
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42812



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

[PATCH] D42812: [clang-tidy] ObjC ARC objects should not trigger performance-unnecessary-value-param

2018-02-01 Thread Ben Hamilton via Phabricator via cfe-commits 
benhamilton created this revision.
benhamilton added reviewers: alexfh, hokein.
Herald added subscribers: cfe-commits, xazax.hun, klimek.

The following Objective-C code currently incorrectly triggers
clang-tidy's performance-unnecessary-value-param check:

  % cat /tmp/performance-unnecessary-value-param-arc.m
  void foo(id object) { }
  
  clang-tidy /tmp/performance-unnecessary-value-param-arc.m
  -checks=-\*,performance-unnecessary-value-param -- -xobjective-c
  -fobjc-abi-version=2 -fobjc-arc
  1 warning generated.
  
/src/llvm/tools/clang/tools/extra/test/clang-tidy/performance-unnecessary-value-param-arc.m:10:13:
  warning: the parameter 'object' is copied for each invocation but only
  used as a const reference; consider making it a const reference
  [performance-unnecessary-value-param]
  void foo(id object) { }
   ~~ ^
   const &

This is wrong for a few reasons:

1. Objective-C doesn't have references, so `const &` is not going to help
2. ARC heavily optimizes the "expensive" copy which triggers the warning

This fixes the issue by disabling the warning for non-C++, as well as
disabling it for objects under ARC memory management for
Objective-C++.

Test Plan: New tests added. Ran tests with `make -j12 check-clang-tools`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42812

Files:
  clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tidy/utils/TypeTraits.cpp
  test/clang-tidy/performance-unnecessary-value-param-arc.m
  test/clang-tidy/performance-unnecessary-value-param-arc.mm


Index: test/clang-tidy/performance-unnecessary-value-param-arc.mm
===
--- /dev/null
+++ test/clang-tidy/performance-unnecessary-value-param-arc.mm
@@ -0,0 +1,16 @@
+// RUN: clang-tidy %s -checks=-*,performance-unnecessary-value-param -- \
+// RUN:   -xobjective-c++ -fobjc-abi-version=2 -fobjc-arc | count 0
+
+#if !__has_feature(objc_arc)
+#error Objective-C ARC not enabled as expected
+#endif
+
+// Passing an Objective-C ARC-managed object to a C function should
+// not raise performance-unnecessary-value-param.
+void foo(id object) { }
+
+// Same for explcitly non-ARC-managed Objective-C objects.
+void bar(__unsafe_unretained id object) { }
+
+// Same for Objective-c classes.
+void baz(Class c) { }
Index: test/clang-tidy/performance-unnecessary-value-param-arc.m
===
--- /dev/null
+++ test/clang-tidy/performance-unnecessary-value-param-arc.m
@@ -0,0 +1,16 @@
+// RUN: clang-tidy %s -checks=-*,performance-unnecessary-value-param -- \
+// RUN:   -xobjective-c -fobjc-abi-version=2 -fobjc-arc | count 0
+
+#if !__has_feature(objc_arc)
+#error Objective-C ARC not enabled as expected
+#endif
+
+// Passing an Objective-C ARC-managed object to a C function should
+// not raise performance-unnecessary-value-param.
+void foo(id object) { }
+
+// Same for explcitly non-ARC-managed Objective-C objects.
+void bar(__unsafe_unretained id object) { }
+
+// Same for Objective-c classes.
+void baz(Class c) { }
Index: clang-tidy/utils/TypeTraits.cpp
===
--- clang-tidy/utils/TypeTraits.cpp
+++ clang-tidy/utils/TypeTraits.cpp
@@ -45,7 +45,8 @@
 return llvm::None;
   return !Type.isTriviallyCopyableType(Context) &&
  !classHasTrivialCopyAndDestroy(Type) &&
- !hasDeletedCopyConstructor(Type);
+ !hasDeletedCopyConstructor(Type) &&
+ !Type->isObjCLifetimeType();
 }
 
 bool recordIsTriviallyDefaultConstructible(const RecordDecl &RecordDecl,
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -79,6 +79,10 @@
   Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
 
 void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
+  // This check is specific to C++ and doesn't apply to languages like
+  // Objective-C.
+  if (!getLangOpts().CPlusPlus)
+return;
   const auto ExpensiveValueParamDecl =
   parmVarDecl(hasType(hasCanonicalType(allOf(
   unless(referenceType()), 
matchers::isExpensiveToCopy(,


Index: test/clang-tidy/performance-unnecessary-value-param-arc.mm
===
--- /dev/null
+++ test/clang-tidy/performance-unnecessary-value-param-arc.mm
@@ -0,0 +1,16 @@
+// RUN: clang-tidy %s -checks=-*,performance-unnecessary-value-param -- \
+// RUN:   -xobjective-c++ -fobjc-abi-version=2 -fobjc-arc | count 0
+
+#if !__has_feature(objc_arc)
+#error Objective-C ARC not enabled as expected
+#endif
+
+// Passing an Objective-C ARC-managed object to a C function should
+// not raise performance-unnecessary-value-param.
+void foo(id object) { }
+
+// Same for explcitly non-ARC-managed