[PATCH] D145123: Call MarkVirtualMembersReferenced on an actual class definition
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 2 06:30:11 PST 2023
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
Thanks! LGTM, though please add a release note about the fix.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:17940
LoadExternalVTableUses();
Class = Class->getCanonicalDecl();
std::pair<llvm::DenseMap<CXXRecordDecl *, bool>::iterator, bool>
----------------
sberg wrote:
> That call of `getCanonicalDecl` originated with <https://github.com/llvm/llvm-project/commit/88d292ccb86c10857298c252bb93331f7ef2258a> "Rework when and how vtables are emitted, by tracking where vtables are", and I wonder if more of the uses of the modified `Class` below suffer from the same issue, that they expect `Class` to represent a definition when it might potentially reference just a declaration.
The only other place the class is being used is in `VTableUses` and `Sema::DefineUsedVTables()` does `CXXRecordDecl *Class = VTableUses[I].first->getDefinition();` so I think that's safe (if it a bit convoluted). So I think this is okay as-is.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145123/new/
https://reviews.llvm.org/D145123
More information about the cfe-commits
mailing list