https://github.com/Endilll created https://github.com/llvm/llvm-project/pull/113295
This patch improves, but doens't fully resolve the layering violation, which stems from relying on Sema. There's one function that needs to convert enumerator to a string (`buildQualifier` in `FixItHintUtils.cpp`), but `Qualifiers::TQ` doesn't offer such function. Even more, the set of enumerators is not complete compared to `DeclSpec::TQ`, so I'm afraid that this would be a functional change. >From c618860ee1fc6bbac5d262b8edf5141e7c18d427 Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Tue, 22 Oct 2024 14:12:37 +0300 Subject: [PATCH] [clang-tidy][NFC] Replace usages of `DeclSpec::TQ` with `Qualifiers::TQ` This patch improves, but doens't fully resolve the layering violation, which stems from relying on Sema. There's one function that needs to convert enumerator to a string, but `Qualifiers::TQ` doesn't offer such function. Even more, the set of enumerators is not complete compared to `DeclSpec::TQ`, so I'm afraid that this would be a functional change. --- .../clang-tidy/misc/ConstCorrectnessCheck.cpp | 10 ++++----- .../performance/ForRangeCopyCheck.cpp | 4 ++-- .../UnnecessaryCopyInitialization.cpp | 2 +- .../UnnecessaryValueParamCheck.cpp | 2 +- .../clang-tidy/utils/FixItHintUtils.cpp | 22 ++++++++++++------- .../clang-tidy/utils/FixItHintUtils.h | 4 ++-- .../unittests/clang-tidy/AddConstTest.cpp | 4 ++-- 7 files changed, 27 insertions(+), 21 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp index e20cf6fbcb55a7..71a4cee4bdc6ef 100644 --- a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp @@ -172,8 +172,8 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) { using namespace utils::fixit; if (VC == VariableCategory::Value && TransformValues) { - Diag << addQualifierToVarDecl(*Variable, *Result.Context, - DeclSpec::TQ_const, QualifierTarget::Value, + Diag << addQualifierToVarDecl(*Variable, *Result.Context, Qualifiers::Const, + QualifierTarget::Value, QualifierPolicy::Right); // FIXME: Add '{}' for default initialization if no user-defined default // constructor exists and there is no initializer. @@ -181,8 +181,8 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) { } if (VC == VariableCategory::Reference && TransformReferences) { - Diag << addQualifierToVarDecl(*Variable, *Result.Context, - DeclSpec::TQ_const, QualifierTarget::Value, + Diag << addQualifierToVarDecl(*Variable, *Result.Context, Qualifiers::Const, + QualifierTarget::Value, QualifierPolicy::Right); return; } @@ -190,7 +190,7 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) { if (VC == VariableCategory::Pointer) { if (WarnPointersAsValues && TransformPointersAsValues) { Diag << addQualifierToVarDecl(*Variable, *Result.Context, - DeclSpec::TQ_const, QualifierTarget::Value, + Qualifiers::Const, QualifierTarget::Value, QualifierPolicy::Right); } return; diff --git a/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp b/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp index 655e480ffa62cb..f7d44bf8631419 100644 --- a/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp @@ -91,7 +91,7 @@ bool ForRangeCopyCheck::handleConstValueCopy(const VarDecl &LoopVar, << utils::fixit::changeVarDeclToReference(LoopVar, Context); if (!LoopVar.getType().isConstQualified()) { if (std::optional<FixItHint> Fix = utils::fixit::addQualifierToVarDecl( - LoopVar, Context, DeclSpec::TQ::TQ_const)) + LoopVar, Context, Qualifiers::Const)) Diagnostic << *Fix; } return true; @@ -129,7 +129,7 @@ bool ForRangeCopyCheck::handleCopyIsOnlyConstReferenced( "making it a const reference"); if (std::optional<FixItHint> Fix = utils::fixit::addQualifierToVarDecl( - LoopVar, Context, DeclSpec::TQ::TQ_const)) + LoopVar, Context, Qualifiers::Const)) Diag << *Fix << utils::fixit::changeVarDeclToReference(LoopVar, Context); return true; diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp index 61240fa4b0eb8e..034894c11bf2c0 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -36,7 +36,7 @@ void recordFixes(const VarDecl &Var, ASTContext &Context, Diagnostic << utils::fixit::changeVarDeclToReference(Var, Context); if (!Var.getType().isLocalConstQualified()) { if (std::optional<FixItHint> Fix = utils::fixit::addQualifierToVarDecl( - Var, Context, DeclSpec::TQ::TQ_const)) + Var, Context, Qualifiers::Const)) Diagnostic << *Fix; } } diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp index 3a255c5c133f1d..d356f866a8804b 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp @@ -172,7 +172,7 @@ void UnnecessaryValueParamCheck::handleConstRefFix(const FunctionDecl &Function, // declaration. if (!CurrentParam.getType().getCanonicalType().isConstQualified()) { if (std::optional<FixItHint> Fix = utils::fixit::addQualifierToVarDecl( - CurrentParam, Context, DeclSpec::TQ::TQ_const)) + CurrentParam, Context, Qualifiers::Const)) Diag << *Fix; } } diff --git a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp index bbdd4326b0bac2..fa2f894d40a31e 100644 --- a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp +++ b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp @@ -11,6 +11,7 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/Type.h" +#include "clang/Sema/DeclSpec.h" #include "clang/Tooling/FixIt.h" #include <optional> @@ -71,15 +72,20 @@ static std::optional<FixItHint> fixIfNotDangerous(SourceLocation Loc, // Build a string that can be emitted as FixIt with either a space in before // or after the qualifier, either ' const' or 'const '. -static std::string buildQualifier(DeclSpec::TQ Qualifier, +static std::string buildQualifier(Qualifiers::TQ Qualifier, bool WhitespaceBefore = false) { if (WhitespaceBefore) - return (llvm::Twine(' ') + DeclSpec::getSpecifierName(Qualifier)).str(); - return (llvm::Twine(DeclSpec::getSpecifierName(Qualifier)) + " ").str(); + return (llvm::Twine(' ') + + DeclSpec::getSpecifierName(static_cast<DeclSpec::TQ>(Qualifier))) + .str(); + return (llvm::Twine(DeclSpec::getSpecifierName( + static_cast<DeclSpec::TQ>(Qualifier))) + + " ") + .str(); } static std::optional<FixItHint> changeValue(const VarDecl &Var, - DeclSpec::TQ Qualifier, + Qualifiers::TQ Qualifier, QualifierTarget QualTarget, QualifierPolicy QualPolicy, const ASTContext &Context) { @@ -99,7 +105,7 @@ static std::optional<FixItHint> changeValue(const VarDecl &Var, } static std::optional<FixItHint> changePointerItself(const VarDecl &Var, - DeclSpec::TQ Qualifier, + Qualifiers::TQ Qualifier, const ASTContext &Context) { if (locDangerous(Var.getLocation())) return std::nullopt; @@ -112,7 +118,7 @@ static std::optional<FixItHint> changePointerItself(const VarDecl &Var, } static std::optional<FixItHint> -changePointer(const VarDecl &Var, DeclSpec::TQ Qualifier, const Type *Pointee, +changePointer(const VarDecl &Var, Qualifiers::TQ Qualifier, const Type *Pointee, QualifierTarget QualTarget, QualifierPolicy QualPolicy, const ASTContext &Context) { // The pointer itself shall be marked as `const`. This is always to the right @@ -163,7 +169,7 @@ changePointer(const VarDecl &Var, DeclSpec::TQ Qualifier, const Type *Pointee, } static std::optional<FixItHint> -changeReferencee(const VarDecl &Var, DeclSpec::TQ Qualifier, QualType Pointee, +changeReferencee(const VarDecl &Var, Qualifiers::TQ Qualifier, QualType Pointee, QualifierTarget QualTarget, QualifierPolicy QualPolicy, const ASTContext &Context) { if (QualPolicy == QualifierPolicy::Left && isValueType(Pointee)) @@ -183,7 +189,7 @@ changeReferencee(const VarDecl &Var, DeclSpec::TQ Qualifier, QualType Pointee, std::optional<FixItHint> addQualifierToVarDecl(const VarDecl &Var, const ASTContext &Context, - DeclSpec::TQ Qualifier, + Qualifiers::TQ Qualifier, QualifierTarget QualTarget, QualifierPolicy QualPolicy) { assert((QualPolicy == QualifierPolicy::Left || diff --git a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.h b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.h index 2b96b2b2ce600c..e690dbaefe6426 100644 --- a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.h +++ b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.h @@ -11,7 +11,7 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" -#include "clang/Sema/DeclSpec.h" +#include "clang/AST/Type.h" #include <optional> namespace clang::tidy::utils::fixit { @@ -41,7 +41,7 @@ enum class QualifierTarget { /// Requires that `Var` is isolated in written code like in `int foo = 42;`. std::optional<FixItHint> addQualifierToVarDecl(const VarDecl &Var, const ASTContext &Context, - DeclSpec::TQ Qualifier, + Qualifiers::TQ Qualifier, QualifierTarget QualTarget = QualifierTarget::Pointee, QualifierPolicy QualPolicy = QualifierPolicy::Left); diff --git a/clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp b/clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp index dfae25f3f26eb6..d8c76049e393f9 100644 --- a/clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp @@ -27,8 +27,8 @@ class ConstTransform : public ClangTidyCheck { void check(const MatchFinder::MatchResult &Result) override { const auto *D = Result.Nodes.getNodeAs<VarDecl>("var"); using utils::fixit::addQualifierToVarDecl; - std::optional<FixItHint> Fix = addQualifierToVarDecl( - *D, *Result.Context, DeclSpec::TQ::TQ_const, CT, CP); + std::optional<FixItHint> Fix = + addQualifierToVarDecl(*D, *Result.Context, Qualifiers::Const, CT, CP); auto Diag = diag(D->getBeginLoc(), "doing const transformation"); if (Fix) Diag << *Fix; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits