[clang-tools-extra] r324097 - [clang-tidy] ObjC ARC objects should not trigger performance-unnecessary-value-param

[Ben Hamilton via cfe-commits]

Author: benhamilton
Date: Fri Feb  2 07:34:33 2018
New Revision: 324097

URL: http://llvm.org/viewvc/llvm-project?rev=324097&view=rev
Log:
[clang-tidy] ObjC ARC objects should not trigger performance-unnecessary-value-param

Summary:
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++.

Fixes https://bugs.llvm.org/show_bug.cgi?id=32075

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

Reviewers: alexfh, hokein

Reviewed By: hokein

Subscribers: stephanemoore, klimek, xazax.hun, cfe-commits, Wizard

Differential Revision: https://reviews.llvm.org/D42812

Added:
    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
Modified:
    clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
    clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp

Modified: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp?rev=324097&r1=324096&r2=324097&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp Fri Feb  2 07:34:33 2018
@@ -79,6 +79,10 @@ UnnecessaryValueParamCheck::UnnecessaryV
           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()))),

Modified: clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp?rev=324097&r1=324096&r2=324097&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp Fri Feb  2 07:34:33 2018
@@ -45,7 +45,8 @@ llvm::Optional<bool> isExpensiveToCopy(Q
     return llvm::None;
   return !Type.isTriviallyCopyableType(Context) &&
          !classHasTrivialCopyAndDestroy(Type) &&
-         !hasDeletedCopyConstructor(Type);
+         !hasDeletedCopyConstructor(Type) &&
+         !Type->isObjCLifetimeType();
 }
 
 bool recordIsTriviallyDefaultConstructible(const RecordDecl &RecordDecl,

Added: clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.m
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.m?rev=324097&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.m (added)
+++ clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.m Fri Feb  2 07:34:33 2018
@@ -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) { }

Added: clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.mm
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.mm?rev=324097&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.mm (added)
+++ clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-arc.mm Fri Feb  2 07:34:33 2018
@@ -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) { }