[PATCH] D52315: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

"The three checks mentioned in the Title are two noisy if the code uses intrusive smart (reference counting) pointers. LLVM/Clang is such a code, it has lots of such types, e.g. StringRef, SymbolRef or ProgramStateRef, all of them based llvm::IntrusiveRefCntPtr. Every time such a type is passed as parameter, used for initialization or in a range-based for loop a false positive warning is issued together with an (unnecessary) fix."

I'm not quite following this comment - StringRef doesn't appear to have any connection to IntrusiveRefCntPtr so far as I can see? (I don't see anything in SymbolRef either. ProgramStateRef is an IntrusiveRefCntPtr, for sure - and has a non-trivial copy ctor  - yeah, if you pass that by value unnecessarily, I think that's a real/reasonable warning?

The reason these things are passed by value a lot is when passing ownership - if the callee is moving the IntrusiveRefCntPtr into something that exceeds the duration of the function call (eg: moving it into a member function from a setter) - so the warning should take that into account. But it shouldn't rely on the sizeof of the type - that's not really a good signal here, I think.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52315