cjdb added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/performance/ConstParameterValueOrRef.h:40
+ static constexpr int DefaultSmallMaxSize = 16;
+ int SmallMaxSize = Options.get("SmallMaxSize", DefaultSmallMaxSize);
+
----------------
jmarrec wrote:
> 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
Could you please elaborate on why you find this interesting? I don't see any
benefit to ignoring `struct S { char c; }`.
================
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) {}
+
----------------
jmarrec wrote:
> 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...
I don't know if our goals are aligned here. This patch intends to preserve
constness in all cases. If a user defines a function as
```
void f(int const& x) { }
```
Then there's a good chance that they want the constness. Removing that
constness changes the meaning of the program since `x` would then be
modifiable. What's deferred to `readability-avoid-const-params-in-decls` is
const-qualified value parameters in forward declarations, which isn't really in
scope here, since forward declarations are used to overrule this check's
analysis.
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
