[clang] NFC: Unify implementations of CheckMemberPointerConversion #131966
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Originally split-off from #130537 |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesThis deduplicates the implementation of CheckMemberPointerConversion accross SemaCast and SemaOverload. Full diff: https://github.com/llvm/llvm-project/pull/131966.diff 5 Files Affected:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 846c5d09be2a3..5f13648c64b5b 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10041,15 +10041,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 fdfd3bab39db5..a1a10799837be 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
|
erichkeane
approved these changes
Mar 19, 2025
mizvekov
force-pushed
the
users/mizvekov/clang-memptrtype-getclass
branch
from
e44f256 to
ae35359
Compare
Base automatically changed from
users/mizvekov/clang-memptrtype-getclass
to
main
March 20, 2025 00:36
This deduplicates the implementation of CheckMemberPointerConversion accross SemaCast and SemaOverload.
mizvekov
force-pushed
the
users/mizvekov/clang-CheckMemberPointerConversion
branch
from
6a3abfc to
12fcc90
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/18014 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/14933 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This deduplicates the implementation of CheckMemberPointerConversion accross SemaCast and SemaOverload.