[PATCH] D28022: [clang-tidy] Handle constructors in performance-unnecessary-value-param

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 21 09:11:21 PST 2016


aaron.ballman added inline comments.


================
Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:96
 
-  // Do not trigger on non-const value parameters when:
-  // 1. they are in a constructor definition since they can likely trigger
-  //    modernize-pass-by-value which will suggest to move the argument.
-  if (!IsConstQualified && (llvm::isa<CXXConstructorDecl>(Function) ||
-                            !Function->doesThisDeclarationHaveABody()))
-    return;
+  const FunctionDecl &Definition = *Function->getDefinition();
 
----------------
malcolm.parsons wrote:
> aaron.ballman wrote:
> > malcolm.parsons wrote:
> > > malcolm.parsons wrote:
> > > > aaron.ballman wrote:
> > > > > Instead of using `hasBody()` and `getDefinition()`, you should use `FunctionDecl::getBody()` and pass in an argument to receive the function's definition.
> > > > I don't want the body - the `CXXCtorInitializer`s are not in it.
> > > I should use `hasBody()` and pass in an argument.
> > You are calling `Function-hasBody()` followed by `Function->getDefinition()`, which is what `FunctionDecl::getBody()` does. The difference is that `FunctionDecl::getBody()` will also load serialized bodies from the AST (if the body happens to be in a PCH, for instance).
> Do I want to load serialized bodies?
I would imagine you would want to deserialize, since you care about the contents of the body, not just the presence of one. This may or may not be important with modules (Richard Smith would be able to give a more definitive answer there).


https://reviews.llvm.org/D28022





More information about the cfe-commits mailing list