alexfh added a comment.
Awesome! This makes the check far more usable.
See a few minor comments in-line.
================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:140
@@ +139,3 @@
+ Finder->addMatcher(
+ namedDecl(unless(isExpansionInSystemHeader())).bind("decl"), this);
+ Finder->addMatcher(
----------------
ClangTidy takes care of filtering errors. It doesn't show errors in system
headers by default, but there's an option to show them, if needed (e.g. for
library developers). Is there any reason to add filtering here as well?
================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:537
@@ +536,3 @@
+
+ return addUsage(NamingCheckFailures, Decl->getParent(),
+ Decl->getNameInfo().getSourceRange(),
Result.SourceManager);
----------------
Please split `return` and `addUsage` here and in other places. It looks rather
confusing to me and adds no benefits.
================
Comment at: test/clang-tidy/readability-identifier-naming.cpp:69
@@ -72,1 +68,3 @@
+#include <iostream>
+// Expect NO warnings or errors from system headers, it shouldn't even be
checked
----------------
Please don't #include system headers to tests. There are multiple reasons to
avoid this:
* that won't work in some test configurations;
* we don't control the contents of system headers, so we can't make tests
reliable;
* testing time can increase significantly due to the possibly large size of
system headers.
If you need to test includes, put mock headers to
test/clang-tidy/Inputs/<check-name>/ and specify this directory in the
clang-tidy invocation:
// RUN: %python %S/check_clang_tidy.py %s readability-identifier-naming %t \
// RUN: -config=... \
// RUN: -- -std=... -isystem=%S/Inputs/readability-identifier-naming
http://reviews.llvm.org/D13081
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
