Address improved documentation requests from review

Author: keinflue

clang/docs/ReleaseNotes.rst

@@ -43,6 +43,26 @@ C++ Specific Potentially Breaking Changes
   regressions if your build system supports two-phase compilation model but haven't support
   reduced BMI or it is a compiler bug or a bug in users code.
 
+- Clang now correctly diagnoses during constant expression evaluation undefined behavior due to member
+  pointer access to a member which is not a direct or indirect member of the most-derived object
+  of the accessed object but is instead located directly in a sibling class to one of the classes
+  along the inheritance hierarchy of the most-derived object as ill-formed.
+  Other scenarios in which the member is not member of the most derived object were already
+  diagnosed previously. (#GH150709)
+
+  .. code-block:: c++
+
+    struct A {};
+    struct B : A {};
+    struct C : A { constexpr int foo() const { return 1; } };
+    constexpr A a;
+    constexpr B b;
+    constexpr C c;
+    constexpr auto mp = static_cast<int(A::*)() const>(&C::foo);
+    static_assert((a.*mp)() == 1); // continues to be rejected
+    static_assert((b.*mp)() == 1); // newly rejected
+    static_assert((c.*mp)() == 1); // accepted
+
 ABI Changes in This Version
 ---------------------------
 

clang/lib/AST/ExprConstant.cpp

@@ -5035,6 +5035,9 @@ static const ValueDecl *HandleMemberPointerAccess(EvalInfo &Info,
     // This is a member of some derived class. Truncate LV appropriately.
     // The end of the derived-to-base path for the base object must match the
     // derived-to-base path for the member pointer.
+    // C++23 [expr.mptr.oper]p4:
+    //   If the result of E1 is an object [...] whose most derived object does
+    //   not contain the member to which E2 refers, the behavior is undefined.
     if (LV.Designator.MostDerivedPathLength + MemPtr.Path.size() >
         LV.Designator.Entries.size()) {
       Info.FFDiag(RHS);
@@ -5051,7 +5054,15 @@ static const ValueDecl *HandleMemberPointerAccess(EvalInfo &Info,
         return nullptr;
       }
     }
-    // Consider member in a sibling.
+    // MemPtr.Path only contains the base classes of the class directly
+    // containing the member E2. It is still necessary to check that the class
+    // directly containing the member E2 lies on the derived-to-base path of E1
+    // to avoid incorrectly permitting member pointer access into a sibling
+    // class of the class containing the member E2. If this class would
+    // correspond to the most-derived class of E1, it either isn't contained in
+    // LV.Designator.Entries or the corresponding entry refers to an array
+    // element instead. Therefore get the most derived class directly in this
+    // case. Otherwise the previous entry should correpond to this class.
     const CXXRecordDecl *LastLVDecl =
         (PathLengthToMember > LV.Designator.MostDerivedPathLength)
             ? getAsBaseClass(LV.Designator.Entries[PathLengthToMember - 1])

clang/test/SemaCXX/constant-expression-cxx11.cpp

@@ -1210,24 +1210,6 @@ namespace MemberPointer {
     return (a.*f)();
   }
   static_assert(apply(A(2), &A::f) == 5, "");
-
-  struct C { };
-  struct D : C {
-    constexpr int f() const { return 1; };
-  };
-  struct E : C { };
-  struct F : D { };
-  constexpr C c1, c2[2];
-  constexpr D d1, d2[2];
-  constexpr E e1, e2[2];
-  constexpr F f;
-  static_assert((c1.*(static_cast<int (C::*)() const>(&D::f)))() == 1, ""); // expected-error {{constant expression}}
-  static_assert((d1.*(static_cast<int (C::*)() const>(&D::f)))() == 1, "");
-  static_assert((e1.*(static_cast<int (C::*)() const>(&D::f)))() == 1, ""); // expected-error {{constant expression}}
-  static_assert((f.*(static_cast<int (C::*)() const>(&D::f)))() == 1, "");
-  static_assert((c2[0].*(static_cast<int (C::*)() const>(&D::f)))() == 1, ""); // expected-error {{constant expression}}
-  static_assert((d2[0].*(static_cast<int (C::*)() const>(&D::f)))() == 1, "");
-  static_assert((e2[0].*(static_cast<int (C::*)() const>(&D::f)))() == 1, ""); // expected-error {{constant expression}}
 }
 
 namespace ArrayBaseDerived {
@@ -2633,3 +2615,33 @@ namespace DoubleCapture {
     };
   }
 }
+
+namespace GH150709 {
+  struct C { };
+  struct D : C {
+    constexpr int f() const { return 1; };
+  };
+  struct E : C { };
+  struct F : D { };
+  struct G : E { };
+  
+  constexpr C c1, c2[2];
+  constexpr D d1, d2[2];
+  constexpr E e1, e2[2];
+  constexpr F f;
+  constexpr G g;
+
+  constexpr auto mp = static_cast<int (C::*)() const>(&D::f);
+
+  // sanity checks for fix of GH150709 (unchanged behavior)
+  static_assert((c1.*mp)() == 1, ""); // expected-error {{constant expression}}
+  static_assert((d1.*mp)() == 1, "");
+  static_assert((f.*mp)() == 1, "");
+  static_assert((c2[0].*mp)() == 1, ""); // expected-error {{constant expression}}
+  static_assert((d2[0].*mp)() == 1, "");
+
+  // incorrectly undiagnosed before fix of GH150709
+  static_assert((e1.*mp)() == 1, ""); // expected-error {{constant expression}}
+  static_assert((e2[0].*mp)() == 1, ""); // expected-error {{constant expression}}
+  static_assert((g.*mp)() == 1, ""); // expected-error {{constant expression}}
+}