From a62e2232815e537952bd6df271763521a0472498 Mon Sep 17 00:00:00 2001 From: Benjamin Kramer Date: Thu, 7 Apr 2016 14:55:25 +0000 Subject: [PATCH] [clang-tidy] Remove unnecessary getName() on Decls and Types feeding into a DiagnosticBuilder Going through a string removes some of the smarts of the diagnosic printer and makes the code more complicated. This change has some cosmetic impact on the output but that's mostly minor. llvm-svn: 265680 --- .../clang-tidy/google/NonConstReferences.cpp | 4 ++-- .../clang-tidy/misc/DefinitionsInHeadersCheck.cpp | 11 +++++---- .../misc/ForwardDeclarationNamespaceCheck.cpp | 18 +++++++-------- .../misc/MacroRepeatedSideEffectsCheck.cpp | 4 ++-- .../clang-tidy/misc/NonCopyableObjects.cpp | 10 ++++----- .../clang-tidy/misc/UnusedAliasDeclsCheck.cpp | 5 ++--- .../clang-tidy/misc/UnusedParametersCheck.cpp | 3 +-- .../performance/FasterStringFindCheck.cpp | 9 ++++---- .../performance/ImplicitCastInLoopCheck.cpp | 10 +++------ .../clang-tidy/readability/FunctionSizeCheck.cpp | 4 ++-- .../clang-tidy/misc-definitions-in-headers.hpp | 4 ++-- .../clang-tidy/performance-faster-string-find.cpp | 26 +++++++++++----------- 12 files changed, 50 insertions(+), 58 deletions(-) diff --git a/clang-tools-extra/clang-tidy/google/NonConstReferences.cpp b/clang-tools-extra/clang-tidy/google/NonConstReferences.cpp index cafc908..5667bd1 100644 --- a/clang-tools-extra/clang-tidy/google/NonConstReferences.cpp +++ b/clang-tools-extra/clang-tidy/google/NonConstReferences.cpp @@ -115,8 +115,8 @@ void NonConstReferences::check(const MatchFinder::MatchResult &Result) { << Parameter->getFunctionScopeIndex(); } else { diag(Parameter->getLocation(), - "non-const reference parameter '%0', make it const or use a pointer") - << Parameter->getName(); + "non-const reference parameter %0, make it const or use a pointer") + << Parameter; } } diff --git a/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp b/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp index b5ac6ce..1ee9757 100644 --- a/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp @@ -118,11 +118,10 @@ void DefinitionsInHeadersCheck::check(const MatchFinder::MatchResult &Result) { } diag(FD->getLocation(), - "function '%0' defined in a header file; " + "function %0 defined in a header file; " "function definitions in header files can lead to ODR violations") - << FD->getNameInfo().getName().getAsString() - << FixItHint::CreateInsertion(FD->getSourceRange().getBegin(), - "inline "); + << FD << FixItHint::CreateInsertion(FD->getSourceRange().getBegin(), + "inline "); } else if (const auto *VD = dyn_cast(ND)) { // Static data members of a class template are allowed. if (VD->getDeclContext()->isDependentContext() && VD->isStaticDataMember()) @@ -134,9 +133,9 @@ void DefinitionsInHeadersCheck::check(const MatchFinder::MatchResult &Result) { return; diag(VD->getLocation(), - "variable '%0' defined in a header file; " + "variable %0 defined in a header file; " "variable definitions in header files can lead to ODR violations") - << VD->getName(); + << VD; } } diff --git a/clang-tools-extra/clang-tidy/misc/ForwardDeclarationNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/misc/ForwardDeclarationNamespaceCheck.cpp index db5d6f0..98053ff 100644 --- a/clang-tools-extra/clang-tidy/misc/ForwardDeclarationNamespaceCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/ForwardDeclarationNamespaceCheck.cpp @@ -139,12 +139,12 @@ void ForwardDeclarationNamespaceCheck::onEndOfTranslationUnit() { if (!CurDecl->hasDefinition() && !haveSameNamespaceOrTranslationUnit(CurDecl, Decl)) { diag(CurDecl->getLocation(), - "declaration '%0' is never referenced, but a declaration with " + "declaration %0 is never referenced, but a declaration with " "the same name found in another namespace '%1'") - << CurDecl->getName() << getNameOfNamespace(Decl); - diag(Decl->getLocation(), "a declaration of '%0' is found here", + << CurDecl << getNameOfNamespace(Decl); + diag(Decl->getLocation(), "a declaration of %0 is found here", DiagnosticIDs::Note) - << Decl->getName(); + << Decl; break; // FIXME: We only generate one warning for each declaration. } } @@ -158,12 +158,12 @@ void ForwardDeclarationNamespaceCheck::onEndOfTranslationUnit() { const auto &Definitions = DeclNameToDefinitions[DeclName]; for (const auto *Def : Definitions) { diag(CurDecl->getLocation(), - "no definition found for '%0', but a definition with " - "the same name '%1' found in another namespace '%2'") - << CurDecl->getName() << Def->getName() << getNameOfNamespace(Def); - diag(Def->getLocation(), "a definition of '%0' is found here", + "no definition found for %0, but a definition with " + "the same name %1 found in another namespace '%2'") + << CurDecl << Def << getNameOfNamespace(Def); + diag(Def->getLocation(), "a definition of %0 is found here", DiagnosticIDs::Note) - << Def->getName(); + << Def; } } } diff --git a/clang-tools-extra/clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp b/clang-tools-extra/clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp index 24593cb..e8ad7c5 100644 --- a/clang-tools-extra/clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/MacroRepeatedSideEffectsCheck.cpp @@ -65,9 +65,9 @@ void MacroRepeatedPPCallbacks::MacroExpands(const Token &MacroNameTok, if (hasSideEffects(ResultArgToks) && countArgumentExpansions(MI, Arg) >= 2) { Check.diag(ResultArgToks->getLocation(), - "side effects in the %ordinal0 macro argument '%1' are " + "side effects in the %ordinal0 macro argument %1 are " "repeated in macro expansion") - << (ArgNo + 1) << Arg->getName(); + << (ArgNo + 1) << Arg; Check.diag(MI->getDefinitionLoc(), "macro %0 defined here", DiagnosticIDs::Note) << MacroNameTok.getIdentifierInfo(); diff --git a/clang-tools-extra/clang-tidy/misc/NonCopyableObjects.cpp b/clang-tools-extra/clang-tidy/misc/NonCopyableObjects.cpp index 64c4508..929bc0c 100644 --- a/clang-tools-extra/clang-tidy/misc/NonCopyableObjects.cpp +++ b/clang-tools-extra/clang-tidy/misc/NonCopyableObjects.cpp @@ -81,14 +81,14 @@ void NonCopyableObjectsCheck::check(const MatchFinder::MatchResult &Result) { const auto *E = Result.Nodes.getNodeAs("expr"); if (D && BD) - diag(D->getLocation(), "'%0' declared as type '%1', which is unsafe to copy" - "; did you mean '%1 *'?") - << D->getName() << BD->getName(); + diag(D->getLocation(), "%0 declared as type '%1', which is unsafe to copy" + "; did you mean '%1 *'?") + << D << BD->getName(); else if (E) diag(E->getExprLoc(), - "expression has opaque data structure type '%0'; type should only be " + "expression has opaque data structure type %0; type should only be " "used as a pointer and not dereferenced") - << BD->getName(); + << BD; } } // namespace tidy diff --git a/clang-tools-extra/clang-tidy/misc/UnusedAliasDeclsCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnusedAliasDeclsCheck.cpp index 3af988a..d9a1296 100644 --- a/clang-tools-extra/clang-tidy/misc/UnusedAliasDeclsCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UnusedAliasDeclsCheck.cpp @@ -53,9 +53,8 @@ void UnusedAliasDeclsCheck::onEndOfTranslationUnit() { for (const auto &FoundDecl : FoundDecls) { if (!FoundDecl.second.isValid()) continue; - diag(FoundDecl.first->getLocation(), "namespace alias decl '%0' is unused") - << FoundDecl.first->getName() - << FixItHint::CreateRemoval(FoundDecl.second); + diag(FoundDecl.first->getLocation(), "namespace alias decl %0 is unused") + << FoundDecl.first << FixItHint::CreateRemoval(FoundDecl.second); } } diff --git a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp index 3927a78..b2504d4 100644 --- a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp @@ -67,8 +67,7 @@ void UnusedParametersCheck::warnOnUnusedParameter( const MatchFinder::MatchResult &Result, const FunctionDecl *Function, unsigned ParamIndex) { const auto *Param = Function->getParamDecl(ParamIndex); - auto MyDiag = diag(Param->getLocation(), "parameter '%0' is unused") - << Param->getName(); + auto MyDiag = diag(Param->getLocation(), "parameter %0 is unused") << Param; auto DeclRefExpr = declRefExpr(to(equalsNode(Function)), diff --git a/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp b/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp index 942b3db..ea43d35 100644 --- a/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp @@ -116,11 +116,10 @@ void FasterStringFindCheck::check(const MatchFinder::MatchResult &Result) { diag(Literal->getLocStart(), "%0 called with a string literal consisting of " "a single character; consider using the more " "effective overload accepting a character") - << FindFunc->getName() - << FixItHint::CreateReplacement( - CharSourceRange::getTokenRange(Literal->getLocStart(), - Literal->getLocEnd()), - *Replacement); + << FindFunc << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(Literal->getLocStart(), + Literal->getLocEnd()), + *Replacement); } } // namespace performance diff --git a/clang-tools-extra/clang-tidy/performance/ImplicitCastInLoopCheck.cpp b/clang-tools-extra/clang-tidy/performance/ImplicitCastInLoopCheck.cpp index 5f3d2b2..04dadfb 100644 --- a/clang-tools-extra/clang-tidy/performance/ImplicitCastInLoopCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/ImplicitCastInLoopCheck.cpp @@ -85,16 +85,12 @@ void ImplicitCastInLoopCheck::ReportAndFix( QualType ConstType = OperatorCall->getType().withConst(); QualType ConstRefType = Context->getLValueReferenceType(ConstType); const char Message[] = - "the type of the loop variable '%0' is different from the one returned " + "the type of the loop variable %0 is different from the one returned " "by the iterator and generates an implicit cast; you can either " - "change the type to the correct one ('%1' but 'const auto&' is always a " + "change the type to the correct one (%1 but 'const auto&' is always a " "valid option) or remove the reference to make it explicit that you are " "creating a new value"; - PrintingPolicy Policy(Context->getLangOpts()); - Policy.SuppressTagKeyword = true; - - diag(VD->getLocStart(), Message) << VD->getName() - << ConstRefType.getAsString(Policy); + diag(VD->getLocStart(), Message) << VD << ConstRefType; } } // namespace performance diff --git a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp index 42ce65b..dc411fc 100644 --- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp @@ -74,8 +74,8 @@ void FunctionSizeCheck::onEndOfTranslationUnit() { if (FI.Lines > LineThreshold || FI.Statements > StatementThreshold || FI.Branches > BranchThreshold) { diag(P.first->getLocation(), - "function '%0' exceeds recommended size/complexity thresholds") - << P.first->getNameAsString(); + "function %0 exceeds recommended size/complexity thresholds") + << P.first; } if (FI.Lines > LineThreshold) { diff --git a/clang-tools-extra/test/clang-tidy/misc-definitions-in-headers.hpp b/clang-tools-extra/test/clang-tidy/misc-definitions-in-headers.hpp index fb35749..ab68c33 100644 --- a/clang-tools-extra/test/clang-tidy/misc-definitions-in-headers.hpp +++ b/clang-tools-extra/test/clang-tidy/misc-definitions-in-headers.hpp @@ -28,7 +28,7 @@ void CA::f2() { } template <> int CA::f3() { -// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: function 'f3' defined in a header file; +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: function 'f3' defined in a header file; int a = 1; return a; } @@ -90,7 +90,7 @@ T f3() { } template <> -// CHECK-MESSAGES: :[[@LINE+1]]:5: warning: function 'f3' defined in a header file; +// CHECK-MESSAGES: :[[@LINE+1]]:5: warning: function 'f3' defined in a header file; int f3() { int a = 1; return a; diff --git a/clang-tools-extra/test/clang-tidy/performance-faster-string-find.cpp b/clang-tools-extra/test/clang-tidy/performance-faster-string-find.cpp index 9510616..b36e3230 100644 --- a/clang-tools-extra/test/clang-tidy/performance-faster-string-find.cpp +++ b/clang-tools-extra/test/clang-tidy/performance-faster-string-find.cpp @@ -33,12 +33,12 @@ void StringFind() { std::string Str; Str.find("a"); - // CHECK-MESSAGES: [[@LINE-1]]:12: warning: find called with a string literal consisting of a single character; consider using the more effective overload accepting a character [performance-faster-string-find] + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'find' called with a string literal consisting of a single character; consider using the more effective overload accepting a character [performance-faster-string-find] // CHECK-FIXES: Str.find('a'); // Works with the pos argument. Str.find("a", 1); - // CHECK-MESSAGES: [[@LINE-1]]:12: warning: find called with a string literal + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'find' called with a string literal // CHECK-FIXES: Str.find('a', 1); // Doens't work with strings smaller or larger than 1 char. @@ -50,35 +50,35 @@ void StringFind() { // Other methods that can also be replaced Str.rfind("a"); - // CHECK-MESSAGES: [[@LINE-1]]:13: warning: rfind called with a string literal + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: 'rfind' called with a string literal // CHECK-FIXES: Str.rfind('a'); Str.find_first_of("a"); - // CHECK-MESSAGES: [[@LINE-1]]:21: warning: find_first_of called with a string + // CHECK-MESSAGES: [[@LINE-1]]:21: warning: 'find_first_of' called with a string // CHECK-FIXES: Str.find_first_of('a'); Str.find_first_not_of("a"); - // CHECK-MESSAGES: [[@LINE-1]]:25: warning: find_first_not_of called with a + // CHECK-MESSAGES: [[@LINE-1]]:25: warning: 'find_first_not_of' called with a // CHECK-FIXES: Str.find_first_not_of('a'); Str.find_last_of("a"); - // CHECK-MESSAGES: [[@LINE-1]]:20: warning: find_last_of called with a string + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'find_last_of' called with a string // CHECK-FIXES: Str.find_last_of('a'); Str.find_last_not_of("a"); - // CHECK-MESSAGES: [[@LINE-1]]:24: warning: find_last_not_of called with a + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: 'find_last_not_of' called with a // CHECK-FIXES: Str.find_last_not_of('a'); // std::wstring should work. std::wstring WStr; WStr.find(L"n"); - // CHECK-MESSAGES: [[@LINE-1]]:13: warning: find called with a string literal + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: 'find' called with a string literal // CHECK-FIXES: Str.find(L'n'); // Even with unicode that fits in one wide char. WStr.find(L"\x3A9"); - // CHECK-MESSAGES: [[@LINE-1]]:13: warning: find called with a string literal + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: 'find' called with a string literal // CHECK-FIXES: Str.find(L'\x3A9'); // Also with other types, but only if it was specified in the options. llvm::StringRef sr; sr.find("x"); - // CHECK-MESSAGES: [[@LINE-1]]:11: warning: find called with a string literal + // CHECK-MESSAGES: [[@LINE-1]]:11: warning: 'find' called with a string literal // CHECK-FIXES: sr.find('x'); NotStringRef nsr; nsr.find("x"); @@ -92,7 +92,7 @@ int FindTemplateDependant(T value) { template int FindTemplateNotDependant(T pos) { return std::string().find("A", pos); - // CHECK-MESSAGES: [[@LINE-1]]:29: warning: find called with a string literal + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: 'find' called with a string literal // CHECK-FIXES: return std::string().find('A', pos); } @@ -105,6 +105,6 @@ int FindStr() { int Macros() { return STR_MACRO(std::string()) + POS_MACRO(1); - // CHECK-MESSAGES: [[@LINE-1]]:10: warning: find called with a string literal - // CHECK-MESSAGES: [[@LINE-2]]:37: warning: find called with a string literal + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'find' called with a string literal + // CHECK-MESSAGES: [[@LINE-2]]:37: warning: 'find' called with a string literal } -- 2.7.4