Skip to content

Reland: [clang] NFC: Clear some uses of MemberPointerType::getClass #132317

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 2 commits into from
Mar 21, 2025

Conversation

mizvekov
Copy link
Contributor

@mizvekov mizvekov commented Mar 21, 2025

Relands Original PR: #131965
Addresses #131965 (comment)

  • Fixes isIncompleteType for injected classes

This clears up some uses of getClass on MemberPointerType when equivalent uses of getMostRecentCXXRecordDecl would be just as simple or simpler.

This is split-off from a larger patch which removes getClass, in order to facilitate review.

Original PR: #131965
Addresses #131965 (comment)
* Fixes isIncompleteType for injected classes

This clears up some uses of getClass on MemberPointerType when
equivalent uses of getMostRecentCXXRecordDecl would be just as
simple or simpler.

This is split-off from a larger patch which removes getClass,
in order to facilitate review.
@mizvekov mizvekov requested a review from erichkeane March 21, 2025 01:23
@mizvekov mizvekov self-assigned this Mar 21, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang-tools-extra clang-tidy clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. debuginfo labels Mar 21, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 21, 2025

@llvm/pr-subscribers-debuginfo
@llvm/pr-subscribers-clang-tools-extra
@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

Relands Original PR: #131965
Addresses #131965 (comment)

  • Fixes isIncompleteType for injected classes

This clears up some uses of getClass on MemberPointerType when
equivalent uses of getMostRecentCXXRecordDecl would be just as
simple or simpler.

This is split-off from a larger patch which removes getClass,
in order to facilitate review.


Patch is 28.98 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/132317.diff

20 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp (+1-4)
  • (modified) clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp (+2-2)
  • (modified) clang/include/clang/AST/CanonicalType.h (+2)
  • (modified) clang/include/clang/Sema/Sema.h (+4-3)
  • (modified) clang/lib/AST/ByteCode/Compiler.cpp (+6-4)
  • (modified) clang/lib/AST/ExprConstant.cpp (+3-2)
  • (modified) clang/lib/AST/MicrosoftMangle.cpp (+5-5)
  • (modified) clang/lib/AST/Type.cpp (+11-9)
  • (modified) clang/lib/CodeGen/CGCXXABI.cpp (+2-3)
  • (modified) clang/lib/CodeGen/CGClass.cpp (+3-4)
  • (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+2-1)
  • (modified) clang/lib/CodeGen/CGExprCXX.cpp (+4-4)
  • (modified) clang/lib/CodeGen/CodeGenTypes.cpp (+1-1)
  • (modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+6-5)
  • (modified) clang/lib/Sema/SemaDeclCXX.cpp (+23-25)
  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+18-14)
  • (modified) clang/lib/Sema/SemaLookup.cpp (+2-5)
  • (modified) clang/lib/Sema/SemaOverload.cpp (+17-20)
  • (modified) clang/lib/Sema/SemaStmt.cpp (+2-1)
  • (added) clang/test/SemaCXX/ms-member-pointer.cpp (+9)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
index 9d1d92b989bf1..a8a9e6bdcdff8 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp
@@ -70,10 +70,7 @@ void ComparePointerToMemberVirtualFunctionCheck::check(
   // compare with variable which type is pointer to member function.
   llvm::SmallVector<SourceLocation, 12U> SameSignatureVirtualMethods{};
   const auto *MPT = cast<MemberPointerType>(DRE->getType().getCanonicalType());
-  const Type *T = MPT->getClass();
-  if (T == nullptr)
-    return;
-  const CXXRecordDecl *RD = T->getAsCXXRecordDecl();
+  const CXXRecordDecl *RD = MPT->getMostRecentCXXRecordDecl();
   if (RD == nullptr)
     return;
 
diff --git a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
index 0fea7946a59f9..b66cc8512fad6 100644
--- a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -219,8 +219,8 @@ bool isQualificationConvertiblePointer(QualType From, QualType To,
 
     if (P1->isMemberPointerType())
       return P2->isMemberPointerType() &&
-             P1->getAs<MemberPointerType>()->getClass() ==
-                 P2->getAs<MemberPointerType>()->getClass();
+             P1->getAs<MemberPointerType>()->getMostRecentCXXRecordDecl() ==
+                 P2->getAs<MemberPointerType>()->getMostRecentCXXRecordDecl();
 
     if (P1->isConstantArrayType())
       return P2->isConstantArrayType() &&
diff --git a/clang/include/clang/AST/CanonicalType.h b/clang/include/clang/AST/CanonicalType.h
index 6699284d215bd..50d1ba1b8f63f 100644
--- a/clang/include/clang/AST/CanonicalType.h
+++ b/clang/include/clang/AST/CanonicalType.h
@@ -454,6 +454,8 @@ struct CanProxyAdaptor<MemberPointerType>
   : public CanProxyBase<MemberPointerType> {
   LLVM_CLANG_CANPROXY_TYPE_ACCESSOR(getPointeeType)
   LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(const Type *, getClass)
+  LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(const CXXRecordDecl *,
+                                      getMostRecentCXXRecordDecl)
 };
 
 // CanProxyAdaptors for arrays are intentionally unimplemented because
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 0befe615c4ee1..9724f0def743a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5769,10 +5769,11 @@ class Sema final : public SemaBase {
 
   /// Determine whether the type \p Derived is a C++ class that is
   /// derived from the type \p Base.
+  bool IsDerivedFrom(SourceLocation Loc, CXXRecordDecl *Derived,
+                     CXXRecordDecl *Base, CXXBasePaths &Paths);
+  bool IsDerivedFrom(SourceLocation Loc, CXXRecordDecl *Derived,
+                     CXXRecordDecl *Base);
   bool IsDerivedFrom(SourceLocation Loc, QualType Derived, QualType Base);
-
-  /// Determine whether the type \p Derived is a C++ class that is
-  /// derived from the type \p Base.
   bool IsDerivedFrom(SourceLocation Loc, QualType Derived, QualType Base,
                      CXXBasePaths &Paths);
 
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 91640c4eb5cf9..9dc02b25f8495 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -238,8 +238,9 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
     const auto *FromMP = SubExpr->getType()->getAs<MemberPointerType>();
     const auto *ToMP = CE->getType()->getAs<MemberPointerType>();
 
-    unsigned DerivedOffset = collectBaseOffset(QualType(ToMP->getClass(), 0),
-                                               QualType(FromMP->getClass(), 0));
+    unsigned DerivedOffset =
+        Ctx.collectBaseOffset(ToMP->getMostRecentCXXRecordDecl(),
+                              FromMP->getMostRecentCXXRecordDecl());
 
     if (!this->delegate(SubExpr))
       return false;
@@ -253,8 +254,9 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
     const auto *FromMP = SubExpr->getType()->getAs<MemberPointerType>();
     const auto *ToMP = CE->getType()->getAs<MemberPointerType>();
 
-    unsigned DerivedOffset = collectBaseOffset(QualType(FromMP->getClass(), 0),
-                                               QualType(ToMP->getClass(), 0));
+    unsigned DerivedOffset =
+        Ctx.collectBaseOffset(FromMP->getMostRecentCXXRecordDecl(),
+                              ToMP->getMostRecentCXXRecordDecl());
 
     if (!this->delegate(SubExpr))
       return false;
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 0165a0a3b0df3..92a28897cf3ee 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -10551,8 +10551,9 @@ bool MemberPointerExprEvaluator::VisitCastExpr(const CastExpr *E) {
       if (!Result.castToDerived(Derived))
         return Error(E);
     }
-    const Type *FinalTy = E->getType()->castAs<MemberPointerType>()->getClass();
-    if (!Result.castToDerived(FinalTy->getAsCXXRecordDecl()))
+    if (!Result.castToDerived(E->getType()
+                                  ->castAs<MemberPointerType>()
+                                  ->getMostRecentCXXRecordDecl()))
       return Error(E);
     return true;
   }
diff --git a/clang/lib/AST/MicrosoftMangle.cpp b/clang/lib/AST/MicrosoftMangle.cpp
index fe34251688a98..15de407e122d8 100644
--- a/clang/lib/AST/MicrosoftMangle.cpp
+++ b/clang/lib/AST/MicrosoftMangle.cpp
@@ -695,7 +695,7 @@ void MicrosoftCXXNameMangler::mangleVariableEncoding(const VarDecl *VD) {
       mangleQualifiers(MPT->getPointeeType().getQualifiers(), true);
       // Member pointers are suffixed with a back reference to the member
       // pointer's class name.
-      mangleName(MPT->getClass()->getAsCXXRecordDecl());
+      mangleName(MPT->getMostRecentCXXRecordDecl());
     } else
       mangleQualifiers(Ty->getPointeeType().getQualifiers(), false);
   } else if (const ArrayType *AT = getASTContext().getAsArrayType(Ty)) {
@@ -3331,11 +3331,11 @@ void MicrosoftCXXNameMangler::mangleType(const MemberPointerType *T,
   manglePointerExtQualifiers(Quals, PointeeType);
   if (const FunctionProtoType *FPT = PointeeType->getAs<FunctionProtoType>()) {
     Out << '8';
-    mangleName(T->getClass()->castAs<RecordType>()->getDecl());
+    mangleName(T->getMostRecentCXXRecordDecl());
     mangleFunctionType(FPT, nullptr, true);
   } else {
     mangleQualifiers(PointeeType.getQualifiers(), true);
-    mangleName(T->getClass()->castAs<RecordType>()->getDecl());
+    mangleName(T->getMostRecentCXXRecordDecl());
     mangleType(PointeeType, Range, QMM_Drop);
   }
 }
@@ -4309,11 +4309,11 @@ void MicrosoftCXXNameMangler::mangleAutoReturnType(const MemberPointerType *T,
   manglePointerExtQualifiers(Quals, PointeeType);
   if (const FunctionProtoType *FPT = PointeeType->getAs<FunctionProtoType>()) {
     Out << '8';
-    mangleName(T->getClass()->castAs<RecordType>()->getDecl());
+    mangleName(T->getMostRecentCXXRecordDecl());
     mangleFunctionType(FPT, nullptr, true);
   } else {
     mangleQualifiers(PointeeType.getQualifiers(), true);
-    mangleName(T->getClass()->castAs<RecordType>()->getDecl());
+    mangleName(T->getMostRecentCXXRecordDecl());
     mangleAutoReturnType(PointeeType, QMM_Drop);
   }
 }
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 72161c06a88d4..9c5fd2289887c 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2446,19 +2446,17 @@ bool Type::isIncompleteType(NamedDecl **Def) const {
     // Member pointers in the MS ABI have special behavior in
     // RequireCompleteType: they attach a MSInheritanceAttr to the CXXRecordDecl
     // to indicate which inheritance model to use.
-    auto *MPTy = cast<MemberPointerType>(CanonicalType);
-    const Type *ClassTy = MPTy->getClass();
+    // The inheritance attribute might only be present on the most recent
+    // CXXRecordDecl.
+    const CXXRecordDecl *RD =
+        cast<MemberPointerType>(CanonicalType)->getMostRecentCXXRecordDecl();
     // Member pointers with dependent class types don't get special treatment.
-    if (ClassTy->isDependentType())
+    if (!RD || RD->isDependentType())
       return false;
-    const CXXRecordDecl *RD = ClassTy->getAsCXXRecordDecl();
     ASTContext &Context = RD->getASTContext();
     // Member pointers not in the MS ABI don't get special treatment.
     if (!Context.getTargetInfo().getCXXABI().isMicrosoft())
       return false;
-    // The inheritance attribute might only be present on the most recent
-    // CXXRecordDecl, use that one.
-    RD = RD->getMostRecentNonInjectedDecl();
     // Nothing interesting to do if the inheritance attribute is already set.
     if (RD->hasAttr<MSInheritanceAttr>())
       return false;
@@ -4713,7 +4711,8 @@ LinkageInfo LinkageComputer::computeTypeLinkageInfo(const Type *T) {
     return computeTypeLinkageInfo(cast<ReferenceType>(T)->getPointeeType());
   case Type::MemberPointer: {
     const auto *MPT = cast<MemberPointerType>(T);
-    LinkageInfo LV = computeTypeLinkageInfo(MPT->getClass());
+    LinkageInfo LV =
+        getDeclLinkageAndVisibility(MPT->getMostRecentCXXRecordDecl());
     LV.merge(computeTypeLinkageInfo(MPT->getPointeeType()));
     return LV;
   }
@@ -5179,7 +5178,10 @@ QualType::DestructionKind QualType::isDestructedTypeImpl(QualType type) {
 }
 
 CXXRecordDecl *MemberPointerType::getMostRecentCXXRecordDecl() const {
-  return getClass()->getAsCXXRecordDecl()->getMostRecentNonInjectedDecl();
+  auto *RD = getClass()->getAsCXXRecordDecl();
+  if (!RD)
+    return nullptr;
+  return RD->getMostRecentNonInjectedDecl();
 }
 
 void clang::FixedPointValueToString(SmallVectorImpl<char> &Str,
diff --git a/clang/lib/CodeGen/CGCXXABI.cpp b/clang/lib/CodeGen/CGCXXABI.cpp
index 7c6dfc3e59d8c..2777c78d6459d 100644
--- a/clang/lib/CodeGen/CGCXXABI.cpp
+++ b/clang/lib/CodeGen/CGCXXABI.cpp
@@ -50,8 +50,7 @@ CGCallee CGCXXABI::EmitLoadOfMemberFunctionPointer(
     llvm::Value *MemPtr, const MemberPointerType *MPT) {
   ErrorUnsupportedABI(CGF, "calls through member pointers");
 
-  const auto *RD =
-      cast<CXXRecordDecl>(MPT->getClass()->castAs<RecordType>()->getDecl());
+  const auto *RD = MPT->getMostRecentCXXRecordDecl();
   ThisPtrForCall =
       CGF.getAsNaturalPointerTo(This, CGF.getContext().getRecordType(RD));
   const FunctionProtoType *FPT =
@@ -294,7 +293,7 @@ llvm::Constant *CGCXXABI::getMemberPointerAdjustment(const CastExpr *E) {
     derivedType = E->getType();
 
   const CXXRecordDecl *derivedClass =
-    derivedType->castAs<MemberPointerType>()->getClass()->getAsCXXRecordDecl();
+      derivedType->castAs<MemberPointerType>()->getMostRecentCXXRecordDecl();
 
   return CGM.GetNonVirtualBaseClassOffset(derivedClass,
                                           E->path_begin(),
diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index fa69caa41936c..98c93b5bb4883 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -161,10 +161,9 @@ CodeGenFunction::EmitCXXMemberDataPointerAddress(const Expr *E, Address base,
   QualType memberType = memberPtrType->getPointeeType();
   CharUnits memberAlign =
       CGM.getNaturalTypeAlignment(memberType, BaseInfo, TBAAInfo);
-  memberAlign =
-    CGM.getDynamicOffsetAlignment(base.getAlignment(),
-                            memberPtrType->getClass()->getAsCXXRecordDecl(),
-                                  memberAlign);
+  memberAlign = CGM.getDynamicOffsetAlignment(
+      base.getAlignment(), memberPtrType->getMostRecentCXXRecordDecl(),
+      memberAlign);
   return Address(ptr, ConvertTypeForMem(memberPtrType->getPointeeType()),
                  memberAlign);
 }
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index c462b02676813..dcccbc0835d95 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3490,7 +3490,8 @@ llvm::DIType *CGDebugInfo::CreateType(const MemberPointerType *Ty,
     }
   }
 
-  llvm::DIType *ClassType = getOrCreateType(QualType(Ty->getClass(), 0), U);
+  llvm::DIType *ClassType = getOrCreateType(
+      QualType(Ty->getMostRecentCXXRecordDecl()->getTypeForDecl(), 0), U);
   if (Ty->isMemberDataPointerType())
     return DBuilder.createMemberPointerType(
         getOrCreateType(Ty->getPointeeType(), U), ClassType, Size, /*Align=*/0,
diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index f71c18a8041b1..5d96959065dd9 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -453,8 +453,7 @@ CodeGenFunction::EmitCXXMemberPointerCallExpr(const CXXMemberCallExpr *E,
 
   const auto *MPT = MemFnExpr->getType()->castAs<MemberPointerType>();
   const auto *FPT = MPT->getPointeeType()->castAs<FunctionProtoType>();
-  const auto *RD =
-      cast<CXXRecordDecl>(MPT->getClass()->castAs<RecordType>()->getDecl());
+  const auto *RD = MPT->getMostRecentCXXRecordDecl();
 
   // Emit the 'this' pointer.
   Address This = Address::invalid();
@@ -463,8 +462,9 @@ CodeGenFunction::EmitCXXMemberPointerCallExpr(const CXXMemberCallExpr *E,
   else
     This = EmitLValue(BaseExpr, KnownNonNull).getAddress();
 
-  EmitTypeCheck(TCK_MemberCall, E->getExprLoc(), This.emitRawPointer(*this),
-                QualType(MPT->getClass(), 0));
+  EmitTypeCheck(
+      TCK_MemberCall, E->getExprLoc(), This.emitRawPointer(*this),
+      QualType(MPT->getMostRecentCXXRecordDecl()->getTypeForDecl(), 0));
 
   // Get the member function pointer.
   llvm::Value *MemFnPtr = EmitScalarExpr(MemFnExpr);
diff --git a/clang/lib/CodeGen/CodeGenTypes.cpp b/clang/lib/CodeGen/CodeGenTypes.cpp
index dfbd444a850a5..11cf5758b6d3a 100644
--- a/clang/lib/CodeGen/CodeGenTypes.cpp
+++ b/clang/lib/CodeGen/CodeGenTypes.cpp
@@ -728,7 +728,7 @@ llvm::Type *CodeGenTypes::ConvertType(QualType T) {
   case Type::MemberPointer: {
     auto *MPTy = cast<MemberPointerType>(Ty);
     if (!getCXXABI().isMemberPointerConvertible(MPTy)) {
-      auto *C = MPTy->getClass();
+      auto *C = MPTy->getMostRecentCXXRecordDecl()->getTypeForDecl();
       auto Insertion = RecordsWithOpaqueMemberPointers.insert({C, nullptr});
       if (Insertion.second)
         Insertion.first->second = llvm::StructType::create(getLLVMContext());
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index 7e26a0da3d7d2..a5633f6349ffa 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -628,8 +628,7 @@ CGCallee ItaniumCXXABI::EmitLoadOfMemberFunctionPointer(
 
   const FunctionProtoType *FPT =
       MPT->getPointeeType()->castAs<FunctionProtoType>();
-  auto *RD =
-      cast<CXXRecordDecl>(MPT->getClass()->castAs<RecordType>()->getDecl());
+  auto *RD = MPT->getMostRecentCXXRecordDecl();
 
   llvm::Constant *ptrdiff_1 = llvm::ConstantInt::get(CGM.PtrDiffTy, 1);
 
@@ -798,7 +797,7 @@ CGCallee ItaniumCXXABI::EmitLoadOfMemberFunctionPointer(
 
   // Check the function pointer if CFI on member function pointers is enabled.
   if (ShouldEmitCFICheck) {
-    CXXRecordDecl *RD = MPT->getClass()->getAsCXXRecordDecl();
+    CXXRecordDecl *RD = MPT->getMostRecentCXXRecordDecl();
     if (RD->hasDefinition()) {
       CodeGenFunction::SanitizerScope SanScope(&CGF);
 
@@ -3799,7 +3798,8 @@ static bool ContainsIncompleteClassType(QualType Ty) {
   if (const MemberPointerType *MemberPointerTy =
       dyn_cast<MemberPointerType>(Ty)) {
     // Check if the class type is incomplete.
-    const RecordType *ClassType = cast<RecordType>(MemberPointerTy->getClass());
+    const auto *ClassType = cast<RecordType>(
+        MemberPointerTy->getMostRecentCXXRecordDecl()->getTypeForDecl());
     if (IsIncompleteClassType(ClassType))
       return true;
 
@@ -4538,7 +4538,8 @@ ItaniumRTTIBuilder::BuildPointerToMemberTypeInfo(const MemberPointerType *Ty) {
   //   attributes of the type pointed to.
   unsigned Flags = extractPBaseFlags(CGM.getContext(), PointeeTy);
 
-  const RecordType *ClassType = cast<RecordType>(Ty->getClass());
+  const auto *ClassType =
+      cast<RecordType>(Ty->getMostRecentCXXRecordDecl()->getTypeForDecl());
   if (IsIncompleteClassType(ClassType))
     Flags |= PTI_ContainingClassIncomplete;
 
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index a1551e8027cd3..7b2e0df8cb55d 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -3069,48 +3069,46 @@ void Sema::ActOnBaseSpecifiers(Decl *ClassDecl,
   AttachBaseSpecifiers(cast<CXXRecordDecl>(ClassDecl), Bases);
 }
 
-bool Sema::IsDerivedFrom(SourceLocation Loc, QualType Derived, QualType Base) {
+bool Sema::IsDerivedFrom(SourceLocation Loc, CXXRecordDecl *Derived,
+                         CXXRecordDecl *Base, CXXBasePaths &Paths) {
   if (!getLangOpts().CPlusPlus)
     return false;
 
-  CXXRecordDecl *DerivedRD = Derived->getAsCXXRecordDecl();
-  if (!DerivedRD)
-    return false;
-
-  CXXRecordDecl *BaseRD = Base->getAsCXXRecordDecl();
-  if (!BaseRD)
+  if (!Base || !Derived)
     return false;
 
   // If either the base or the derived type is invalid, don't try to
   // check whether one is derived from the other.
-  if (BaseRD->isInvalidDecl() || DerivedRD->isInvalidDecl())
+  if (Base->isInvalidDecl() || Derived->isInvalidDecl())
     return false;
 
   // FIXME: In a modules build, do we need the entire path to be visible for us
   // to be able to use the inheritance relationship?
-  if (!isCompleteType(Loc, Derived) && !DerivedRD->isBeingDefined())
+  if (!isCompleteType(Loc, Context.getTypeDeclType(Derived)) &&
+      !Derived->isBeingDefined())
     return false;
 
-  return DerivedRD->isDerivedFrom(BaseRD);
+  return Derived->isDerivedFrom(Base, Paths);
 }
 
-bool Sema::IsDerivedFrom(SourceLocation Loc, QualType Derived, QualType Base,
-                         CXXBasePaths &Paths) {
-  if (!getLangOpts().CPlusPlus)
-    return false;
-
-  CXXRecordDecl *DerivedRD = Derived->getAsCXXRecordDecl();
-  if (!DerivedRD)
-    return false;
-
-  CXXRecordDecl *BaseRD = Base->getAsCXXRecordDecl();
-  if (!BaseRD)
-    return false;
+bool Sema::IsDerivedFrom(SourceLocation Loc, CXXRecordDecl *Derived,
+                         CXXRecordDecl *Base) {
+  CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/false,
+                     /*DetectVirtual=*/false);
+  return IsDerivedFrom(Loc, Derived, Base, Paths);
+}
 
-  if (!isCompleteType(Loc, Derived) && !DerivedRD->isBeingDefined())
-    return false;
+bool Sema::IsDerivedFrom(SourceLocation Loc, QualType Derived, QualType Base) {
+  CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/false,
+                     /*DetectVirtual=*/false);
+  return IsDerivedFrom(Loc, Derived->getAsCXXRecordDecl(),
+                       Base->getAsCXXRecordDecl(), Paths);
+}
 
-  return DerivedRD->isDerivedFrom(BaseRD, Paths);
+bool Sema::IsDerivedFrom(SourceLocation Loc, QualType Derived, QualType Base,
+                         CXXBasePaths &Paths) {
+  return IsDerivedFrom(Loc, Derived->getAsCXXRecordDecl(),
+                       Base->getAsCXXRecordDecl(), Paths);
 }
 
 static void BuildBasePathArray(const CXXBasePath &Path,
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index cbea98b8884e9..8f204b949cb2c 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -6529,7 +6529,7 @@ QualType Sema::CheckPointerToMemberOperands(ExprResult &LHS, ExprResult &RHS,
     return QualType();
   }
 
-  QualType Class(MemPtr->getClass(), 0);
+  CXXRecordDecl *RHSClass = MemPtr->getMostRecentCXXRecordDecl();
 
   // Note: C++ [expr.mptr.oper]p2-3 says that the class type into which the
   // member pointer points must be completely-defined. However, there is no
@@ -6552,15 +6552,16 @@ QualType Sema::CheckPointerToMemberOperands(ExprResult &LHS, ExprResult &RHS,
       return QualType();
     }
   }
+  CXXRecordDecl *LHSClass = LHSType->getAsCXXRecordDecl();
 
-  if (!Context.hasSameUnqualifiedType(Class, LHSType)) {
+  if (!declaresSameEntity(LHSClass, RHSClass)) {
     // If we want to check the hierarchy, we need a complete type.
     if (RequireCompleteType(Loc, LH...
[truncated]

@mizvekov
Copy link
Contributor Author

I'm going to go ahead and merge it, since this is such a small trivial and obviously correct fix for a relanding.

Otherwise, this blocks relanding a much bigger patch, which is a rebase burden.

We can always make amendments post commit if anyone feels like it.

@mizvekov mizvekov merged commit 1416566 into main Mar 21, 2025
18 checks passed
@mizvekov mizvekov deleted the users/mizvekov/clang-memptrtype-getclass branch March 21, 2025 13:54
@glandium
Copy link
Contributor

This causes crashes:

1.	/builds/worker/checkouts/gecko/dom/media/webrtc/MediaEngineWebRTC.cpp:48:44: current parser token ')'
2.	/builds/worker/checkouts/gecko/dom/media/webrtc/MediaEngineWebRTC.cpp:31:1: parsing namespace 'mozilla'
3.	/builds/worker/checkouts/gecko/dom/media/webrtc/MediaEngineWebRTC.cpp:42:40: parsing function body 'mozilla::MediaEngineWebRTC::MediaEngineWebRTC'
4.	/builds/worker/checkouts/gecko/dom/media/webrtc/MediaEngineWebRTC.cpp:42:40: in compound statement ('{}')
 #0 0x00007f1beb652d08 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/tmp/llvm/obj-broken/bin/../lib/libLLVM.so.21.0git+0xe96d08)
 #1 0x00007f1beb65075e llvm::sys::RunSignalHandlers() (/tmp/llvm/obj-broken/bin/../lib/libLLVM.so.21.0git+0xe9475e)
 #2 0x00007f1beb6533b1 SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0
 #3 0x00007f1bea25b050 (/lib/x86_64-linux-gnu/libc.so.6+0x3c050)
 #4 0x00007f1bf5e57e31 addAssociatedClassesAndNamespaces((anonymous namespace)::AssociatedLookup&, clang::CXXRecordDecl*) SemaLookup.cpp:0:0
 #5 0x00007f1bf5e45f6a addAssociatedClassesAndNamespaces((anonymous namespace)::AssociatedLookup&, clang::QualType) SemaLookup.cpp:0:0
 #6 0x00007f1bf5e45c81 clang::Sema::FindAssociatedClassesAndNamespaces(clang::SourceLocation, llvm::ArrayRef<clang::Expr*>, llvm::SmallSetVector<clang::DeclContext*, 16u>&, llvm::SmallSetVector<clang::CXXRecordDecl*, 16u>&) (/tmp/llvm/obj-broken/bin/../lib/libclang-cpp.so.21.0git+0x1ad3c81)

I'm running cvise to reduce the source code that leads to this.

@alexfh
Copy link
Contributor

alexfh commented Mar 25, 2025

We're also seeing Clang crashes after the original commit, which are not fixed here. cvise has been running for a few hours. Hopefully, I get a shareable reproducer today.

@alexfh
Copy link
Contributor

alexfh commented Mar 25, 2025

Reduced test case: https://gcc.godbolt.org/z/7xxfsj4vv

struct A {
  template <class T> A(T);
};
struct C;
template <class T> void d(void (T::*)());
void f(A);
void g() { f(d<C>); }

Please revert or fix soon. Thanks!

mizvekov added a commit that referenced this pull request Mar 25, 2025
Fix crash when looking up associated namespaces for member pointers with an
unknown class, due to dependency.

It was a regression introduced in #131965
and reported here: #132317 (comment)

No change notes since the regression was never released.
@mizvekov
Copy link
Contributor Author

@alexfh thanks, this will be fixed here: #132977

@glandium
Copy link
Contributor

For the record, my reduced test case is:

namespace camera {
template <class MEM_FUN> void GetChildAndCall(MEM_FUN);
struct CamerasChild {
  template <typename This>
  void ConnectDeviceListChangeListener(void (This::*)());
};
} // namespace camera
struct MediaEngineWebRTC {
  MediaEngineWebRTC();
};
using camera::CamerasChild;
using camera::GetChildAndCall;
MediaEngineWebRTC::MediaEngineWebRTC() {
  GetChildAndCall(
      &CamerasChild::ConnectDeviceListChangeListener<MediaEngineWebRTC>);
}

and is fixed by #132977.

@alexfh
Copy link
Contributor

alexfh commented Mar 26, 2025

@alexfh thanks, this will be fixed here: #132977

We see more clang crashes after #132977 / 9606159. I'll start a reduction and will post a new test case, but please revert #132977 and this commit.

@mizvekov
Copy link
Contributor Author

You mean crashes caused by this fix, or do you mean crashes not fixed by it?

In any case, I won't be available to revert anything until tomorrow.

You can go ahead and revert, but I am afraid it won't be straightforward.

@mizvekov
Copy link
Contributor Author

If you do revert, please avoid reverting tests if possible.

@alexfh
Copy link
Contributor

alexfh commented Mar 26, 2025

There are new crashes due to #132977.

You can go ahead and revert, but I am afraid it won't be straightforward.

It waits until tomorrow. I get that reverting the right set of commits and leaving tests would be not completely trivial, so I'd leave this to you.

CVise is running for the new issue. Hopefully, tomorrow I'll provide a new test case.

@alexfh
Copy link
Contributor

alexfh commented Mar 26, 2025

The reduced test case:

template <typename>
struct A;
template <typename T>
struct A<void (T::C::*)(int &, T *)> {};
template <typename T>
void e(T) {
  A<T> f;
}
struct S {
  struct C {
    void h(int &, S *);
  };
  void i() { e(&C::h); }
};

https://gcc.godbolt.org/z/8Pq9EWbTv

@rupprecht
Copy link
Collaborator

The reduced test case:

template <typename>
struct A;
template <typename T>
struct A<void (T::C::*)(int &, T *)> {};
template <typename T>
void e(T) {
  A<T> f;
}
struct S {
  struct C {
    void h(int &, S *);
  };
  void i() { e(&C::h); }
};

https://gcc.godbolt.org/z/8Pq9EWbTv

That's the same assertion failure as in #132401 (comment)

@mizvekov
Copy link
Contributor Author

@alexfh @rupprecht thanks, that was indeed a separate regression, and this will be fixed here: #133113

@alexfh
Copy link
Contributor

alexfh commented Mar 26, 2025

Thanks for the fast fix! I would still insist on reverting first and recommiting the changes along with the fix (I could run this through a bit more testing on real code for more confidence), rather than trying to expedite the review and potentially introducing more regressions.

@mizvekov
Copy link
Contributor Author

@alexfh Are you sure the original crashes were due to #132977 ?

Looking at the patch, it seems unlikely it could cause a crash in itself.

What I think happened is that your compilation proceeded further along and encountered the other crash, which was a regression introduced by a different commit, which had been reported even before that last commit was landed.

It's certainly possible to revert this commit, however there are a lot of dependent patches by this point, and I think this last crash is isolated enough to point into #132401 instead of this commit.

@alexfh
Copy link
Contributor

alexfh commented Mar 26, 2025

@alexfh Are you sure the original crashes were due to #132977 ?

I thought so, but your explanation below also seems reasonable. I can try to verify.

Looking at the patch, it seems unlikely it could cause a crash in itself.

What I think happened is that your compilation proceeded further along and encountered the other crash, which was a regression introduced by a different commit, which had been reported even before that last commit was landed.

It's certainly possible to revert this commit, however there are a lot of dependent patches by this point, and I think this last crash is isolated enough to point into #132401 instead of this commit.

@alexfh
Copy link
Contributor

alexfh commented Mar 26, 2025

@alexfh Are you sure the original crashes were due to #132977 ?

I thought so, but your explanation below also seems reasonable. I can try to verify.

Looking at the patch, it seems unlikely it could cause a crash in itself.
What I think happened is that your compilation proceeded further along and encountered the other crash, which was a regression introduced by a different commit, which had been reported even before that last commit was landed.
It's certainly possible to revert this commit, however there are a lot of dependent patches by this point, and I think this last crash is isolated enough to point into #132401 instead of this commit.

I can confirm (using the reduced test case) that the last crash is due to #132401, but it was hidden until #132977 was in the tree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category clang-tidy clang-tools-extra debuginfo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants