https://github.com/Michael137 commented:
Mostly reasonable. This patch does deviate slightly from how `IsPossibleDynamicType` behaves for DWARF. There we also mark a type dynamic if it has virtual bases (even without any virtual functions). We use this to determine the dynamic type of a derived class, for which the Itanium ABI runtime plugin uses the vtable (which exists for virtual bases too). Re. testing, since the metadata is just an optimization, an end-to-end test that checks whether we set the metadata is tricky. Is it possible to use the Itanium ABI with PDB? If so, you could check whether `frame variable` will correctly show you the derived type when printing a pointer to a base class. I think that would go through the Itanium ABI runtime plugin. Since we currently always set it to false, I would expect that to fail. But pass with your new patch. You could also write a targeted unit-test that creates a `CXXRecordDecl` (using `yaml2pdb` to generate the debug-info), and then check that the metadata is set correctly. `DWARFASTParserClang` has examples of tests using YAML to generate debug-info. Maybe you don't even need the `yaml2pdb`, and can hand-construct a record with the UdtRecordCompleter (not sure how convenient it is to instantiate one in a test though). `TestClangASTImporter.cpp` has examples of where we check `ClangASTMetadata` from custom record decls. If this ends up being overly tedious i think this change is simple enough to be landed as is. But would be nice to give testing a shot. https://github.com/llvm/llvm-project/pull/155853 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
