hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
LGTM.
================
Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:108
return;
+ if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(Function)) {
+ for (const auto *Init : Ctor->inits()) {
----------------
shuaiwang wrote:
> hokein wrote:
> > Is this a new fix or a special case not being caught by
> > `ExprMutationAnalyzer`? Do we have a test case covered it?
> It's a special case.
> ExprMutationAnalyzer is designed to analyze within a "scope" indicated by a
> Stmt. So when trying to analyze a "function", merely looking at function body
> is not sufficient because CXXCtorInitializer is not part of the function
> body. So far we only care about this special case when trying to figure out
> whether a function param is mutated or not, which is exactly what this check
> is doing. We might consider making this part of ExprMutationAnalyzer but IMO
> that depends on whether there are more use cases of this special case in the
> future because we'll need some tricks there to match different types of top
> level nodes.
>
> This is already captured in existing unit test case, which basically looks
> like this:
> ```
> class Foo {
> public:
> Foo(ExpensiveObj x) : x(std::move(x)) {}
> private:
> ExpensiveObj x;
> };
> ```
Thanks for the detailed clarification! That makes sense. Could you also add a
proper comment in the code?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50102
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
