aaron.ballman added a comment.
In D88314#2322870 <https://reviews.llvm.org/D88314#2322870>, @bogser01 wrote:
> @aaron.ballman Thank you for picking up this review! Running the check over
> the entire LLVM causes ~74K warnings across 430 files. As to the false
> positive rate it's tricky to measure. Based on previous analysis on //flang//
> codebase, I would say roughly 50% of the fixes would cause compiler errors
> when applied.
Woof, that's a pretty high false-positive rate for the check -- I don't think
we should issue a fix-it for this case because anyone who accidentally applies
fixits automatically will be in for a fair amount of pain.
I'm a bit worried that the number of diagnostics this produces will basically
mean the check has to be turned off for the only project that would use it.
What is the expected use case for the check? For instance, are you expecting to
craft a .clang-tidy file to disable this check on source files in the repo that
are known to have a lot of diagnostics (so that the check mostly only fires for
known-good files and new files)?
================
Comment at: clang-tools-extra/clang-tidy/add_new_check.py:139
const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("x");
+<<<<<<< HEAD
+ if ((!MatchedDecl->getIdentifier()) ||
MatchedDecl->getName().startswith("awesome_"))
----------------
Looks like there are some accidental merge markers in the patch.
================
Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:204
void AwesomeFunctionNamesCheck::check(const MatchFinder::MatchResult
&Result) {
+<<<<<<< HEAD
+ const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("x");
----------------
More merge conflict markers.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/llvm-string-referencing.cpp:25
+void f1(const string &P);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter of type 'const
std::string &' could potentially be replaced with 'llvm::StringRef'
[llvm-string-referencing]
+// CHECK-FIXES: void f1(llvm::StringRef P);{{$}}
----------------
You can drop the name of the check from the `CHECK-MESSAGES` line (we usually
only check the full diagnostic once).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88314/new/
https://reviews.llvm.org/D88314
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
