[clang-tools-extra] [clang-tidy][NFC] Replace usages of `DeclSpec::TQ` with `Qualifiers::TQ` (PR #113295)
Vlad Serebrennikov via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 22 04:53:34 PDT 2024
https://github.com/Endilll updated https://github.com/llvm/llvm-project/pull/113295
>From c618860ee1fc6bbac5d262b8edf5141e7c18d427 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Tue, 22 Oct 2024 14:12:37 +0300
Subject: [PATCH 1/2] [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;
>From 364f68704d921b3f2e683edb6d455ca39a42d847 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Tue, 22 Oct 2024 14:53:19 +0300
Subject: [PATCH 2/2] Reimplement `buildQualifier()` in terms of
`Qualifiers::getAsString()`
---
clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
index fa2f894d40a31e..a15589f9721c76 100644
--- a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
@@ -75,12 +75,9 @@ static std::optional<FixItHint> fixIfNotDangerous(SourceLocation Loc,
static std::string buildQualifier(Qualifiers::TQ Qualifier,
bool WhitespaceBefore = false) {
if (WhitespaceBefore)
- return (llvm::Twine(' ') +
- DeclSpec::getSpecifierName(static_cast<DeclSpec::TQ>(Qualifier)))
+ return (llvm::Twine(' ') + Qualifiers::fromCVRMask(Qualifier).getAsString())
.str();
- return (llvm::Twine(DeclSpec::getSpecifierName(
- static_cast<DeclSpec::TQ>(Qualifier))) +
- " ")
+ return (llvm::Twine(Qualifiers::fromCVRMask(Qualifier).getAsString()) + " ")
.str();
}
More information about the cfe-commits
mailing list