Skip to content

[PATCH 4/7] [clang] Improve NestedNameSpecifier: clang-tools-extra changes #148015

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: users/mizvekov/name-qualification-refactor-3
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,7 @@ void FindAllSymbols::registerMatchers(MatchFinder *MatchFinder) {
// Uses of most types: just look at what the typeLoc refers to.
MatchFinder->addMatcher(
typeLoc(isExpansionInMainFile(),
loc(qualType(allOf(unless(elaboratedType()),
hasDeclaration(Types.bind("use")))))),
loc(qualType(hasDeclaration(Types.bind("use"))))),
this);
// Uses of typedefs: these are often transparent to hasDeclaration, so we need
// to handle them explicitly.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ static bool isDerivedClassBefriended(const CXXRecordDecl *CRTP,
return false;
}

return FriendType->getType()->getAsCXXRecordDecl() == Derived;
return declaresSameEntity(FriendType->getType()->getAsCXXRecordDecl(),
Derived);
});
}

Expand All @@ -55,7 +56,8 @@ getDerivedParameter(const ClassTemplateSpecializationDecl *CRTP,
CRTP->getTemplateArgs().asArray(), [&](const TemplateArgument &Arg) {
++Idx;
return Arg.getKind() == TemplateArgument::Type &&
Arg.getAsType()->getAsCXXRecordDecl() == Derived;
declaresSameEntity(Arg.getAsType()->getAsCXXRecordDecl(),
Derived);
});

return AnyOf ? CRTP->getSpecializedTemplate()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ approximateImplicitConversion(const TheCheck &Check, QualType LType,
ImplicitConversionModellingMode ImplicitMode);

static inline bool isUselessSugar(const Type *T) {
return isa<AttributedType, DecayedType, ElaboratedType, ParenType>(T);
return isa<AttributedType, DecayedType, ParenType>(T);
}

namespace {
Expand Down Expand Up @@ -1040,7 +1040,9 @@ approximateStandardConversionSequence(const TheCheck &Check, QualType From,
const auto *ToRecord = To->getAsCXXRecordDecl();
if (isDerivedToBase(FromRecord, ToRecord)) {
LLVM_DEBUG(llvm::dbgs() << "--- approximateStdConv. Derived To Base.\n");
WorkType = QualType{ToRecord->getTypeForDecl(), FastQualifiersToApply};
WorkType = QualType{
ToRecord->getASTContext().getCanonicalTagType(ToRecord)->getTypePtr(),
FastQualifiersToApply};
}

if (Ctx.getLangOpts().CPlusPlus17 && FromPtr && ToPtr) {
Expand Down Expand Up @@ -1072,9 +1074,9 @@ approximateStandardConversionSequence(const TheCheck &Check, QualType From,
WorkType = To;
}

if (WorkType == To) {
if (Ctx.hasSameType(WorkType, To)) {
LLVM_DEBUG(llvm::dbgs() << "<<< approximateStdConv. Reached 'To' type.\n");
return {WorkType};
return {Ctx.getCommonSugaredType(WorkType, To)};
}

LLVM_DEBUG(llvm::dbgs() << "<<< approximateStdConv. Did not reach 'To'.\n");
Expand Down Expand Up @@ -1219,7 +1221,7 @@ tryConversionOperators(const TheCheck &Check, const CXXRecordDecl *RD,

if (std::optional<UserDefinedConversionSelector::PreparedConversion>
SelectedConversion = ConversionSet()) {
QualType RecordType{RD->getTypeForDecl(), 0};
CanQualType RecordType = RD->getASTContext().getCanonicalTagType(RD);

ConversionSequence Result{RecordType, ToType};
// The conversion from the operator call's return type to ToType was
Expand Down Expand Up @@ -1270,7 +1272,7 @@ tryConvertingConstructors(const TheCheck &Check, QualType FromType,

if (std::optional<UserDefinedConversionSelector::PreparedConversion>
SelectedConversion = ConversionSet()) {
QualType RecordType{RD->getTypeForDecl(), 0};
CanQualType RecordType = RD->getASTContext().getCanonicalTagType(RD);

ConversionSequence Result{FromType, RecordType};
Result.AfterFirstStandard = SelectedConversion->Seq.AfterFirstStandard;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,9 @@ void ForwardDeclarationNamespaceCheck::check(
// struct B { friend A; };
// \endcode
// `A` will not be marked as "referenced" in the AST.
if (const TypeSourceInfo *Tsi = Decl->getFriendType()) {
QualType Desugared = Tsi->getType().getDesugaredType(*Result.Context);
FriendTypes.insert(Desugared.getTypePtr());
}
if (const TypeSourceInfo *Tsi = Decl->getFriendType())
FriendTypes.insert(
Tsi->getType()->getCanonicalTypeUnqualified().getTypePtr());
}
}

Expand Down Expand Up @@ -119,7 +118,9 @@ void ForwardDeclarationNamespaceCheck::onEndOfTranslationUnit() {
if (CurDecl->hasDefinition() || CurDecl->isReferenced()) {
continue; // Skip forward declarations that are used/referenced.
}
if (FriendTypes.contains(CurDecl->getTypeForDecl())) {
if (FriendTypes.contains(CurDecl->getASTContext()
.getCanonicalTagType(CurDecl)
->getTypePtr())) {
continue; // Skip forward declarations referenced as friend.
}
if (CurDecl->getLocation().isMacroID() ||
Expand Down
19 changes: 8 additions & 11 deletions clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,27 +32,22 @@ AST_MATCHER_P(TemplateTypeParmDecl, hasUnnamedDefaultArgument,
void IncorrectEnableIfCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
templateTypeParmDecl(
hasUnnamedDefaultArgument(
elaboratedTypeLoc(
hasNamedTypeLoc(templateSpecializationTypeLoc(
loc(qualType(hasDeclaration(namedDecl(
hasName("::std::enable_if"))))))
.bind("enable_if_specialization")))
.bind("elaborated")))
hasUnnamedDefaultArgument(templateSpecializationTypeLoc(
loc(qualType(hasDeclaration(namedDecl(
hasName("::std::enable_if"))))))
.bind("enable_if_specialization")))
.bind("enable_if"),
this);
}

void IncorrectEnableIfCheck::check(const MatchFinder::MatchResult &Result) {
const auto *EnableIf =
Result.Nodes.getNodeAs<TemplateTypeParmDecl>("enable_if");
const auto *ElaboratedLoc =
Result.Nodes.getNodeAs<ElaboratedTypeLoc>("elaborated");
const auto *EnableIfSpecializationLoc =
Result.Nodes.getNodeAs<TemplateSpecializationTypeLoc>(
"enable_if_specialization");

if (!EnableIf || !ElaboratedLoc || !EnableIfSpecializationLoc)
if (!EnableIf || !EnableIfSpecializationLoc)
return;

const SourceManager &SM = *Result.SourceManager;
Expand All @@ -62,8 +57,10 @@ void IncorrectEnableIfCheck::check(const MatchFinder::MatchResult &Result) {
auto Diag = diag(EnableIf->getBeginLoc(),
"incorrect std::enable_if usage detected; use "
"'typename std::enable_if<...>::type'");
// FIXME: This should handle the enable_if specialization already having an
// elaborated keyword.
if (!getLangOpts().CPlusPlus20) {
Diag << FixItHint::CreateInsertion(ElaboratedLoc->getBeginLoc(),
Diag << FixItHint::CreateInsertion(EnableIfSpecializationLoc->getBeginLoc(),
"typename ");
}
Diag << FixItHint::CreateInsertion(RAngleLoc.getLocWithOffset(1), "::type");
Expand Down
10 changes: 6 additions & 4 deletions clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
"suspicious usage of 'sizeof(array)/sizeof(...)';"
" denominator differs from the size of array elements")
<< E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
} else if (NumTy && DenomTy && NumTy == DenomTy &&
} else if (NumTy && DenomTy && Ctx.hasSameType(NumTy, DenomTy) &&
!NumTy->isDependentType()) {
// Dependent type should not be compared.
diag(E->getOperatorLoc(),
Expand All @@ -434,7 +434,7 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
<< E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
} else if (!WarnOnSizeOfPointer) {
// When 'WarnOnSizeOfPointer' is enabled, these messages become redundant:
if (PointedTy && DenomTy && PointedTy == DenomTy) {
if (PointedTy && DenomTy && Ctx.hasSameType(PointedTy, DenomTy)) {
diag(E->getOperatorLoc(),
"suspicious usage of 'sizeof(...)/sizeof(...)'; size of pointer "
"is divided by size of pointed type")
Expand Down Expand Up @@ -463,7 +463,8 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
const auto *SizeOfExpr =
Result.Nodes.getNodeAs<UnaryExprOrTypeTraitExpr>("sizeof-ptr-mul-expr");

if ((LPtrTy == RPtrTy) && (LPtrTy == SizeofArgTy)) {
if (Ctx.hasSameType(LPtrTy, RPtrTy) &&
Ctx.hasSameType(LPtrTy, SizeofArgTy)) {
diag(SizeOfExpr->getBeginLoc(), "suspicious usage of 'sizeof(...)' in "
"pointer arithmetic")
<< SizeOfExpr->getSourceRange() << E->getOperatorLoc()
Expand All @@ -477,7 +478,8 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
const auto *SizeOfExpr =
Result.Nodes.getNodeAs<UnaryExprOrTypeTraitExpr>("sizeof-ptr-div-expr");

if ((LPtrTy == RPtrTy) && (LPtrTy == SizeofArgTy)) {
if (Ctx.hasSameType(LPtrTy, RPtrTy) &&
Ctx.hasSameType(LPtrTy, SizeofArgTy)) {
diag(SizeOfExpr->getBeginLoc(), "suspicious usage of 'sizeof(...)' in "
"pointer arithmetic")
<< SizeOfExpr->getSourceRange() << E->getOperatorLoc()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ void NoSuspendWithLockCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
}

void NoSuspendWithLockCheck::registerMatchers(MatchFinder *Finder) {
auto LockType = elaboratedType(namesType(templateSpecializationType(
auto LockType = templateSpecializationType(
hasDeclaration(namedDecl(matchers::matchesAnyListedName(
utils::options::parseStringList(LockGuards)))))));
utils::options::parseStringList(LockGuards)))));

StatementMatcher Lock =
declStmt(has(varDecl(hasType(LockType)).bind("lock-decl")))
Expand Down
31 changes: 25 additions & 6 deletions clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,30 @@ static StringRef getDestTypeString(const SourceManager &SM,
SM, LangOpts);
}

static bool sameTypeAsWritten(QualType X, QualType Y) {
if (X.getCanonicalType() != Y.getCanonicalType())
return false;

auto TC = X->getTypeClass();
if (TC != Y->getTypeClass())
return false;

switch (TC) {
case Type::Typedef:
return declaresSameEntity(cast<TypedefType>(X)->getDecl(),
cast<TypedefType>(Y)->getDecl());
case Type::Pointer:
return sameTypeAsWritten(cast<PointerType>(X)->getPointeeType(),
cast<PointerType>(Y)->getPointeeType());
case Type::RValueReference:
case Type::LValueReference:
return sameTypeAsWritten(cast<ReferenceType>(X)->getPointeeType(),
cast<ReferenceType>(Y)->getPointeeType());
default:
return true;
}
}

void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult &Result) {
const auto *CastExpr = Result.Nodes.getNodeAs<ExplicitCastExpr>("cast");

Expand Down Expand Up @@ -128,12 +152,7 @@ void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult &Result) {
// case of overloaded functions, so detection of redundant casts is trickier
// in this case. Don't emit "redundant cast" warnings for function
// pointer/reference types.
QualType Src = SourceTypeAsWritten, Dst = DestTypeAsWritten;
if (const auto *ElTy = dyn_cast<ElaboratedType>(Src))
Src = ElTy->getNamedType();
if (const auto *ElTy = dyn_cast<ElaboratedType>(Dst))
Dst = ElTy->getNamedType();
if (Src == Dst) {
if (sameTypeAsWritten(SourceTypeAsWritten, DestTypeAsWritten)) {
diag(CastExpr->getBeginLoc(), "redundant cast to the same type")
<< FixItHint::CreateRemoval(ReplaceRange);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,13 @@ getAliasNameRange(const MatchFinder::MatchResult &Result) {
return CharSourceRange::getTokenRange(
Using->getNameInfo().getSourceRange());
}
return CharSourceRange::getTokenRange(
Result.Nodes.getNodeAs<TypeLoc>("typeloc")->getSourceRange());
TypeLoc TL = *Result.Nodes.getNodeAs<TypeLoc>("typeloc");
if (auto QTL = TL.getAs<QualifiedTypeLoc>())
TL = QTL.getUnqualifiedLoc();

if (auto TTL = TL.getAs<TypedefTypeLoc>())
return CharSourceRange::getTokenRange(TTL.getNameLoc());
return CharSourceRange::getTokenRange(TL.castAs<UsingTypeLoc>().getNameLoc());
}

void UpgradeGoogletestCaseCheck::check(const MatchFinder::MatchResult &Result) {
Expand Down
11 changes: 6 additions & 5 deletions clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,12 @@ void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) {
hasType(referenceType(pointee(hasCanonicalType(templateTypeParmType())))),
hasType(referenceType(pointee(substTemplateTypeParmType()))));

const auto AllowedType = hasType(qualType(anyOf(
hasDeclaration(namedDecl(matchers::matchesAnyListedName(AllowedTypes))),
references(namedDecl(matchers::matchesAnyListedName(AllowedTypes))),
pointerType(pointee(hasDeclaration(
namedDecl(matchers::matchesAnyListedName(AllowedTypes))))))));
auto AllowedTypeDecl = namedDecl(
anyOf(matchers::matchesAnyListedName(AllowedTypes), usingShadowDecl()));

const auto AllowedType = hasType(qualType(
anyOf(hasDeclaration(AllowedTypeDecl), references(AllowedTypeDecl),
pointerType(pointee(hasDeclaration(AllowedTypeDecl))))));

const auto AutoTemplateType = varDecl(
anyOf(hasType(autoType()), hasType(referenceType(pointee(autoType()))),
Expand Down
14 changes: 7 additions & 7 deletions clang-tools-extra/clang-tidy/misc/MisplacedConstCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ void MisplacedConstCheck::registerMatchers(MatchFinder *Finder) {
pointee(anyOf(isConstQualified(), ignoringParens(functionType()))))));

Finder->addMatcher(
valueDecl(hasType(qualType(
isConstQualified(),
elaboratedType(namesType(typedefType(hasDeclaration(
anyOf(typedefDecl(NonConstAndNonFunctionPointerType)
.bind("typedef"),
typeAliasDecl(NonConstAndNonFunctionPointerType)
.bind("typeAlias")))))))))
valueDecl(
hasType(qualType(isConstQualified(),
typedefType(hasDeclaration(anyOf(
typedefDecl(NonConstAndNonFunctionPointerType)
.bind("typedef"),
typeAliasDecl(NonConstAndNonFunctionPointerType)
.bind("typeAlias")))))))
.bind("decl"),
this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,8 @@ static bool areEquivalentExpr(const Expr *Left, const Expr *Right) {
if (cast<DependentScopeDeclRefExpr>(Left)->getDeclName() !=
cast<DependentScopeDeclRefExpr>(Right)->getDeclName())
return false;
return areEquivalentNameSpecifier(
cast<DependentScopeDeclRefExpr>(Left)->getQualifier(),
cast<DependentScopeDeclRefExpr>(Right)->getQualifier());
return cast<DependentScopeDeclRefExpr>(Left)->getQualifier() ==
cast<DependentScopeDeclRefExpr>(Right)->getQualifier();
case Stmt::DeclRefExprClass:
return cast<DeclRefExpr>(Left)->getDecl() ==
cast<DeclRefExpr>(Right)->getDecl();
Expand Down
6 changes: 1 addition & 5 deletions clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,7 @@ void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) {
templateArgument().bind("used")))),
this);
Finder->addMatcher(userDefinedLiteral().bind("used"), this);
Finder->addMatcher(
loc(elaboratedType(unless(hasQualifier(nestedNameSpecifier())),
hasUnqualifiedDesugaredType(
type(asTagDecl(tagDecl().bind("used")))))),
this);
Finder->addMatcher(loc(asTagDecl(tagDecl().bind("used"))), this);
// Cases where we can identify the UsingShadowDecl directly, rather than
// just its target.
// FIXME: cover more cases in this way, as the AST supports it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ static std::optional<const char *> getReplacementType(StringRef Type) {

void DeprecatedIosBaseAliasesCheck::registerMatchers(MatchFinder *Finder) {
auto IoStateDecl = typedefDecl(hasAnyName(DeprecatedTypes)).bind("TypeDecl");
auto IoStateType =
qualType(hasDeclaration(IoStateDecl), unless(elaboratedType()));
auto IoStateType = typedefType(hasDeclaration(IoStateDecl));

Finder->addMatcher(typeLoc(loc(IoStateType)).bind("TypeLoc"), this);
}
Expand All @@ -43,12 +42,14 @@ void DeprecatedIosBaseAliasesCheck::check(
StringRef TypeName = Typedef->getName();
auto Replacement = getReplacementType(TypeName);

const auto *TL = Result.Nodes.getNodeAs<TypeLoc>("TypeLoc");
SourceLocation IoStateLoc = TL->getBeginLoc();
TypeLoc TL = *Result.Nodes.getNodeAs<TypeLoc>("TypeLoc");
if (auto QTL = TL.getAs<QualifiedTypeLoc>())
TL = QTL.getUnqualifiedLoc();

SourceLocation IoStateLoc = TL.castAs<TypedefTypeLoc>().getNameLoc();
// Do not generate fixits for matches depending on template arguments and
// macro expansions.
bool Fix = Replacement && !TL->getType()->isDependentType();
bool Fix = Replacement && !TL.getType()->isDependentType();
if (IoStateLoc.isMacroID()) {
IoStateLoc = SM.getSpellingLoc(IoStateLoc);
Fix = false;
Expand Down
3 changes: 1 addition & 2 deletions clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ AST_MATCHER_P(CXXRecordDecl, isMoveConstructibleInBoundCXXRecordDecl, StringRef,

static TypeMatcher notTemplateSpecConstRefType() {
return lValueReferenceType(
pointee(unless(elaboratedType(namesType(templateSpecializationType()))),
isConstQualified()));
pointee(unless(templateSpecializationType()), isConstQualified()));
}

static TypeMatcher nonConstValueType() {
Expand Down
Loading
Loading