This is a continuation of my quest to make the size 0 a supported value.
Details
- Reviewers
jingham labath - Commits
- rGd13777aa18cf: Make Type::GetByteSize optional (NFC)
rG729fcf179356: Make Type::GetByteSize optional (NFC)
rLLDB352521: Make Type::GetByteSize optional (NFC)
rL352521: Make Type::GetByteSize optional (NFC)
rLLDB352394: Make Type::GetByteSize optional (NFC)
rL352394: Make Type::GetByteSize optional (NFC)
Diff Detail
- Repository
- rL LLVM
Event Timeline
Looks fine to me.
I guess this would be the place I'd start thinking about an OptionalSize class (to replace the two fields in the Type class). But I don't think that's necessary yet...
include/lldb/Symbol/Type.h | ||
---|---|---|
221 ↗ | (On Diff #183671) | Does this need to be uint64_t to share the same space as m_byte_size? |
include/lldb/Symbol/Type.h | ||
---|---|---|
221 ↗ | (On Diff #183671) | I don't think so, but it might be more readable. |
Pavel, I may need you help. I reverted the commit because it broke three PDB-related tests (SymbolFilePDBTests, func-symbols, and typedefs) that need more than just lld in order to run. The fix is probably trivial. We only need to set a breakpoint in Type.cpp:119 and every time a type with size 0 is passed in look at the frame we're coming from and make sure that that llvm::None is passed in instead of 0, similar to the changes in DWARFASTParserClang.cpp.
The attached patch is what it took to get the tests passing for me. It could use a bit of cleanup though.
I am not sure if you saw this, but it looks like it broke one of the tests on the Windows bot:
http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/1092