Skip to content

Revert "[clang] improve class type sugar preservation in pointers to members" #132215

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

Merged
Merged
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
3 changes: 2 additions & 1 deletion clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,8 @@ UseNullptrCheck::UseNullptrCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
NullMacrosStr(Options.get("NullMacros", "NULL")),
IgnoredTypes(utils::options::parseStringList(Options.get(
"IgnoredTypes", "_CmpUnspecifiedParam;^std::__cmp_cat::__unspec"))) {
"IgnoredTypes",
"std::_CmpUnspecifiedParam::;^std::__cmp_cat::__unspec"))) {
StringRef(NullMacrosStr).split(NullMacros, ",");
}

Expand Down
4 changes: 1 addition & 3 deletions clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,7 @@ bool isFunctionPointerConvertible(QualType From, QualType To) {

// Note: converting Derived::* to Base::* is a different kind of conversion,
// called Pointer-to-member conversion.
return FromMember->getQualifier() == ToMember->getQualifier() &&
FromMember->getMostRecentCXXRecordDecl() ==
ToMember->getMostRecentCXXRecordDecl() &&
return FromMember->getClass() == ToMember->getClass() &&
FromMember->getPointeeType() == ToMember->getPointeeType();
}

Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/unittests/FindTargetTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1489,7 +1489,7 @@ TEST_F(FindExplicitReferencesTest, AllRefsInFoo) {
"4: targets = {a}\n"
"5: targets = {a::b}, qualifier = 'a::'\n"
"6: targets = {a::b::S}\n"
"7: targets = {a::b::S::type}, qualifier = 'S::'\n"
"7: targets = {a::b::S::type}, qualifier = 'struct S::'\n"
"8: targets = {y}, decl\n"},
{R"cpp(
void foo() {
Expand Down
1 change: 0 additions & 1 deletion clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,6 @@ Improvements to Clang's diagnostics
under the subgroup ``-Wunsafe-buffer-usage-in-libc-call``.
- Diagnostics on chained comparisons (``a < b < c``) are now an error by default. This can be disabled with
``-Wno-error=parentheses``.
- Clang now better preserves the sugared types of pointers to member.
- The ``-Wshift-bool`` warning has been added to warn about shifting a boolean. (#GH28334)
- Fixed diagnostics adding a trailing ``::`` when printing some source code
constructs, like base classes.
Expand Down
7 changes: 4 additions & 3 deletions clang/include/clang/AST/ASTContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -1558,9 +1558,10 @@ class ASTContext : public RefCountedBase<ASTContext> {
QualType getRValueReferenceType(QualType T) const;

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

/// Return a non-unique reference to the type for a variable array of
/// the specified element type.
Expand Down
7 changes: 2 additions & 5 deletions clang/include/clang/AST/ASTNodeTraverser.h
Original file line number Diff line number Diff line change
Expand Up @@ -393,9 +393,7 @@ class ASTNodeTraverser
Visit(T->getPointeeType());
}
void VisitMemberPointerType(const MemberPointerType *T) {
// FIXME: Provide a NestedNameSpecifier visitor.
Visit(T->getQualifier()->getAsType());
Visit(T->getMostRecentCXXRecordDecl());
Visit(T->getClass());
Visit(T->getPointeeType());
}
void VisitArrayType(const ArrayType *T) { Visit(T->getElementType()); }
Expand Down Expand Up @@ -487,8 +485,7 @@ class ASTNodeTraverser
}
}
void VisitMemberPointerTypeLoc(MemberPointerTypeLoc TL) {
// FIXME: Provide NestedNamespecifierLoc visitor.
Visit(TL.getQualifierLoc().getTypeLoc());
Visit(TL.getClassTInfo()->getTypeLoc());
}
void VisitVariableArrayTypeLoc(VariableArrayTypeLoc TL) {
Visit(TL.getSizeExpr());
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/AST/CanonicalType.h
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ template<>
struct CanProxyAdaptor<MemberPointerType>
: public CanProxyBase<MemberPointerType> {
LLVM_CLANG_CANPROXY_TYPE_ACCESSOR(getPointeeType)
LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(NestedNameSpecifier *, getQualifier)
LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(const Type *, getClass)
LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(const CXXRecordDecl *,
getMostRecentCXXRecordDecl)
};
Expand Down
9 changes: 4 additions & 5 deletions clang/include/clang/AST/RecursiveASTVisitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -1004,8 +1004,7 @@ DEF_TRAVERSE_TYPE(RValueReferenceType,
{ TRY_TO(TraverseType(T->getPointeeType())); })

DEF_TRAVERSE_TYPE(MemberPointerType, {
TRY_TO(TraverseNestedNameSpecifier(T->getQualifier()));
TRY_TO(TraverseDecl(T->getMostRecentCXXRecordDecl()));
TRY_TO(TraverseType(QualType(T->getClass(), 0)));
TRY_TO(TraverseType(T->getPointeeType()));
})

Expand Down Expand Up @@ -1270,10 +1269,10 @@ DEF_TRAVERSE_TYPELOC(RValueReferenceType,
// We traverse this in the type case as well, but how is it not reached through
// the pointee type?
DEF_TRAVERSE_TYPELOC(MemberPointerType, {
if (NestedNameSpecifierLoc QL = TL.getQualifierLoc())
TRY_TO(TraverseNestedNameSpecifierLoc(QL));
if (auto *TSI = TL.getClassTInfo())
TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()));
else
TRY_TO(TraverseNestedNameSpecifier(TL.getTypePtr()->getQualifier()));
TRY_TO(TraverseType(QualType(TL.getTypePtr()->getClass(), 0)));
TRY_TO(TraverseTypeLoc(TL.getPointeeLoc()));
})

Expand Down
28 changes: 13 additions & 15 deletions clang/include/clang/AST/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -3527,16 +3527,14 @@ class MemberPointerType : public Type, public llvm::FoldingSetNode {
QualType PointeeType;

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

MemberPointerType(QualType Pointee, NestedNameSpecifier *Qualifier,
QualType CanonicalPtr)
MemberPointerType(QualType Pointee, const Type *Cls, QualType CanonicalPtr)
: Type(MemberPointer, CanonicalPtr,
(toTypeDependence(Qualifier->getDependence()) &
~TypeDependence::VariablyModified) |
(Cls->getDependence() & ~TypeDependence::VariablyModified) |
Pointee->getDependence()),
PointeeType(Pointee), Qualifier(Qualifier) {}
PointeeType(Pointee), Class(Cls) {}

public:
QualType getPointeeType() const { return PointeeType; }
Expand All @@ -3553,21 +3551,21 @@ class MemberPointerType : public Type, public llvm::FoldingSetNode {
return !PointeeType->isFunctionProtoType();
}

NestedNameSpecifier *getQualifier() const { return Qualifier; }
const Type *getClass() const { return Class; }
CXXRecordDecl *getMostRecentCXXRecordDecl() const;

bool isSugared() const;
QualType desugar() const {
return isSugared() ? getCanonicalTypeInternal() : QualType(this, 0);
}
bool isSugared() const { return false; }
QualType desugar() const { return QualType(this, 0); }

void Profile(llvm::FoldingSetNodeID &ID) {
Profile(ID, getPointeeType(), getQualifier(), getMostRecentCXXRecordDecl());
Profile(ID, getPointeeType(), getClass());
}

static void Profile(llvm::FoldingSetNodeID &ID, QualType Pointee,
const NestedNameSpecifier *Qualifier,
const CXXRecordDecl *Cls);
const Type *Class) {
ID.AddPointer(Pointee.getAsOpaquePtr());
ID.AddPointer(Class);
}

static bool classof(const Type *T) {
return T->getTypeClass() == MemberPointer;
Expand Down
33 changes: 14 additions & 19 deletions clang/include/clang/AST/TypeLoc.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ class TypeLoc {
}

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

struct MemberPointerLocInfo : public PointerLikeLocInfo {
void *QualifierData = nullptr;
TypeSourceInfo *ClassTInfo;
};

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

NestedNameSpecifierLoc getQualifierLoc() const {
return NestedNameSpecifierLoc(getTypePtr()->getQualifier(),
getLocalData()->QualifierData);
const Type *getClass() const {
return getTypePtr()->getClass();
}

void setQualifierLoc(NestedNameSpecifierLoc QualifierLoc) {
assert(QualifierLoc.getNestedNameSpecifier() ==
getTypePtr()->getQualifier() &&
"Inconsistent nested-name-specifier pointer");
getLocalData()->QualifierData = QualifierLoc.getOpaqueData();
TypeSourceInfo *getClassTInfo() const {
return getLocalData()->ClassTInfo;
}

void setClassTInfo(TypeSourceInfo* TI) {
getLocalData()->ClassTInfo = TI;
}

void initializeLocal(ASTContext &Context, SourceLocation Loc) {
setSigilLoc(Loc);
if (auto *Qualifier = getTypePtr()->getQualifier()) {
NestedNameSpecifierLocBuilder Builder;
Builder.MakeTrivial(Context, Qualifier, Loc);
setQualifierLoc(Builder.getWithLocInContext(Context));
} else
getLocalData()->QualifierData = nullptr;
setClassTInfo(nullptr);
}

SourceRange getLocalSourceRange() const {
if (NestedNameSpecifierLoc QL = getQualifierLoc())
return SourceRange(QL.getBeginLoc(), getStarLoc());
return SourceRange(getStarLoc());
if (TypeSourceInfo *TI = getClassTInfo())
return SourceRange(TI->getTypeLoc().getBeginLoc(), getStarLoc());
else
return SourceRange(getStarLoc());
}
};

Expand Down
9 changes: 3 additions & 6 deletions clang/include/clang/AST/TypeProperties.td
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,12 @@ let Class = MemberPointerType in {
def : Property<"pointeeType", QualType> {
let Read = [{ node->getPointeeType() }];
}
def : Property<"Qualifier", NestedNameSpecifier> {
let Read = [{ node->getQualifier() }];
}
def : Property<"Cls", DeclRef> {
let Read = [{ node->getMostRecentCXXRecordDecl() }];
def : Property<"baseType", QualType> {
let Read = [{ QualType(node->getClass(), 0) }];
}

def : Creator<[{
return ctx.getMemberPointerType(pointeeType, Qualifier, cast_or_null<CXXRecordDecl>(Cls));
return ctx.getMemberPointerType(pointeeType, baseType.getTypePtr());
}]>;
}

Expand Down
6 changes: 4 additions & 2 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -7065,8 +7065,10 @@ def err_illegal_decl_mempointer_to_reference : Error<
"'%0' declared as a member pointer to a reference of type %1">;
def err_illegal_decl_mempointer_to_void : Error<
"'%0' declared as a member pointer to void">;
def err_illegal_decl_mempointer_in_nonclass
: Error<"'%0' does not point into a class">;
def err_illegal_decl_mempointer_in_nonclass : Error<
"'%0' does not point into a class">;
def err_mempointer_in_nonclass_type : Error<
"member pointer refers into non-class type %0">;
def err_reference_to_void : Error<"cannot form a reference to 'void'">;
def err_nonfunction_block_type : Error<
"block pointer to non-function type is invalid">;
Expand Down
11 changes: 2 additions & 9 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -1348,12 +1348,6 @@ class Sema final : public SemaBase {
unsigned DiagID, bool ForceCheck = false,
bool ForceUnprivileged = false);

AccessResult CheckBaseClassAccess(
SourceLocation AccessLoc, CXXRecordDecl *Base, CXXRecordDecl *Derived,
const CXXBasePath &Path, unsigned DiagID,
llvm::function_ref<void(PartialDiagnostic &PD)> SetupPDiag,
bool ForceCheck = false, bool ForceUnprivileged = false);

/// Checks access to all the declarations in the given result set.
void CheckLookupAccess(const LookupResult &R);

Expand Down Expand Up @@ -14885,9 +14879,8 @@ class Sema final : public SemaBase {
///
/// \returns a member pointer type, if successful, or a NULL type if there was
/// an error.
QualType BuildMemberPointerType(QualType T, NestedNameSpecifier *Qualifier,
CXXRecordDecl *Cls, SourceLocation Loc,
DeclarationName Entity);
QualType BuildMemberPointerType(QualType T, QualType Class,
SourceLocation Loc, DeclarationName Entity);

/// Build a block pointer type.
///
Expand Down
68 changes: 21 additions & 47 deletions clang/lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3322,8 +3322,7 @@ static void encodeTypeForFunctionPointerAuth(const ASTContext &Ctx,
case Type::MemberPointer: {
OS << "M";
const auto *MPT = T->castAs<MemberPointerType>();
encodeTypeForFunctionPointerAuth(
Ctx, OS, QualType(MPT->getQualifier()->getAsType(), 0));
encodeTypeForFunctionPointerAuth(Ctx, OS, QualType(MPT->getClass(), 0));
encodeTypeForFunctionPointerAuth(Ctx, OS, MPT->getPointeeType());
return;
}
Expand Down Expand Up @@ -3512,8 +3511,7 @@ uint16_t ASTContext::getPointerAuthTypeDiscriminator(QualType T) {
if (PointeeType->castAs<FunctionProtoType>()->getExceptionSpecType() !=
EST_None) {
QualType FT = getFunctionTypeWithExceptionSpec(PointeeType, EST_None);
T = getMemberPointerType(FT, MPT->getQualifier(),
MPT->getMostRecentCXXRecordDecl());
T = getMemberPointerType(FT, MPT->getClass());
}
}
std::unique_ptr<MangleContext> MC(createMangleContext());
Expand Down Expand Up @@ -4027,50 +4025,32 @@ QualType ASTContext::getRValueReferenceType(QualType T) const {
return QualType(New, 0);
}

QualType ASTContext::getMemberPointerType(QualType T,
NestedNameSpecifier *Qualifier,
const CXXRecordDecl *Cls) const {
if (!Qualifier) {
assert(Cls && "At least one of Qualifier or Cls must be provided");
Qualifier = NestedNameSpecifier::Create(*this, /*Prefix=*/nullptr,
/*Template=*/false,
getTypeDeclType(Cls).getTypePtr());
} else if (!Cls) {
Cls = Qualifier->getAsRecordDecl();
}
/// getMemberPointerType - Return the uniqued reference to the type for a
/// member pointer to the specified type, in the specified class.
QualType ASTContext::getMemberPointerType(QualType T, const Type *Cls) const {
// Unique pointers, to guarantee there is only one pointer of a particular
// structure.
llvm::FoldingSetNodeID ID;
MemberPointerType::Profile(ID, T, Qualifier, Cls);
MemberPointerType::Profile(ID, T, Cls);

void *InsertPos = nullptr;
if (MemberPointerType *PT =
MemberPointerTypes.FindNodeOrInsertPos(ID, InsertPos))
return QualType(PT, 0);

NestedNameSpecifier *CanonicalQualifier = [&] {
if (!Cls)
return getCanonicalNestedNameSpecifier(Qualifier);
NestedNameSpecifier *R = NestedNameSpecifier::Create(
*this, /*Prefix=*/nullptr, /*Template=*/false,
Cls->getCanonicalDecl()->getTypeForDecl());
assert(R == getCanonicalNestedNameSpecifier(R));
return R;
}();
// If the pointee or class type isn't canonical, this won't be a canonical
// type either, so fill in the canonical type field.
QualType Canonical;
if (!T.isCanonical() || Qualifier != CanonicalQualifier) {
Canonical =
getMemberPointerType(getCanonicalType(T), CanonicalQualifier, Cls);
assert(!cast<MemberPointerType>(Canonical)->isSugared());
if (!T.isCanonical() || !Cls->isCanonicalUnqualified()) {
Canonical = getMemberPointerType(getCanonicalType(T),getCanonicalType(Cls));

// Get the new insert position for the node we care about.
[[maybe_unused]] MemberPointerType *NewIP =
MemberPointerTypes.FindNodeOrInsertPos(ID, InsertPos);
assert(!NewIP && "Shouldn't be in the map!");
MemberPointerType *NewIP =
MemberPointerTypes.FindNodeOrInsertPos(ID, InsertPos);
assert(!NewIP && "Shouldn't be in the map!"); (void)NewIP;
}
auto *New = new (*this, alignof(MemberPointerType))
MemberPointerType(T, Qualifier, Canonical);
MemberPointerType(T, Cls, Canonical);
Types.push_back(New);
MemberPointerTypes.InsertNode(New, InsertPos);
return QualType(New, 0);
Expand Down Expand Up @@ -6832,16 +6812,11 @@ bool ASTContext::UnwrapSimilarTypes(QualType &T1, QualType &T2,
return true;
}

if (const auto *T1MPType = T1->getAs<MemberPointerType>(),
*T2MPType = T2->getAs<MemberPointerType>();
T1MPType && T2MPType) {
if (auto *RD1 = T1MPType->getMostRecentCXXRecordDecl(),
*RD2 = T2MPType->getMostRecentCXXRecordDecl();
RD1 != RD2 && RD1->getCanonicalDecl() != RD2->getCanonicalDecl())
return false;
if (getCanonicalNestedNameSpecifier(T1MPType->getQualifier()) !=
getCanonicalNestedNameSpecifier(T2MPType->getQualifier()))
return false;
const auto *T1MPType = T1->getAs<MemberPointerType>();
const auto *T2MPType = T2->getAs<MemberPointerType>();
if (T1MPType && T2MPType &&
hasSameUnqualifiedType(QualType(T1MPType->getClass(), 0),
QualType(T2MPType->getClass(), 0))) {
T1 = T1MPType->getPointeeType();
T2 = T2MPType->getPointeeType();
return true;
Expand Down Expand Up @@ -13882,12 +13857,11 @@ static QualType getCommonNonSugarTypeNode(ASTContext &Ctx, const Type *X,
case Type::MemberPointer: {
const auto *PX = cast<MemberPointerType>(X),
*PY = cast<MemberPointerType>(Y);
assert(declaresSameEntity(PX->getMostRecentCXXRecordDecl(),
PY->getMostRecentCXXRecordDecl()));
return Ctx.getMemberPointerType(
getCommonPointeeType(Ctx, PX, PY),
getCommonQualifier(Ctx, PX, PY, /*IsSame=*/true),
PX->getMostRecentCXXRecordDecl());
Ctx.getCommonSugaredType(QualType(PX->getClass(), 0),
QualType(PY->getClass(), 0))
.getTypePtr());
}
case Type::LValueReference: {
const auto *PX = cast<LValueReferenceType>(X),
Expand Down
Loading
Loading