Skip to content

Commit

Permalink
[clang-tidy][NFC] Replace usages of DeclSpec::TQ with `Qualifiers::…
Browse files Browse the repository at this point in the history
…TQ` (#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.
  • Loading branch information
Endilll authored and EricWF committed Oct 22, 2024
1 parent 43587f8 commit 593e408
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 21 deletions.
10 changes: 5 additions & 5 deletions clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,25 +172,25 @@ 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.
return;
}

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;
}

if (VC == VariableCategory::Pointer) {
if (WarnPointersAsValues && TransformPointersAsValues) {
Diag << addQualifierToVarDecl(*Variable, *Result.Context,
DeclSpec::TQ_const, QualifierTarget::Value,
Qualifiers::Const, QualifierTarget::Value,
QualifierPolicy::Right);
}
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
19 changes: 11 additions & 8 deletions clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down Expand Up @@ -71,15 +72,17 @@ 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(' ') + Qualifiers::fromCVRMask(Qualifier).getAsString())
.str();
return (llvm::Twine(Qualifiers::fromCVRMask(Qualifier).getAsString()) + " ")
.str();
}

static std::optional<FixItHint> changeValue(const VarDecl &Var,
DeclSpec::TQ Qualifier,
Qualifiers::TQ Qualifier,
QualifierTarget QualTarget,
QualifierPolicy QualPolicy,
const ASTContext &Context) {
Expand All @@ -99,7 +102,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;
Expand All @@ -112,7 +115,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
Expand Down Expand Up @@ -163,7 +166,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))
Expand All @@ -183,7 +186,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 ||
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 593e408

Please sign in to comment.