shafik added inline comments.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2393
lldb_private::ClangASTImporter::LayoutInfo &layout_info,
- BitfieldInfo &last_field_info) {
+ FieldInfo &last_bitfield_info, FieldInfo &last_field_info) {
ModuleSP module_sp = parent_die.GetDWARF()->GetObjectFile()->GetModule();
----------------
teemperor wrote:
> IIUC then those two bitfield info variables are mutually exclusive? I.e.,
> either the last field was bitfield or a normal field? If yes, would it make
> sense to model this as one FieldInfo variable and have a bool value for
> differentiating between them? That way we don't have the possibility that we
> forget to clear one when we set the other.
It was not obvious during the initial refactoring wave but yes, I removed one
of them.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2674
- if (anon_field_info.IsValid()) {
+ if (unnamed_field_info.IsValid()) {
+ if (data_bit_offset != UINT64_MAX)
----------------
aprantl wrote:
> Can you please add comments explaining what each of these cases handle?
I removed a lot of in the next wave of refactoring.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h:174
+ struct FieldInfo {
uint64_t bit_size;
uint64_t bit_offset;
----------------
aprantl wrote:
> Not you fault, but: This data structure is a disaster waiting to happen.
> Instead of having magic sentinel values, should we use an Optional<FieldInfo>
> that is always valid if it exists? Or should the members be Optionals?
This is a good point, it took a bit more refactoring of the code using this but
I removed the magic numbers and used an `Optional<FieldInfo>` for the
`unnamed_field_info` case.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72953/new/
https://reviews.llvm.org/D72953
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
