alexfh added a comment.
Thanks, looks better now, but still a few comments, mostly nits.
================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:701
+ const ASTContext::DynTypedNodeList &Parents = Context->getParents(*DeclRef);
+ if (!Parents.empty()) {
+ const Stmt *ST = Parents[0].get<Stmt>();
----------------
Please use early return:
if (Parents.empty())
return false;
================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:702
+ if (!Parents.empty()) {
+ const Stmt *ST = Parents[0].get<Stmt>();
+ // Ignore ImplicitCastExpr if we find one.
----------------
Same here:
if (!ST)
return false;
================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:710
+ }
+ if (ST && isa<LambdaExpr>(ST))
+ return true;
----------------
And here just `return ST && isa<LambdaExpr>(ST);`
================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:810
SourceRange Range = DeclRef->getNameInfo().getSourceRange();
- addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
- Result.SourceManager);
- return;
+ bool LambdaDeclRef = isLambda(DeclRef, Result.Context);
+ if (LambdaDeclRef) {
----------------
Let's drop this variable. The code is going to be simpler without it:
if (isLambda(DeclRef, Result.Context)) {
StringRef CaptureText = ...;
if (CaptureText != "=" && CaptureText != "&") {
addUsage(...);
return;
}
}
================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:812
+ if (LambdaDeclRef) {
+ std::string captureText =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(Range),
----------------
Lexer::getSourceText returns a StringRef, there's no need to copy its contents.
================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:812
+ if (LambdaDeclRef) {
+ std::string captureText =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(Range),
----------------
alexfh wrote:
> Lexer::getSourceText returns a StringRef, there's no need to copy its
> contents.
nit: CaptureText
================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:815
+ *Result.SourceManager, getLangOpts());
+ if (!(captureText == "=" || captureText == "&"))
+ LambdaDeclRef = false;
----------------
I find it easier to understand, if negation is propagated through the
parentheses:
if (captureText != "=" && captureText != "&")
================
Comment at: test/clang-tidy/readability-identifier-naming.cpp:506
+bool Foo() {
+ bool Columns=false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local
variable 'Columns'
----------------
alexfh wrote:
> If the formatting is not critical for the logic of the test, please
> clang-format the new test code.
nit: The formatting is still off here, e.g. missing spaces around the equals
sign.
================
Comment at: test/clang-tidy/readability-identifier-naming.cpp:508
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local
variable 'Columns'
+// CHECK-FIXES: {{^}} bool columns=false;
+ auto ptr = [&]{
----------------
Let's make the CHECK-FIXES lines unique to reduce the chance of mixing them up
and to make identifying broken test cases easier. One way to do this is to make
the variable names distinct, another way is to add unique trailing comments
both in the code and in the CHECK-FIXES patterns.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59540/new/
https://reviews.llvm.org/D59540
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
