[PATCH] D107873: [clang-tidy] Add 'performance-const-parameter-value-or-ref' for checking const-qualified parameters

[Julien Marrec via Phabricator via cfe-commits]

jmarrec added a comment.

Imported some discussion from https://reviews.llvm.org/D107900#inline-1029358. Sorry it took that long



================
Comment at: clang-tools-extra/clang-tidy/performance/ConstParameterValueOrRef.h:40
+  static constexpr int DefaultSmallMaxSize = 16;
+  int SmallMaxSize = Options.get("SmallMaxSize", DefaultSmallMaxSize);
+
----------------
https://reviews.llvm.org/D107900#inline-1029358


On mine, I had another option `RestrictToBuiltInTypes` which I find an interesting option. It has a ` builtinType()` matcher in the registerMatchers function


================
Comment at: clang-tools-extra/clang-tidy/performance/ConstParameterValueOrRef.h:43
+  bool ForwardDeclarationsSuppressWarnings =
+      Options.get("ForwardDeclarationsSuppressWarnings", true);
+
----------------
Perhaps two additional options to turn off either "side" would be helpful? eg `CheckPassByValueIsAppropriate` and `CheckPassByRefIsAppropriate`, both defaulted to `true`


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/performance-const-parameter-value-or-ref.rst:44
+  // warning: 'const int' is a small, trivially copyable type: pass by value instead
+  // fix-it: void f6(const int i) {}
+
----------------
https://reviews.llvm.org/D107900#inline-1029358

I am removing the `const` as well in addition to the `&`. I guess you may be just relying on using it in conjunction with `readability-avoid-const-params-in-decls` but I think that makes it harder to use (you can't just do `-checks=-*,<yourcheck>` you have to remember to add the `readability-avoid-const-params-in-decls` too...).

Taht being said, perhaps that's how it's meant to be...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107873/new/

https://reviews.llvm.org/D107873