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) { } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits