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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to