[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

[Eli Friedman via Phabricator via cfe-commits]

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


================
Comment at: clang/lib/AST/MicrosoftMangle.cpp:1857
         T->castAs<MemberPointerType>()->getMostRecentCXXRecordDecl();
     const ValueDecl *D = V.getMemberPointerDecl();
     if (T->isMemberDataPointerType())
----------------
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).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146386/new/

https://reviews.llvm.org/D146386