[clang-tidy][NFC] Replace usages of DeclSpec::TQ with Qualifiers::TQ
#113295
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
Endilll
requested review from
cor3ntin,
AaronBallman,
kazutakahirata and
HerrCai0907
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: Vlad Serebrennikov (Endilll) ChangesThis 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 ( Full diff: https://github.com/llvm/llvm-project/pull/113295.diff 7 Files Affected:
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;
|
cor3ntin
reviewed
Oct 22, 2024
|
LGTM, Thanks - However this is a clang-tidy change so you should wait for its maintainers to approve it |
It would be nice to know who I'm waiting for, because there are not listed maintainers for clang-tidy. Is it Aaron then? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 (
buildQualifierinFixItHintUtils.cpp), butQualifiers::TQdoesn't offer such function. Even more, the set of enumerators is not complete compared toDeclSpec::TQ, so I'm afraid that this would be a functional change.