Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding a release note. I am fine with how this paragraph is written but I must say that first run-on sentence reminded me of this short video 😅

The handling of the member pointer access looks good and the tests look great. LGTM.


.. 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
---------------------------

Expand Down
21 changes: 21 additions & 0 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -5051,6 +5054,24 @@ static const ValueDecl *HandleMemberPointerAccess(EvalInfo &Info,
return nullptr;
}
}
// 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])
: LV.Designator.MostDerivedType->getAsCXXRecordDecl();
const CXXRecordDecl *LastMPDecl = MemPtr.getContainingRecord();
if (LastLVDecl->getCanonicalDecl() != LastMPDecl->getCanonicalDecl()) {
Info.FFDiag(RHS);
return nullptr;
}

// Truncate the lvalue to the appropriate derived class.
if (!CastToDerivedClass(Info, RHS, LV, MemPtr.getContainingRecord(),
Expand Down
30 changes: 30 additions & 0 deletions clang/test/SemaCXX/constant-expression-cxx11.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2615,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}}
}
13 changes: 13 additions & 0 deletions clang/test/SemaCXX/constant-expression-cxx2a.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1497,3 +1497,16 @@ namespace GH67317 {
// expected-note {{subobject of type 'const unsigned char' is not initialized}}
__builtin_bit_cast(unsigned char, *new char[3][1]);
};

namespace GH150705 {
struct A { };
struct B : A { };
struct C : A {
constexpr virtual int foo() const { return 0; }
};
constexpr auto p = &C::foo;
constexpr auto q = static_cast<int (A::*)() const>(p);
constexpr B b;
constexpr const A& a = b;
constexpr auto x = (a.*q)(); // expected-error {{constant expression}}
}