[clang] 6003c30 - [clang] NFC: Unify implementations of CheckMemberPointerConversion (#131966)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 19 18:48:53 PDT 2025
Author: Matheus Izvekov
Date: 2025-03-19T22:48:49-03:00
New Revision: 6003c3055a4630be31cc3d459cdbb88248a007b9
URL: https://github.com/llvm/llvm-project/commit/6003c3055a4630be31cc3d459cdbb88248a007b9
DIFF: https://github.com/llvm/llvm-project/commit/6003c3055a4630be31cc3d459cdbb88248a007b9.diff
LOG: [clang] NFC: Unify implementations of CheckMemberPointerConversion (#131966)
This deduplicates the implementation of CheckMemberPointerConversion
accross SemaCast and SemaOverload.
Added:
Modified:
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaAccess.cpp
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaOverload.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9a367f156d9ff..9724f0def743a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10057,15 +10057,24 @@ class Sema final : public SemaBase {
bool InOverloadResolution,
QualType &ConvertedType);
+ enum class MemberPointerConversionResult {
+ Success,
+ DifferentPointee,
+ NotDerived,
+ Ambiguous,
+ Virtual,
+ Inaccessible
+ };
+ enum class MemberPointerConversionDirection : bool { Downcast, Upcast };
/// CheckMemberPointerConversion - Check the member pointer conversion from
/// the expression From to the type ToType. This routine checks for ambiguous
/// or virtual or inaccessible base-to-derived member pointer conversions for
- /// which IsMemberPointerConversion has already returned true. It returns true
- /// and produces a diagnostic if there was an error, or returns false
- /// otherwise.
- bool CheckMemberPointerConversion(Expr *From, QualType ToType, CastKind &Kind,
- CXXCastPath &BasePath,
- bool IgnoreBaseAccess);
+ /// which IsMemberPointerConversion has already returned true. It produces a
+ // diagnostic if there was an error.
+ MemberPointerConversionResult CheckMemberPointerConversion(
+ QualType FromType, const MemberPointerType *ToPtrType, CastKind &Kind,
+ CXXCastPath &BasePath, SourceLocation CheckLoc, SourceRange OpRange,
+ bool IgnoreBaseAccess, MemberPointerConversionDirection Direction);
/// IsQualificationConversion - Determines whether the conversion from
/// an rvalue of type FromType to ToType is a qualification conversion
diff --git a/clang/lib/Sema/SemaAccess.cpp b/clang/lib/Sema/SemaAccess.cpp
index 6813786df3fc4..4ba46a957fa0e 100644
--- a/clang/lib/Sema/SemaAccess.cpp
+++ b/clang/lib/Sema/SemaAccess.cpp
@@ -1874,11 +1874,9 @@ Sema::AccessResult Sema::CheckAddressOfMemberAccess(Expr *OvlExpr,
}
Sema::AccessResult Sema::CheckBaseClassAccess(SourceLocation AccessLoc,
- QualType Base,
- QualType Derived,
+ QualType Base, QualType Derived,
const CXXBasePath &Path,
- unsigned DiagID,
- bool ForceCheck,
+ unsigned DiagID, bool ForceCheck,
bool ForceUnprivileged) {
if (!ForceCheck && !getLangOpts().AccessControl)
return AR_accessible;
@@ -1886,21 +1884,20 @@ Sema::AccessResult Sema::CheckBaseClassAccess(SourceLocation AccessLoc,
if (Path.Access == AS_public)
return AR_accessible;
- CXXRecordDecl *BaseD, *DerivedD;
- BaseD = cast<CXXRecordDecl>(Base->castAs<RecordType>()->getDecl());
- DerivedD = cast<CXXRecordDecl>(Derived->castAs<RecordType>()->getDecl());
-
- AccessTarget Entity(Context, AccessTarget::Base, BaseD, DerivedD,
- Path.Access);
+ AccessTarget Entity(Context, AccessTarget::Base, Base->getAsCXXRecordDecl(),
+ Derived->getAsCXXRecordDecl(), Path.Access);
if (DiagID)
Entity.setDiag(DiagID) << Derived << Base;
if (ForceUnprivileged) {
- switch (CheckEffectiveAccess(*this, EffectiveContext(),
- AccessLoc, Entity)) {
- case ::AR_accessible: return Sema::AR_accessible;
- case ::AR_inaccessible: return Sema::AR_inaccessible;
- case ::AR_dependent: return Sema::AR_dependent;
+ switch (
+ CheckEffectiveAccess(*this, EffectiveContext(), AccessLoc, Entity)) {
+ case ::AR_accessible:
+ return Sema::AR_accessible;
+ case ::AR_inaccessible:
+ return Sema::AR_inaccessible;
+ case ::AR_dependent:
+ return Sema::AR_dependent;
}
llvm_unreachable("unexpected result from CheckEffectiveAccess");
}
diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index 89e8082ee80e7..718f6bec34910 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -1794,72 +1794,25 @@ TryStaticMemberPointerUpcast(Sema &Self, ExprResult &SrcExpr, QualType SrcType,
}
}
- const MemberPointerType *SrcMemPtr = SrcType->getAs<MemberPointerType>();
- if (!SrcMemPtr) {
- msg = diag::err_bad_static_cast_member_pointer_nonmp;
- return TC_NotApplicable;
- }
-
- // Lock down the inheritance model right now in MS ABI, whether or not the
- // pointee types are the same.
- if (Self.Context.getTargetInfo().getCXXABI().isMicrosoft()) {
- (void)Self.isCompleteType(OpRange.getBegin(), SrcType);
- (void)Self.isCompleteType(OpRange.getBegin(), DestType);
- }
-
- // T == T, modulo cv
- if (!Self.Context.hasSameUnqualifiedType(SrcMemPtr->getPointeeType(),
- DestMemPtr->getPointeeType()))
- return TC_NotApplicable;
-
- // B base of D
- QualType SrcClass(SrcMemPtr->getClass(), 0);
- QualType DestClass(DestMemPtr->getClass(), 0);
- CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/true,
- /*DetectVirtual=*/true);
- if (!Self.IsDerivedFrom(OpRange.getBegin(), SrcClass, DestClass, Paths))
+ switch (Self.CheckMemberPointerConversion(
+ SrcType, DestMemPtr, Kind, BasePath, OpRange.getBegin(), OpRange, CStyle,
+ Sema::MemberPointerConversionDirection::Upcast)) {
+ case Sema::MemberPointerConversionResult::Success:
+ if (Kind == CK_NullToMemberPointer) {
+ msg = diag::err_bad_static_cast_member_pointer_nonmp;
+ return TC_NotApplicable;
+ }
+ break;
+ case Sema::MemberPointerConversionResult::DifferentPointee:
+ case Sema::MemberPointerConversionResult::NotDerived:
return TC_NotApplicable;
-
- // B is a base of D. But is it an allowed base? If not, it's a hard error.
- if (Paths.isAmbiguous(Self.Context.getCanonicalType(DestClass))) {
- Paths.clear();
- Paths.setRecordingPaths(true);
- bool StillOkay =
- Self.IsDerivedFrom(OpRange.getBegin(), SrcClass, DestClass, Paths);
- assert(StillOkay);
- (void)StillOkay;
- std::string PathDisplayStr = Self.getAmbiguousPathsDisplayString(Paths);
- Self.Diag(OpRange.getBegin(), diag::err_ambiguous_memptr_conv)
- << 1 << SrcClass << DestClass << PathDisplayStr << OpRange;
- msg = 0;
- return TC_Failed;
- }
-
- if (const RecordType *VBase = Paths.getDetectedVirtual()) {
- Self.Diag(OpRange.getBegin(), diag::err_memptr_conv_via_virtual)
- << SrcClass << DestClass << QualType(VBase, 0) << OpRange;
+ case Sema::MemberPointerConversionResult::Ambiguous:
+ case Sema::MemberPointerConversionResult::Virtual:
+ case Sema::MemberPointerConversionResult::Inaccessible:
msg = 0;
return TC_Failed;
}
- if (!CStyle) {
- switch (Self.CheckBaseClassAccess(OpRange.getBegin(),
- DestClass, SrcClass,
- Paths.front(),
- diag::err_upcast_to_inaccessible_base)) {
- case Sema::AR_accessible:
- case Sema::AR_delayed:
- case Sema::AR_dependent:
- // Optimistically assume that the delayed and dependent cases
- // will work out.
- break;
-
- case Sema::AR_inaccessible:
- msg = 0;
- return TC_Failed;
- }
- }
-
if (WasOverloadedFunction) {
// Resolve the address of the overloaded function again, this time
// allowing complaints if something goes wrong.
@@ -1878,9 +1831,6 @@ TryStaticMemberPointerUpcast(Sema &Self, ExprResult &SrcExpr, QualType SrcType,
return TC_Failed;
}
}
-
- Self.BuildBasePathArray(Paths, BasePath);
- Kind = CK_DerivedToBaseMemberPointer;
return TC_Success;
}
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 063f9b57fb264..f085facf7ff55 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -4666,18 +4666,29 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
case ICK_Pointer_Member: {
CastKind Kind;
CXXCastPath BasePath;
- if (CheckMemberPointerConversion(From, ToType, Kind, BasePath, CStyle))
+ switch (CheckMemberPointerConversion(
+ From->getType(), ToType->castAs<MemberPointerType>(), Kind, BasePath,
+ From->getExprLoc(), From->getSourceRange(), CStyle,
+ MemberPointerConversionDirection::Downcast)) {
+ case MemberPointerConversionResult::Success:
+ assert(Kind != CK_NullToMemberPointer ||
+ From->isNullPointerConstant(Context,
+ Expr::NPC_ValueDependentIsNull) &&
+ "Expr must be null pointer constant!");
+ break;
+ case MemberPointerConversionResult::Inaccessible:
+ break;
+ case MemberPointerConversionResult::DifferentPointee:
+ llvm_unreachable("unexpected result");
+ case MemberPointerConversionResult::NotDerived:
+ llvm_unreachable("Should not have been called if derivation isn't OK.");
+ case MemberPointerConversionResult::Ambiguous:
+ case MemberPointerConversionResult::Virtual:
return ExprError();
+ }
if (CheckExceptionSpecCompatibility(From, ToType))
return ExprError();
- // We may not have been able to figure out what this member pointer resolved
- // to up until this exact point. Attempt to lock-in it's inheritance model.
- if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
- (void)isCompleteType(From->getExprLoc(), From->getType());
- (void)isCompleteType(From->getExprLoc(), ToType);
- }
-
From =
ImpCastExprToType(From, ToType, Kind, VK_PRValue, &BasePath, CCK).get();
break;
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index cfb263fb3933f..161c0daa2f510 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -3548,64 +3548,79 @@ bool Sema::IsMemberPointerConversion(Expr *From, QualType FromType,
return false;
}
-bool Sema::CheckMemberPointerConversion(Expr *From, QualType ToType,
- CastKind &Kind,
- CXXCastPath &BasePath,
- bool IgnoreBaseAccess) {
- QualType FromType = From->getType();
+Sema::MemberPointerConversionResult Sema::CheckMemberPointerConversion(
+ QualType FromType, const MemberPointerType *ToPtrType, CastKind &Kind,
+ CXXCastPath &BasePath, SourceLocation CheckLoc, SourceRange OpRange,
+ bool IgnoreBaseAccess, MemberPointerConversionDirection Direction) {
const MemberPointerType *FromPtrType = FromType->getAs<MemberPointerType>();
if (!FromPtrType) {
// This must be a null pointer to member pointer conversion
- assert(From->isNullPointerConstant(Context,
- Expr::NPC_ValueDependentIsNull) &&
- "Expr must be null pointer constant!");
Kind = CK_NullToMemberPointer;
- return false;
+ return MemberPointerConversionResult::Success;
+ }
+
+ // Lock down the inheritance model right now in MS ABI, whether or not the
+ // pointee types are the same.
+ if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+ (void)isCompleteType(CheckLoc, FromType);
+ (void)isCompleteType(CheckLoc, QualType(ToPtrType, 0));
}
- const MemberPointerType *ToPtrType = ToType->getAs<MemberPointerType>();
- assert(ToPtrType && "No member pointer cast has a target type "
- "that is not a member pointer.");
+ // T == T, modulo cv
+ if (Direction == MemberPointerConversionDirection::Upcast &&
+ !Context.hasSameUnqualifiedType(FromPtrType->getPointeeType(),
+ ToPtrType->getPointeeType()))
+ return MemberPointerConversionResult::DifferentPointee;
- QualType FromClass = QualType(FromPtrType->getClass(), 0);
- QualType ToClass = QualType(ToPtrType->getClass(), 0);
+ QualType FromClass = QualType(FromPtrType->getClass(), 0),
+ ToClass = QualType(ToPtrType->getClass(), 0);
- // FIXME: What about dependent types?
- assert(FromClass->isRecordType() && "Pointer into non-class.");
- assert(ToClass->isRecordType() && "Pointer into non-class.");
+ QualType Base = FromClass, Derived = ToClass;
+ if (Direction == MemberPointerConversionDirection::Upcast)
+ std::swap(Base, Derived);
CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/true,
/*DetectVirtual=*/true);
- bool DerivationOkay =
- IsDerivedFrom(From->getBeginLoc(), ToClass, FromClass, Paths);
- assert(DerivationOkay &&
- "Should not have been called if derivation isn't OK.");
- (void)DerivationOkay;
-
- if (Paths.isAmbiguous(Context.getCanonicalType(FromClass).
- getUnqualifiedType())) {
- std::string PathDisplayStr = getAmbiguousPathsDisplayString(Paths);
- Diag(From->getExprLoc(), diag::err_ambiguous_memptr_conv)
- << 0 << FromClass << ToClass << PathDisplayStr << From->getSourceRange();
- return true;
+ if (!IsDerivedFrom(OpRange.getBegin(), Derived, Base, Paths))
+ return MemberPointerConversionResult::NotDerived;
+
+ if (Paths.isAmbiguous(Base->getCanonicalTypeUnqualified())) {
+ Diag(CheckLoc, diag::err_ambiguous_memptr_conv)
+ << int(Direction) << FromClass << ToClass
+ << getAmbiguousPathsDisplayString(Paths) << OpRange;
+ return MemberPointerConversionResult::Ambiguous;
}
if (const RecordType *VBase = Paths.getDetectedVirtual()) {
- Diag(From->getExprLoc(), diag::err_memptr_conv_via_virtual)
- << FromClass << ToClass << QualType(VBase, 0)
- << From->getSourceRange();
- return true;
+ Diag(CheckLoc, diag::err_memptr_conv_via_virtual)
+ << FromClass << ToClass << QualType(VBase, 0) << OpRange;
+ return MemberPointerConversionResult::Virtual;
}
- if (!IgnoreBaseAccess)
- CheckBaseClassAccess(From->getExprLoc(), FromClass, ToClass,
- Paths.front(),
- diag::err_downcast_from_inaccessible_base);
-
// Must be a base to derived member conversion.
BuildBasePathArray(Paths, BasePath);
- Kind = CK_BaseToDerivedMemberPointer;
- return false;
+ Kind = Direction == MemberPointerConversionDirection::Upcast
+ ? CK_DerivedToBaseMemberPointer
+ : CK_BaseToDerivedMemberPointer;
+
+ if (!IgnoreBaseAccess)
+ switch (CheckBaseClassAccess(
+ CheckLoc, Base, Derived, Paths.front(),
+ Direction == MemberPointerConversionDirection::Upcast
+ ? diag::err_upcast_to_inaccessible_base
+ : diag::err_downcast_from_inaccessible_base)) {
+ case Sema::AR_accessible:
+ case Sema::AR_delayed:
+ case Sema::AR_dependent:
+ // Optimistically assume that the delayed and dependent cases
+ // will work out.
+ break;
+
+ case Sema::AR_inaccessible:
+ return MemberPointerConversionResult::Inaccessible;
+ }
+
+ return MemberPointerConversionResult::Success;
}
/// Determine whether the lifetime conversion between the two given
More information about the cfe-commits
mailing list