Skip to content

Commit c1c419d

Browse files
mizvekovmizvekov
committed
[clang] preserve class type sugar when taking pointer to member
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 b326cb6 commit c1c419d

File tree

81 files changed

+1361
-659
lines changed

Some content is hidden

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

81 files changed

+1361
-659
lines changed

‎clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,7 @@ void ComparePointerToMemberVirtualFunctionCheck::check(
7070
// compare with variable which type is pointer to member function.
7171
llvm::SmallVector<SourceLocation, 12U> SameSignatureVirtualMethods{};
7272
const auto *MPT = cast<MemberPointerType>(DRE->getType().getCanonicalType());
73-
const Type *T = MPT->getClass();
74-
if (T == nullptr)
75-
return;
76-
const CXXRecordDecl *RD = T->getAsCXXRecordDecl();
73+
const CXXRecordDecl *RD = MPT->getMostRecentCXXRecordDecl();
7774
if (RD == nullptr)
7875
return;
7976

‎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: 5 additions & 3 deletions
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

@@ -219,8 +221,8 @@ bool isQualificationConvertiblePointer(QualType From, QualType To,
219221

220222
if (P1->isMemberPointerType())
221223
return P2->isMemberPointerType() &&
222-
P1->getAs<MemberPointerType>()->getClass() ==
223-
P2->getAs<MemberPointerType>()->getClass();
224+
P1->getAs<MemberPointerType>()->getMostRecentCXXRecordDecl() ==
225+
P2->getAs<MemberPointerType>()->getMostRecentCXXRecordDecl();
224226

225227
if (P1->isConstantArrayType())
226228
return P2->isConstantArrayType() &&

‎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: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,7 @@ Improvements to Clang's diagnostics
256256
under the subgroup ``-Wunsafe-buffer-usage-in-libc-call``.
257257
- Diagnostics on chained comparisons (``a < b < c``) are now an error by default. This can be disabled with
258258
``-Wno-error=parentheses``.
259+
- Clang now better preserves the sugared types of pointers to member.
259260
- The ``-Wshift-bool`` warning has been added to warn about shifting a boolean. (#GH28334)
260261
- Fixed diagnostics adding a trailing ``::`` when printing some source code
261262
constructs, like base classes.
@@ -264,7 +265,7 @@ Improvements to Clang's diagnostics
264265
as function arguments or return value respectively. Note that
265266
:doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The
266267
feature will be default-enabled with ``-Wthread-safety`` in a future release.
267-
- The ``-Wsign-compare`` warning now treats expressions with bitwise not(~) and minus(-) as signed integers
268+
- The ``-Wsign-compare`` warning now treats expressions with bitwise not(~) and minus(-) as signed integers
268269
except for the case where the operand is an unsigned integer
269270
and throws warning if they are compared with unsigned integers (##18878).
270271
- The ``-Wunnecessary-virtual-specifier`` warning has been added to warn about

‎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: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,9 @@ 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)
457+
LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(const CXXRecordDecl *,
458+
getMostRecentCXXRecordDecl)
457459
};
458460

459461
// CanProxyAdaptors for arrays are intentionally unimplemented because

‎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: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1355,7 +1355,7 @@ class BlockPointerTypeLoc : public PointerLikeTypeLoc<BlockPointerTypeLoc,
13551355
};
13561356

13571357
struct MemberPointerLocInfo : public PointerLikeLocInfo {
1358-
TypeSourceInfo *ClassTInfo;
1358+
void *QualifierData = nullptr;
13591359
};
13601360

13611361
/// Wrapper for source info for member pointers.
@@ -1371,28 +1371,32 @@ class MemberPointerTypeLoc : public PointerLikeTypeLoc<MemberPointerTypeLoc,
13711371
setSigilLoc(Loc);
13721372
}
13731373

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

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

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

13911396
SourceRange getLocalSourceRange() const {
1392-
if (TypeSourceInfo *TI = getClassTInfo())
1393-
return SourceRange(TI->getTypeLoc().getBeginLoc(), getStarLoc());
1394-
else
1395-
return SourceRange(getStarLoc());
1397+
if (NestedNameSpecifierLoc QL = getQualifierLoc())
1398+
return SourceRange(QL.getBeginLoc(), getStarLoc());
1399+
return SourceRange(getStarLoc());
13961400
}
13971401
};
13981402

‎clang/include/clang/AST/TypeProperties.td

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,12 +100,15 @@ let Class = MemberPointerType in {
100100
def : Property<"pointeeType", QualType> {
101101
let Read = [{ node->getPointeeType() }];
102102
}
103-
def : Property<"baseType", QualType> {
104-
let Read = [{ QualType(node->getClass(), 0) }];
103+
def : Property<"Qualifier", NestedNameSpecifier> {
104+
let Read = [{ node->getQualifier() }];
105+
}
106+
def : Property<"Cls", DeclRef> {
107+
let Read = [{ node->getMostRecentCXXRecordDecl() }];
105108
}
106109

107110
def : Creator<[{
108-
return ctx.getMemberPointerType(pointeeType, baseType.getTypePtr());
111+
return ctx.getMemberPointerType(pointeeType, Qualifier, cast_or_null<CXXRecordDecl>(Cls));
109112
}]>;
110113
}
111114

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7061,10 +7061,8 @@ def err_illegal_decl_mempointer_to_reference : Error<
70617061
"'%0' declared as a member pointer to a reference of type %1">;
70627062
def err_illegal_decl_mempointer_to_void : Error<
70637063
"'%0' declared as a member pointer to void">;
7064-
def err_illegal_decl_mempointer_in_nonclass : Error<
7065-
"'%0' does not point into a class">;
7066-
def err_mempointer_in_nonclass_type : Error<
7067-
"member pointer refers into non-class type %0">;
7064+
def err_illegal_decl_mempointer_in_nonclass
7065+
: Error<"'%0' does not point into a class">;
70687066
def err_reference_to_void : Error<"cannot form a reference to 'void'">;
70697067
def err_nonfunction_block_type : Error<
70707068
"block pointer to non-function type is invalid">;

‎clang/include/clang/Sema/Sema.h

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,6 +1348,12 @@ class Sema final : public SemaBase {
13481348
unsigned DiagID, bool ForceCheck = false,
13491349
bool ForceUnprivileged = false);
13501350

1351+
AccessResult CheckBaseClassAccess(
1352+
SourceLocation AccessLoc, CXXRecordDecl *Base, CXXRecordDecl *Derived,
1353+
const CXXBasePath &Path, unsigned DiagID,
1354+
llvm::function_ref<void(PartialDiagnostic &PD)> SetupPDiag,
1355+
bool ForceCheck = false, bool ForceUnprivileged = false);
1356+
13511357
/// Checks access to all the declarations in the given result set.
13521358
void CheckLookupAccess(const LookupResult &R);
13531359

@@ -5754,10 +5760,11 @@ class Sema final : public SemaBase {
57545760

57555761
/// Determine whether the type \p Derived is a C++ class that is
57565762
/// derived from the type \p Base.
5763+
bool IsDerivedFrom(SourceLocation Loc, CXXRecordDecl *Derived,
5764+
CXXRecordDecl *Base, CXXBasePaths &Paths);
5765+
bool IsDerivedFrom(SourceLocation Loc, CXXRecordDecl *Derived,
5766+
CXXRecordDecl *Base);
57575767
bool IsDerivedFrom(SourceLocation Loc, QualType Derived, QualType Base);
5758-
5759-
/// Determine whether the type \p Derived is a C++ class that is
5760-
/// derived from the type \p Base.
57615768
bool IsDerivedFrom(SourceLocation Loc, QualType Derived, QualType Base,
57625769
CXXBasePaths &Paths);
57635770

@@ -10040,15 +10047,24 @@ class Sema final : public SemaBase {
1004010047
bool InOverloadResolution,
1004110048
QualType &ConvertedType);
1004210049

10050+
enum class MemberPointerConversionResult {
10051+
Success,
10052+
DifferentPointee,
10053+
NotDerived,
10054+
Ambiguous,
10055+
Virtual,
10056+
Inaccessible
10057+
};
10058+
enum class MemberPointerConversionDirection : bool { Downcast, Upcast };
1004310059
/// CheckMemberPointerConversion - Check the member pointer conversion from
1004410060
/// the expression From to the type ToType. This routine checks for ambiguous
1004510061
/// or virtual or inaccessible base-to-derived member pointer conversions for
10046-
/// which IsMemberPointerConversion has already returned true. It returns true
10047-
/// and produces a diagnostic if there was an error, or returns false
10048-
/// otherwise.
10049-
bool CheckMemberPointerConversion(Expr *From, QualType ToType, CastKind &Kind,
10050-
CXXCastPath &BasePath,
10051-
bool IgnoreBaseAccess);
10062+
/// which IsMemberPointerConversion has already returned true. It produces a
10063+
// diagnostic if there was an error.
10064+
MemberPointerConversionResult CheckMemberPointerConversion(
10065+
QualType FromType, const MemberPointerType *ToPtrType, CastKind &Kind,
10066+
CXXCastPath &BasePath, SourceLocation CheckLoc, SourceRange OpRange,
10067+
bool IgnoreBaseAccess, MemberPointerConversionDirection Direction);
1005210068

1005310069
/// IsQualificationConversion - Determines whether the conversion from
1005410070
/// an rvalue of type FromType to ToType is a qualification conversion
@@ -14855,8 +14871,9 @@ class Sema final : public SemaBase {
1485514871
///
1485614872
/// \returns a member pointer type, if successful, or a NULL type if there was
1485714873
/// an error.
14858-
QualType BuildMemberPointerType(QualType T, QualType Class,
14859-
SourceLocation Loc, DeclarationName Entity);
14874+
QualType BuildMemberPointerType(QualType T, NestedNameSpecifier *Qualifier,
14875+
CXXRecordDecl *Cls, SourceLocation Loc,
14876+
DeclarationName Entity);
1486014877

1486114878
/// Build a block pointer type.
1486214879
///

0 commit comments

Comments
 (0)