From 1a721b6a2634d9740b389a7604275b426c22600a Mon Sep 17 00:00:00 2001 From: Nathan James Date: Fri, 26 Feb 2021 19:10:25 +0000 Subject: [PATCH] [clang-tidy][NFC] Tweak some generation of diag messages Fix up cases where diag is called by piecing together a string in favour of placeholders. Fix up cases where select could be used instead of duplicating the message for sake of 1 word difference. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D97488 --- .../abseil/DurationFactoryFloatCheck.cpp | 6 ++---- .../abseil/StringFindStartswithCheck.cpp | 20 ++++++----------- .../clang-tidy/bugprone/ExceptionEscapeCheck.cpp | 6 ++---- .../clang-tidy/bugprone/ParentVirtualCallCheck.cpp | 4 ++-- .../cppcoreguidelines/ProTypeMemberInitCheck.cpp | 7 +++--- .../llvm/PreferRegisterOverUnsignedCheck.cpp | 12 +++++------ .../misc/UnconventionalAssignOperatorCheck.cpp | 25 ++++++++++------------ .../clang-tidy/misc/UniqueptrResetReleaseCheck.cpp | 17 +++++++++------ .../clang-tidy/modernize/AvoidCArraysCheck.cpp | 9 +++----- .../clang-tidy/modernize/UseEqualsDefaultCheck.cpp | 19 ++++++++-------- .../clang-tidy/modernize/UseNodiscardCheck.cpp | 4 ++-- .../modernize/UseTransparentFunctorsCheck.cpp | 7 +++--- .../performance/MoveConstructorInitCheck.cpp | 5 +++-- .../performance/NoexceptMoveConstructorCheck.cpp | 16 ++++++++------ .../performance/UnnecessaryCopyInitialization.cpp | 11 ++++------ .../performance/UnnecessaryValueParamCheck.cpp | 11 ++++------ .../clang-tidy/portability/SIMDIntrinsicsCheck.cpp | 17 +++++++-------- .../clang-tidy/readability/QualifiedAutoCheck.cpp | 19 +++++++++------- .../clang-tidy/readability/UseAnyOfAllOfCheck.cpp | 7 +++--- .../checkers/misc-uniqueptr-reset-release.cpp | 12 +++++------ 20 files changed, 110 insertions(+), 124 deletions(-) diff --git a/clang-tools-extra/clang-tidy/abseil/DurationFactoryFloatCheck.cpp b/clang-tools-extra/clang-tidy/abseil/DurationFactoryFloatCheck.cpp index 1f1dc07..4b0ef1b 100644 --- a/clang-tools-extra/clang-tidy/abseil/DurationFactoryFloatCheck.cpp +++ b/clang-tools-extra/clang-tidy/abseil/DurationFactoryFloatCheck.cpp @@ -59,10 +59,8 @@ void DurationFactoryFloatCheck::check(const MatchFinder::MatchResult &Result) { SimpleArg = stripFloatLiteralFraction(Result, *Arg); if (SimpleArg) { - diag(MatchedCall->getBeginLoc(), - (llvm::Twine("use the integer version of absl::") + - MatchedCall->getDirectCallee()->getName()) - .str()) + diag(MatchedCall->getBeginLoc(), "use the integer version of absl::%0") + << MatchedCall->getDirectCallee()->getName() << FixItHint::CreateReplacement(Arg->getSourceRange(), *SimpleArg); } } diff --git a/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp b/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp index 8506313..aa43e7d 100644 --- a/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp +++ b/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp @@ -83,24 +83,18 @@ void StringFindStartswithCheck::check(const MatchFinder::MatchResult &Result) { Context.getLangOpts()); // Create the StartsWith string, negating if comparison was "!=". - bool Neg = ComparisonExpr->getOpcodeStr() == "!="; - StringRef StartswithStr; - if (Neg) { - StartswithStr = "!absl::StartsWith"; - } else { - StartswithStr = "absl::StartsWith"; - } + bool Neg = ComparisonExpr->getOpcode() == BO_NE; // Create the warning message and a FixIt hint replacing the original expr. - auto Diagnostic = - diag(ComparisonExpr->getBeginLoc(), - (StringRef("use ") + StartswithStr + " instead of find() " + - ComparisonExpr->getOpcodeStr() + " 0") - .str()); + auto Diagnostic = diag(ComparisonExpr->getBeginLoc(), + "use %select{absl::StartsWith|!absl::StartsWith}0 " + "instead of find() %select{==|!=}0 0") + << Neg; Diagnostic << FixItHint::CreateReplacement( ComparisonExpr->getSourceRange(), - (StartswithStr + "(" + HaystackExprCode + ", " + NeedleExprCode + ")") + ((Neg ? "!absl::StartsWith(" : "absl::StartsWith(") + HaystackExprCode + + ", " + NeedleExprCode + ")") .str()); // Create a preprocessor #include FixIt hint (CreateIncludeInsertion checks diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp index a78e93a..85af540 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp @@ -72,10 +72,8 @@ void ExceptionEscapeCheck::check(const MatchFinder::MatchResult &Result) { utils::ExceptionAnalyzer::State::Throwing) // FIXME: We should provide more information about the exact location where // the exception is thrown, maybe the full path the exception escapes - diag(MatchedDecl->getLocation(), - "an exception may be thrown in function %0 " - - "which should not throw exceptions") + diag(MatchedDecl->getLocation(), "an exception may be thrown in function " + "%0 which should not throw exceptions") << MatchedDecl; } diff --git a/clang-tools-extra/clang-tidy/bugprone/ParentVirtualCallCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ParentVirtualCallCheck.cpp index 6b8b7fc..8f21678 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ParentVirtualCallCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ParentVirtualCallCheck.cpp @@ -137,9 +137,9 @@ void ParentVirtualCallCheck::check(const MatchFinder::MatchResult &Result) { assert(Member->getQualifierLoc().getSourceRange().getBegin().isValid()); auto Diag = diag(Member->getQualifierLoc().getSourceRange().getBegin(), "qualified name '%0' refers to a member overridden " - "in subclass%1; did you mean %2?") + "in %plural{1:subclass|:subclasses}1; did you mean %2?") << getExprAsString(*Member, *Result.Context) - << (Parents.size() > 1 ? "es" : "") << ParentsStr; + << static_cast(Parents.size()) << ParentsStr; // Propose a fix if there's only one parent class... if (Parents.size() == 1 && diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp index 6b40e52..f446836 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp @@ -439,10 +439,9 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer( DiagnosticBuilder Diag = diag(Ctor ? Ctor->getBeginLoc() : ClassDecl.getLocation(), - IsUnion - ? "union constructor should initialize one of these fields: %0" - : "constructor does not initialize these fields: %0") - << toCommaSeparatedString(OrderedFields, AllFieldsToInit); + "%select{|union }0constructor %select{does not|should}0 initialize " + "%select{|one of }0these fields: %1") + << IsUnion << toCommaSeparatedString(OrderedFields, AllFieldsToInit); // Do not propose fixes for constructors in macros since we cannot place them // correctly. diff --git a/clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp b/clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp index 17e75ab..6e8cabe 100644 --- a/clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp @@ -38,27 +38,27 @@ void PreferRegisterOverUnsignedCheck::check( const auto *VarType = Result.Nodes.getNodeAs("varType"); const auto *UserVarDecl = Result.Nodes.getNodeAs("var"); - StringRef Replacement = "llvm::Register"; + bool NeedsQualification = true; const DeclContext *Context = UserVarDecl->getDeclContext(); while (Context) { if (const auto *Namespace = dyn_cast(Context)) if (isa(Namespace->getDeclContext()) && Namespace->getName() == "llvm") - Replacement = "Register"; + NeedsQualification = false; for (const auto *UsingDirective : Context->using_directives()) { const NamespaceDecl *Namespace = UsingDirective->getNominatedNamespace(); if (isa(Namespace->getDeclContext()) && Namespace->getName() == "llvm") - Replacement = "Register"; + NeedsQualification = false; } Context = Context->getParent(); } diag(UserVarDecl->getLocation(), - "variable %0 declared as %1; use '%2' instead") - << UserVarDecl << *VarType << Replacement + "variable %0 declared as %1; use '%select{|llvm::}2Register' instead") + << UserVarDecl << *VarType << NeedsQualification << FixItHint::CreateReplacement( UserVarDecl->getTypeSourceInfo()->getTypeLoc().getSourceRange(), - Replacement); + NeedsQualification ? "llvm::Register" : "Register"); } } // namespace llvm_check diff --git a/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp index 5fc9732..594c83a 100644 --- a/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp @@ -73,21 +73,18 @@ void UnconventionalAssignOperatorCheck::check( if (const auto *RetStmt = Result.Nodes.getNodeAs("returnStmt")) { diag(RetStmt->getBeginLoc(), "operator=() should always return '*this'"); } else { - static const char *const Messages[][2] = { - {"ReturnType", "operator=() should return '%0&'"}, - {"ArgumentType", - getLangOpts().CPlusPlus11 - ? "operator=() should take '%0 const&', '%0&&' or '%0'" - : "operator=() should take '%0 const&' or '%0'"}, - {"cv", "operator=() should not be marked '%1'"}}; - const auto *Method = Result.Nodes.getNodeAs("method"); - for (const auto &Message : Messages) { - if (Result.Nodes.getNodeAs(Message[0])) - diag(Method->getBeginLoc(), Message[1]) - << Method->getParent()->getName() - << (Method->isConst() ? "const" : "virtual"); - } + if (Result.Nodes.getNodeAs("ReturnType")) + diag(Method->getBeginLoc(), "operator=() should return '%0&'") + << Method->getParent()->getName(); + if (Result.Nodes.getNodeAs("ArgumentType")) + diag(Method->getBeginLoc(), + "operator=() should take '%0 const&'%select{|, '%0&&'}1 or '%0'") + << Method->getParent()->getName() << getLangOpts().CPlusPlus11; + if (Result.Nodes.getNodeAs("cv")) + diag(Method->getBeginLoc(), + "operator=() should not be marked '%select{const|virtual}0'") + << !Method->isConst(); } } diff --git a/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp b/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp index 98b53e3..f1e2cbe 100644 --- a/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp @@ -110,19 +110,22 @@ void UniqueptrResetReleaseCheck::check(const MatchFinder::MatchResult &Result) { LeftText = "*" + LeftText; if (ReleaseMember->isArrow()) RightText = "*" + RightText; - std::string DiagText; + bool IsMove = false; // Even if x was rvalue, *x is not rvalue anymore. if (!Right->isRValue() || ReleaseMember->isArrow()) { RightText = "std::move(" + RightText + ")"; - DiagText = "prefer ptr1 = std::move(ptr2) over ptr1.reset(ptr2.release())"; - } else { - DiagText = - "prefer ptr = ReturnUnique() over ptr.reset(ReturnUnique().release())"; + IsMove = true; } + std::string NewText = LeftText + " = " + RightText; - diag(ResetMember->getExprLoc(), DiagText) << FixItHint::CreateReplacement( - CharSourceRange::getTokenRange(ResetCall->getSourceRange()), NewText); + diag(ResetMember->getExprLoc(), + "prefer ptr = %select{std::move(ptr2)|ReturnUnique()}0 over " + "ptr.reset(%select{ptr2|ReturnUnique()}0.release())") + << !IsMove + << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(ResetCall->getSourceRange()), + NewText); } } // namespace misc diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp b/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp index bc586d3..872dd18 100644 --- a/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp @@ -57,13 +57,10 @@ void AvoidCArraysCheck::registerMatchers(MatchFinder *Finder) { void AvoidCArraysCheck::check(const MatchFinder::MatchResult &Result) { const auto *ArrayType = Result.Nodes.getNodeAs("typeloc"); - static constexpr llvm::StringLiteral UseArray = llvm::StringLiteral( - "do not declare C-style arrays, use std::array<> instead"); - static constexpr llvm::StringLiteral UseVector = llvm::StringLiteral( - "do not declare C VLA arrays, use std::vector<> instead"); - diag(ArrayType->getBeginLoc(), - ArrayType->getTypePtr()->isVariableArrayType() ? UseVector : UseArray); + "do not declare %select{C-style|C VLA}0 arrays, use " + "%select{std::array<>|std::vector<>}0 instead") + << ArrayType->getTypePtr()->isVariableArrayType(); } } // namespace modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp index 5c79e86..0f10dd3 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp @@ -245,8 +245,6 @@ void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) { } void UseEqualsDefaultCheck::check(const MatchFinder::MatchResult &Result) { - std::string SpecialFunctionName; - // Both CXXConstructorDecl and CXXDestructorDecl inherit from CXXMethodDecl. const auto *SpecialFunctionDecl = Result.Nodes.getNodeAs(SpecialFunction); @@ -276,14 +274,14 @@ void UseEqualsDefaultCheck::check(const MatchFinder::MatchResult &Result) { bodyEmpty(Result.Context, Body); std::vector RemoveInitializers; - + unsigned MemberType; if (const auto *Ctor = dyn_cast(SpecialFunctionDecl)) { if (Ctor->getNumParams() == 0) { - SpecialFunctionName = "default constructor"; + MemberType = 0; } else { if (!isCopyConstructorAndCanBeDefaulted(Result.Context, Ctor)) return; - SpecialFunctionName = "copy constructor"; + MemberType = 1; // If there are constructor initializers, they must be removed. for (const auto *Init : Ctor->inits()) { RemoveInitializers.emplace_back( @@ -291,11 +289,11 @@ void UseEqualsDefaultCheck::check(const MatchFinder::MatchResult &Result) { } } } else if (isa(SpecialFunctionDecl)) { - SpecialFunctionName = "destructor"; + MemberType = 2; } else { if (!isCopyAssignmentAndCanBeDefaulted(Result.Context, SpecialFunctionDecl)) return; - SpecialFunctionName = "copy-assignment operator"; + MemberType = 3; } // The location of the body is more useful inside a macro as spelling and @@ -304,8 +302,11 @@ void UseEqualsDefaultCheck::check(const MatchFinder::MatchResult &Result) { if (Location.isMacroID()) Location = Body->getBeginLoc(); - auto Diag = diag(Location, "use '= default' to define a trivial " + - SpecialFunctionName); + auto Diag = diag( + Location, + "use '= default' to define a trivial %select{default constructor|copy " + "constructor|destructor|copy-assignment operator}0"); + Diag << MemberType; if (ApplyFix) { // Skipping comments, check for a semicolon after Body->getSourceRange() diff --git a/clang-tools-extra/clang-tidy/modernize/UseNodiscardCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseNodiscardCheck.cpp index a2e6a7a..f992e9d 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseNodiscardCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseNodiscardCheck.cpp @@ -120,8 +120,8 @@ void UseNodiscardCheck::check(const MatchFinder::MatchResult &Result) { ASTContext &Context = *Result.Context; - auto Diag = diag(RetLoc, "function %0 should be marked " + NoDiscardMacro) - << MatchedDecl; + auto Diag = diag(RetLoc, "function %0 should be marked %1") + << MatchedDecl << NoDiscardMacro; // Check for the existence of the keyword being used as the ``[[nodiscard]]``. if (!doesNoDiscardMacroExist(Context, NoDiscardMacro)) diff --git a/clang-tools-extra/clang-tidy/modernize/UseTransparentFunctorsCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseTransparentFunctorsCheck.cpp index cbd11f0..02b602e 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseTransparentFunctorsCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseTransparentFunctorsCheck.cpp @@ -64,7 +64,7 @@ void UseTransparentFunctorsCheck::registerMatchers(MatchFinder *Finder) { this); } -static const StringRef Message = "prefer transparent functors '%0'"; +static const StringRef Message = "prefer transparent functors '%0<>'"; template static T getInnerTypeLocAs(TypeLoc Loc) { T Result; @@ -81,8 +81,7 @@ void UseTransparentFunctorsCheck::check( Result.Nodes.getNodeAs("FunctorClass"); if (const auto *FuncInst = Result.Nodes.getNodeAs("FuncInst")) { - diag(FuncInst->getBeginLoc(), Message) - << (FuncClass->getName() + "<>").str(); + diag(FuncInst->getBeginLoc(), Message) << FuncClass->getName(); return; } @@ -119,7 +118,7 @@ void UseTransparentFunctorsCheck::check( SourceLocation ReportLoc = FunctorLoc.getLocation(); if (ReportLoc.isInvalid()) return; - diag(ReportLoc, Message) << (FuncClass->getName() + "<>").str() + diag(ReportLoc, Message) << FuncClass->getName() << FixItHint::CreateRemoval( FunctorTypeLoc.getArgLoc(0).getSourceRange()); } diff --git a/clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp b/clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp index 34d8e72..dd6e5a8 100644 --- a/clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp @@ -74,8 +74,9 @@ void MoveConstructorInitCheck::check(const MatchFinder::MatchResult &Result) { // There's a move constructor candidate that the caller probably intended // to call instead. diag(Initializer->getSourceLocation(), - "move constructor initializes %0 by calling a copy constructor") - << (Initializer->isBaseInitializer() ? "base class" : "class member"); + "move constructor initializes %select{class member|base class}0 by " + "calling a copy constructor") + << Initializer->isBaseInitializer(); diag(CopyCtor->getLocation(), "copy constructor being called", DiagnosticIDs::Note); diag(Candidate->getLocation(), "candidate move constructor here", diff --git a/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp b/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp index d23f0ab..f87f214 100644 --- a/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp @@ -29,11 +29,11 @@ void NoexceptMoveConstructorCheck::registerMatchers(MatchFinder *Finder) { void NoexceptMoveConstructorCheck::check( const MatchFinder::MatchResult &Result) { if (const auto *Decl = Result.Nodes.getNodeAs("decl")) { - StringRef MethodType = "assignment operator"; + bool IsConstructor = false; if (const auto *Ctor = dyn_cast(Decl)) { if (!Ctor->isMoveConstructor()) return; - MethodType = "constructor"; + IsConstructor = true; } else if (!Decl->isMoveAssignmentOperator()) { return; } @@ -44,9 +44,10 @@ void NoexceptMoveConstructorCheck::check( return; if (!isNoexceptExceptionSpec(ProtoType->getExceptionSpecType())) { - auto Diag = - diag(Decl->getLocation(), "move %0s should be marked noexcept") - << MethodType; + auto Diag = diag(Decl->getLocation(), + "move %select{assignment operator|constructor}0s should " + "be marked noexcept") + << IsConstructor; // Add FixIt hints. SourceManager &SM = *Result.SourceManager; assert(Decl->getNumParams() > 0); @@ -68,8 +69,9 @@ void NoexceptMoveConstructorCheck::check( E = E->IgnoreImplicit(); if (!isa(E)) { diag(E->getExprLoc(), - "noexcept specifier on the move %0 evaluates to 'false'") - << MethodType; + "noexcept specifier on the move %select{assignment " + "operator|constructor}0 evaluates to 'false'") + << IsConstructor; } } } diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp index f63dbab..0cc9a44 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -206,13 +206,10 @@ void UnnecessaryCopyInitialization::handleCopyFromMethodReturn( auto Diagnostic = diag(Var.getLocation(), - IsConstQualified ? "the const qualified variable %0 is " - "copy-constructed from a const reference; " - "consider making it a const reference" - : "the variable %0 is copy-constructed from a " - "const reference but is only used as const " - "reference; consider making it a const reference") - << &Var; + "the %select{|const qualified }0variable %1 is copy-constructed " + "from a const reference%select{ but is only used as const " + "reference|}0; consider making it a const reference") + << IsConstQualified << &Var; if (IssueFix) recordFixes(Var, Context, Diagnostic); } diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp index e3dae9a..2a9f9b7 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp @@ -137,13 +137,10 @@ void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) { auto Diag = diag(Param->getLocation(), - IsConstQualified ? "the const qualified parameter %0 is " - "copied for each invocation; consider " - "making it a reference" - : "the parameter %0 is copied for each " - "invocation but only used as a const reference; " - "consider making it a const reference") - << paramNameOrIndex(Param->getName(), Index); + "the %select{|const qualified }0parameter %1 is copied for each " + "invocation%select{ but only used as a const reference|}0; consider " + "making it a %select{const |}0reference") + << IsConstQualified << paramNameOrIndex(Param->getName(), Index); // Do not propose fixes when: // 1. the ParmVarDecl is in a macro, since we cannot place them correctly // 2. the function is virtual as it might break overrides diff --git a/clang-tools-extra/clang-tidy/portability/SIMDIntrinsicsCheck.cpp b/clang-tools-extra/clang-tidy/portability/SIMDIntrinsicsCheck.cpp index 65d7c17..8ea00a0 100644 --- a/clang-tools-extra/clang-tidy/portability/SIMDIntrinsicsCheck.cpp +++ b/clang-tools-extra/clang-tidy/portability/SIMDIntrinsicsCheck.cpp @@ -12,6 +12,7 @@ #include "clang/Basic/TargetInfo.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/Triple.h" +#include "llvm/Support/ManagedStatic.h" #include "llvm/Support/Regex.h" using namespace clang::ast_matchers; @@ -130,20 +131,18 @@ void SIMDIntrinsicsCheck::check(const MatchFinder::MatchResult &Result) { // We have found a std::simd replacement. if (!New.empty()) { - std::string Message; // If Suggest is true, give a P0214 alternative, otherwise point it out it // is non-portable. if (Suggest) { - Message = (Twine("'") + Old + "' can be replaced by " + New).str(); - Message = llvm::Regex("\\$std").sub(Std, Message); - Message = - llvm::Regex("\\$simd").sub((Std.str() + "::simd").str(), Message); + static const llvm::Regex StdRegex("\\$std"), SimdRegex("\\$simd"); + diag(Call->getExprLoc(), "'%0' can be replaced by %1") + << Old + << SimdRegex.sub(SmallString<32>({Std, "::simd"}), + StdRegex.sub(Std, New)); } else { - Message = (Twine("'") + Old + "' is a non-portable " + - llvm::Triple::getArchTypeName(Arch) + " intrinsic function") - .str(); + diag("'%0' is a non-portable %1 intrinsic function") + << Old << llvm::Triple::getArchTypeName(Arch); } - diag(Call->getExprLoc(), Message); } } diff --git a/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp b/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp index 9ca4d2e..f2838cd 100644 --- a/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp @@ -209,10 +209,11 @@ void QualifiedAutoCheck::check(const MatchFinder::MatchResult &Result) { }(); DiagnosticBuilder Diag = - diag(FixitLoc, "'%0%1%2auto %3' can be declared as '%4%3'") - << (IsLocalConst ? "const " : "") - << (IsLocalVolatile ? "volatile " : "") - << (IsLocalRestrict ? "__restrict " : "") << Var->getName() << ReplStr; + diag(FixitLoc, + "'%select{|const }0%select{|volatile }1%select{|__restrict }2auto " + "%3' can be declared as '%4%3'") + << IsLocalConst << IsLocalVolatile << IsLocalRestrict << Var->getName() + << ReplStr; for (SourceRange &Range : RemoveQualifiersRange) { Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(Range)); @@ -253,10 +254,12 @@ void QualifiedAutoCheck::check(const MatchFinder::MatchResult &Result) { TypeSpec->getEnd().isMacroID()) return; SourceLocation InsertPos = TypeSpec->getBegin(); - diag(InsertPos, "'auto *%0%1%2' can be declared as 'const auto *%0%1%2'") - << (Var->getType().isLocalConstQualified() ? "const " : "") - << (Var->getType().isLocalVolatileQualified() ? "volatile " : "") - << Var->getName() << FixItHint::CreateInsertion(InsertPos, "const "); + diag(InsertPos, + "'auto *%select{|const }0%select{|volatile }1%2' can be declared as " + "'const auto *%select{|const }0%select{|volatile }1%2'") + << Var->getType().isLocalConstQualified() + << Var->getType().isLocalVolatileQualified() << Var->getName() + << FixItHint::CreateInsertion(InsertPos, "const "); } return; } diff --git a/clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp index 6e83750..495b2fa 100644 --- a/clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp @@ -88,19 +88,20 @@ static bool isViableLoop(const CXXForRangeStmt &S, ASTContext &Context) { } void UseAnyOfAllOfCheck::check(const MatchFinder::MatchResult &Result) { - StringRef Ranges = getLangOpts().CPlusPlus20 ? "::ranges" : ""; if (const auto *S = Result.Nodes.getNodeAs("any_of_loop")) { if (!isViableLoop(*S, *Result.Context)) return; - diag(S->getForLoc(), "replace loop by 'std%0::any_of()'") << Ranges; + diag(S->getForLoc(), "replace loop by 'std%select{|::ranges}0::any_of()'") + << getLangOpts().CPlusPlus20; } else if (const auto *S = Result.Nodes.getNodeAs("all_of_loop")) { if (!isViableLoop(*S, *Result.Context)) return; - diag(S->getForLoc(), "replace loop by 'std%0::all_of()'") << Ranges; + diag(S->getForLoc(), "replace loop by 'std%select{|::ranges}0::all_of()'") + << getLangOpts().CPlusPlus20; } } diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc-uniqueptr-reset-release.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc-uniqueptr-reset-release.cpp index 1bd6e6f..c97b9b3 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc-uniqueptr-reset-release.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc-uniqueptr-reset-release.cpp @@ -33,27 +33,27 @@ void f() { std::unique_ptr *y = &b; a.reset(b.release()); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr1 = std::move(ptr2) over ptr1.reset(ptr2.release()) [misc-uniqueptr-reset-release] + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr = std::move(ptr2) over ptr.reset(ptr2.release()) [misc-uniqueptr-reset-release] // CHECK-FIXES: a = std::move(b); a.reset(c.release()); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr1 = std::move(ptr2) + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr = std::move(ptr2) // CHECK-FIXES: a = std::move(c); a.reset(Create().release()); // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr = ReturnUnique() over ptr.reset(ReturnUnique().release()) [misc-uniqueptr-reset-release] // CHECK-FIXES: a = Create(); x->reset(y->release()); - // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: prefer ptr1 = std::move(ptr2) + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: prefer ptr = std::move(ptr2) // CHECK-FIXES: *x = std::move(*y); Look().reset(Look().release()); - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr1 = std::move(ptr2) + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr = std::move(ptr2) // CHECK-FIXES: Look() = std::move(Look()); Get()->reset(Get()->release()); - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr1 = std::move(ptr2) + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr = std::move(ptr2) // CHECK-FIXES: *Get() = std::move(*Get()); std::unique_ptr func_a, func_b; func_a.reset(func_b.release()); - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr1 = std::move(ptr2) + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr = std::move(ptr2) // CHECK-FIXES: func_a = std::move(func_b); } -- 2.7.4