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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
