mehdi_amini added inline comments.
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst:43
+ a human reader, and there's basically no place for a bug to hide. On the
other
+ hand for non-public functions, all the call-sites are visible and the
parameter
+ can be eliminated entirely.
----------------
aaron.ballman wrote:
> Call sites are not always visible for protected functions, so this seems a
> bit suspicious. The changes are missing test coverage for that situation.
You're using `public` for "access control" while I was using the linkage
aspect: my reasoning is that if a method isn't "externally visible" from the
current translation unit, you see all the call-sites. This is orthogonal to
public/private/protected as far as I know.
I am likely missing a check for "isDefinedInMainFile" (or whatever the api is
called) to not flag included headers.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp:157-159
+// CHECK-FIXES: C() {}
+ C(int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning
----------------
aaron.ballman wrote:
> I think this demonstrates a bad fix -- this changes code meaning from being a
> converting constructor to being a default constructor, which are very
> different things.
Oh you're right: so we can't do it for a Ctor with a single parameter...
But we also can't do it for a Ctor with two parameters as it'll turn it into a
converting ctor. Unless you can eliminate both parameters, in which case it is
become a default ctor (which can conflict with an existing one, in which case
it can be just deleted?)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116512/new/
https://reviews.llvm.org/D116512
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
