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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
