bolshakov-a added inline comments.
================
Comment at: clang/lib/AST/MicrosoftMangle.cpp:720
+ const CXXRecordDecl *RD, const ValueDecl *VD) {
+ MSInheritanceModel IM = RD->getMSInheritanceModel();
+ // <nttp-class-member-data-pointer> ::= <member-data-pointer>
----------------
rjmccall wrote:
> efriedma wrote:
> > It's not obvious to me why the inheritance model is relevant here. If you
> > want to check if the class has virtual bases, please do that explicitly.
> > (Note that the inheritance model can be messed with using attributes; see
> > https://learn.microsoft.com/en-us/cpp/cpp/inheritance-keywords.)
> It would at least be good to test what exactly this is based on.
>
> If RD is the base class of the member pointer type, isn't RD followed by the
> unqualified name of VD potentially ambiguous when there's any inheritance at
> all? You don't even need multiple inheritance, just the presence of multiple
> classes in the hierarchy that declare a field with the same name. I mean, if
> that's what MSVC does then we need to match it, but we should make certain by
> testing member pointers at different positions in the hierarchy of the
> declared base.
>
> (And this is assuming the MSVC model that restricts what subclass member
> pointers are allowed in superclass member pointer types.)
@efriedma, MS inheritance model overrides real inheritance. I've added a couple
of test cases to reflect this. Btw, thanks for the link! I really didn't
understand what "unspecified inheritance" is and how it is even possible.
@rjmccall, thank you! I've updated the mangling to a more correct algorithm.
================
Comment at: clang/lib/AST/MicrosoftMangle.cpp:1857
T->castAs<MemberPointerType>()->getMostRecentCXXRecordDecl();
const ValueDecl *D = V.getMemberPointerDecl();
if (T->isMemberDataPointerType())
----------------
efriedma wrote:
> It might be more clear here to use `APValue::isNullPointer()`, instead of
> checking if `getMemberPointerDecl()` returns null. At first glance, it's not
> really obvious why `getMemberPointerDecl()` would return null. And in
> theory, it's possible to have a non-null APValue where
> `getMemberPointerDecl()` returns null (although in this context, such cases
> should get rejected earlier).
`APValue::isNullPointer()` works only with `LValue`s, not with `MemberPointer`s:
https://github.com/llvm/llvm-project/blob/llvmorg-17-init/clang/lib/AST/APValue.cpp#L1000
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146386/new/
https://reviews.llvm.org/D146386
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
