Skip to content

Commit 14f7bd6

Browse files
mizvekovmizvekov
authored
Reland: [clang] preserve class type sugar when taking pointer to member (#132401)
Original PR: #130537 Originally reverted due to revert of dependent commit. Relanding with no changes. This changes the MemberPointerType representation to use a NestedNameSpecifier instead of a Type to represent the base class. Since the qualifiers are always parsed as nested names, there was an impedance mismatch when converting these back and forth into types, and this led to issues in preserving sugar. The nested names are indeed a better match for these, as the differences which a QualType can represent cannot be expressed syntatically, and they represent the use case more exactly, being either dependent or referring to a CXXRecord, unqualified. This patch also makes the MemberPointerType able to represent sugar for a {up/downcast}cast conversion of the base class, although for now the underlying type is canonical, as preserving the sugar up to that point requires further work. As usual, includes a few drive-by fixes in order to make use of the improvements.
1 parent 4dd7fea commit 14f7bd6

File tree

71 files changed

+1044
-427
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

71 files changed

+1044
-427
lines changed

‎clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -493,8 +493,7 @@ UseNullptrCheck::UseNullptrCheck(StringRef Name, ClangTidyContext *Context)
493493
: ClangTidyCheck(Name, Context),
494494
NullMacrosStr(Options.get("NullMacros", "NULL")),
495495
IgnoredTypes(utils::options::parseStringList(Options.get(
496-
"IgnoredTypes",
497-
"std::_CmpUnspecifiedParam::;^std::__cmp_cat::__unspec"))) {
496+
"IgnoredTypes", "_CmpUnspecifiedParam;^std::__cmp_cat::__unspec"))) {
498497
StringRef(NullMacrosStr).split(NullMacros, ",");
499498
}
500499

‎clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,9 @@ bool isFunctionPointerConvertible(QualType From, QualType To) {
178178

179179
// Note: converting Derived::* to Base::* is a different kind of conversion,
180180
// called Pointer-to-member conversion.
181-
return FromMember->getClass() == ToMember->getClass() &&
181+
return FromMember->getQualifier() == ToMember->getQualifier() &&
182+
FromMember->getMostRecentCXXRecordDecl() ==
183+
ToMember->getMostRecentCXXRecordDecl() &&
182184
FromMember->getPointeeType() == ToMember->getPointeeType();
183185
}
184186

‎clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1489,7 +1489,7 @@ TEST_F(FindExplicitReferencesTest, AllRefsInFoo) {
14891489
"4: targets = {a}\n"
14901490
"5: targets = {a::b}, qualifier = 'a::'\n"
14911491
"6: targets = {a::b::S}\n"
1492-
"7: targets = {a::b::S::type}, qualifier = 'struct S::'\n"
1492+
"7: targets = {a::b::S::type}, qualifier = 'S::'\n"
14931493
"8: targets = {y}, decl\n"},
14941494
{R"cpp(
14951495
void foo() {

‎clang/docs/ReleaseNotes.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ Improvements to Clang's diagnostics
267267
under the subgroup ``-Wunsafe-buffer-usage-in-libc-call``.
268268
- Diagnostics on chained comparisons (``a < b < c``) are now an error by default. This can be disabled with
269269
``-Wno-error=parentheses``.
270+
- Clang now better preserves the sugared types of pointers to member.
270271
- The ``-Wshift-bool`` warning has been added to warn about shifting a boolean. (#GH28334)
271272
- Fixed diagnostics adding a trailing ``::`` when printing some source code
272273
constructs, like base classes.

‎clang/include/clang/AST/ASTContext.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1558,10 +1558,9 @@ class ASTContext : public RefCountedBase<ASTContext> {
15581558
QualType getRValueReferenceType(QualType T) const;
15591559

15601560
/// Return the uniqued reference to the type for a member pointer to
1561-
/// the specified type in the specified class.
1562-
///
1563-
/// The class \p Cls is a \c Type because it could be a dependent name.
1564-
QualType getMemberPointerType(QualType T, const Type *Cls) const;
1561+
/// the specified type in the specified nested name.
1562+
QualType getMemberPointerType(QualType T, NestedNameSpecifier *Qualifier,
1563+
const CXXRecordDecl *Cls) const;
15651564

15661565
/// Return a non-unique reference to the type for a variable array of
15671566
/// the specified element type.

‎clang/include/clang/AST/ASTNodeTraverser.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,9 @@ class ASTNodeTraverser
393393
Visit(T->getPointeeType());
394394
}
395395
void VisitMemberPointerType(const MemberPointerType *T) {
396-
Visit(T->getClass());
396+
// FIXME: Provide a NestedNameSpecifier visitor.
397+
Visit(T->getQualifier()->getAsType());
398+
Visit(T->getMostRecentCXXRecordDecl());
397399
Visit(T->getPointeeType());
398400
}
399401
void VisitArrayType(const ArrayType *T) { Visit(T->getElementType()); }
@@ -485,7 +487,8 @@ class ASTNodeTraverser
485487
}
486488
}
487489
void VisitMemberPointerTypeLoc(MemberPointerTypeLoc TL) {
488-
Visit(TL.getClassTInfo()->getTypeLoc());
490+
// FIXME: Provide NestedNamespecifierLoc visitor.
491+
Visit(TL.getQualifierLoc().getTypeLoc());
489492
}
490493
void VisitVariableArrayTypeLoc(VariableArrayTypeLoc TL) {
491494
Visit(TL.getSizeExpr());

‎clang/include/clang/AST/CanonicalType.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ template<>
453453
struct CanProxyAdaptor<MemberPointerType>
454454
: public CanProxyBase<MemberPointerType> {
455455
LLVM_CLANG_CANPROXY_TYPE_ACCESSOR(getPointeeType)
456-
LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(const Type *, getClass)
456+
LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(NestedNameSpecifier *, getQualifier)
457457
LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(const CXXRecordDecl *,
458458
getMostRecentCXXRecordDecl)
459459
};

‎clang/include/clang/AST/RecursiveASTVisitor.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,7 +1004,8 @@ DEF_TRAVERSE_TYPE(RValueReferenceType,
10041004
{ TRY_TO(TraverseType(T->getPointeeType())); })
10051005

10061006
DEF_TRAVERSE_TYPE(MemberPointerType, {
1007-
TRY_TO(TraverseType(QualType(T->getClass(), 0)));
1007+
TRY_TO(TraverseNestedNameSpecifier(T->getQualifier()));
1008+
TRY_TO(TraverseDecl(T->getMostRecentCXXRecordDecl()));
10081009
TRY_TO(TraverseType(T->getPointeeType()));
10091010
})
10101011

@@ -1269,10 +1270,10 @@ DEF_TRAVERSE_TYPELOC(RValueReferenceType,
12691270
// We traverse this in the type case as well, but how is it not reached through
12701271
// the pointee type?
12711272
DEF_TRAVERSE_TYPELOC(MemberPointerType, {
1272-
if (auto *TSI = TL.getClassTInfo())
1273-
TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()));
1273+
if (NestedNameSpecifierLoc QL = TL.getQualifierLoc())
1274+
TRY_TO(TraverseNestedNameSpecifierLoc(QL));
12741275
else
1275-
TRY_TO(TraverseType(QualType(TL.getTypePtr()->getClass(), 0)));
1276+
TRY_TO(TraverseNestedNameSpecifier(TL.getTypePtr()->getQualifier()));
12761277
TRY_TO(TraverseTypeLoc(TL.getPointeeLoc()));
12771278
})
12781279

‎clang/include/clang/AST/Type.h

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3527,14 +3527,16 @@ class MemberPointerType : public Type, public llvm::FoldingSetNode {
35273527
QualType PointeeType;
35283528

35293529
/// The class of which the pointee is a member. Must ultimately be a
3530-
/// RecordType, but could be a typedef or a template parameter too.
3531-
const Type *Class;
3530+
/// CXXRecordType, but could be a typedef or a template parameter too.
3531+
NestedNameSpecifier *Qualifier;
35323532

3533-
MemberPointerType(QualType Pointee, const Type *Cls, QualType CanonicalPtr)
3533+
MemberPointerType(QualType Pointee, NestedNameSpecifier *Qualifier,
3534+
QualType CanonicalPtr)
35343535
: Type(MemberPointer, CanonicalPtr,
3535-
(Cls->getDependence() & ~TypeDependence::VariablyModified) |
3536+
(toTypeDependence(Qualifier->getDependence()) &
3537+
~TypeDependence::VariablyModified) |
35363538
Pointee->getDependence()),
3537-
PointeeType(Pointee), Class(Cls) {}
3539+
PointeeType(Pointee), Qualifier(Qualifier) {}
35383540

35393541
public:
35403542
QualType getPointeeType() const { return PointeeType; }
@@ -3551,21 +3553,21 @@ class MemberPointerType : public Type, public llvm::FoldingSetNode {
35513553
return !PointeeType->isFunctionProtoType();
35523554
}
35533555

3554-
const Type *getClass() const { return Class; }
3556+
NestedNameSpecifier *getQualifier() const { return Qualifier; }
35553557
CXXRecordDecl *getMostRecentCXXRecordDecl() const;
35563558

3557-
bool isSugared() const { return false; }
3558-
QualType desugar() const { return QualType(this, 0); }
3559+
bool isSugared() const;
3560+
QualType desugar() const {
3561+
return isSugared() ? getCanonicalTypeInternal() : QualType(this, 0);
3562+
}
35593563

35603564
void Profile(llvm::FoldingSetNodeID &ID) {
3561-
Profile(ID, getPointeeType(), getClass());
3565+
Profile(ID, getPointeeType(), getQualifier(), getMostRecentCXXRecordDecl());
35623566
}
35633567

35643568
static void Profile(llvm::FoldingSetNodeID &ID, QualType Pointee,
3565-
const Type *Class) {
3566-
ID.AddPointer(Pointee.getAsOpaquePtr());
3567-
ID.AddPointer(Class);
3568-
}
3569+
const NestedNameSpecifier *Qualifier,
3570+
const CXXRecordDecl *Cls);
35693571

35703572
static bool classof(const Type *T) {
35713573
return T->getTypeClass() == MemberPointer;

‎clang/include/clang/AST/TypeLoc.h

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ class TypeLoc {
139139
}
140140

141141
/// Get the pointer where source information is stored.
142+
// FIXME: This should provide a type-safe interface.
142143
void *getOpaqueData() const {
143144
return Data;
144145
}
@@ -1355,7 +1356,7 @@ class BlockPointerTypeLoc : public PointerLikeTypeLoc<BlockPointerTypeLoc,
13551356
};
13561357

13571358
struct MemberPointerLocInfo : public PointerLikeLocInfo {
1358-
TypeSourceInfo *ClassTInfo;
1359+
void *QualifierData = nullptr;
13591360
};
13601361

13611362
/// Wrapper for source info for member pointers.
@@ -1371,28 +1372,32 @@ class MemberPointerTypeLoc : public PointerLikeTypeLoc<MemberPointerTypeLoc,
13711372
setSigilLoc(Loc);
13721373
}
13731374

1374-
const Type *getClass() const {
1375-
return getTypePtr()->getClass();
1376-
}
1377-
1378-
TypeSourceInfo *getClassTInfo() const {
1379-
return getLocalData()->ClassTInfo;
1375+
NestedNameSpecifierLoc getQualifierLoc() const {
1376+
return NestedNameSpecifierLoc(getTypePtr()->getQualifier(),
1377+
getLocalData()->QualifierData);
13801378
}
13811379

1382-
void setClassTInfo(TypeSourceInfo* TI) {
1383-
getLocalData()->ClassTInfo = TI;
1380+
void setQualifierLoc(NestedNameSpecifierLoc QualifierLoc) {
1381+
assert(QualifierLoc.getNestedNameSpecifier() ==
1382+
getTypePtr()->getQualifier() &&
1383+
"Inconsistent nested-name-specifier pointer");
1384+
getLocalData()->QualifierData = QualifierLoc.getOpaqueData();
13841385
}
13851386

13861387
void initializeLocal(ASTContext &Context, SourceLocation Loc) {
13871388
setSigilLoc(Loc);
1388-
setClassTInfo(nullptr);
1389+
if (auto *Qualifier = getTypePtr()->getQualifier()) {
1390+
NestedNameSpecifierLocBuilder Builder;
1391+
Builder.MakeTrivial(Context, Qualifier, Loc);
1392+
setQualifierLoc(Builder.getWithLocInContext(Context));
1393+
} else
1394+
getLocalData()->QualifierData = nullptr;
13891395
}
13901396

13911397
SourceRange getLocalSourceRange() const {
1392-
if (TypeSourceInfo *TI = getClassTInfo())
1393-
return SourceRange(TI->getTypeLoc().getBeginLoc(), getStarLoc());
1394-
else
1395-
return SourceRange(getStarLoc());
1398+
if (NestedNameSpecifierLoc QL = getQualifierLoc())
1399+
return SourceRange(QL.getBeginLoc(), getStarLoc());
1400+
return SourceRange(getStarLoc());
13961401
}
13971402
};
13981403

0 commit comments

Comments
 (0)