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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D52360#1246474, @baloghadamsoftware wrote:

> In https://reviews.llvm.org/D52360#1246452, @lebedev.ri wrote:
>
> > I disagree, it **must** not have false-negatives by default.
>
>
> What kind of false-negatives could be caused by smart pointers or references? Sane people do not name a type pointer or reference if they are not. Such types must never be passed by reference. Few people will use the check if they have to set tons of options for the basic expected behavior. For example in `CodeChecker` this check is disabled by default and is only enabled in the `Sensitive` profile.


Ok, so let's entertain the current patch.
Now, some time passes, and someone either discovers that the check does not warn on some code that one would have thought it should warn.
Shitty code happens. Not everyone is sane in their naming choices. More importantly, there isn't a common coding style naming guidelines everyone **has to** follow.
Someone somewhere will surely hit this. When he does, how does one get the check to warn on that?
Or other side of the coin, how does one get the check //not// to warn on some custom type that does not match those hardcoded regexes?

I don't disagree that this has false-positives. But this is **really** sensitive to the type names. It is not a good idea to hardcode this.
//Please// follow pre-existing practice of //configurable// type white/blacklists.

  $ clang-tidy -checks=\* -dump-config | grep -i "types$" -A1
    - key:             cert-msc32-c.DisallowedSeedTypes
      value:           'time_t,std::time_t'
    - key:             cert-msc51-cpp.DisallowedSeedTypes
      value:           'time_t,std::time_t'
  --
    - key:             google-runtime-references.WhiteListTypes
      value:           ''
  --
    - key:             hicpp-use-emplace.TupleTypes
      value:           '::std::pair;::std::tuple'
  --
    - key:             modernize-use-emplace.TupleTypes
      value:           '::std::pair;::std::tuple'
  --
    - key:             readability-simplify-subscript-expr.Types
      value:           '::std::basic_string;::std::basic_string_view;::std::vector;::std::array'


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52360