werat added a comment. Pavel, thanks for review!
In D92223#2422184 <https://reviews.llvm.org/D92223#2422184>, @labath wrote: > I am not sure if this is the right way to implement this feature. Changing > ManualDWARFIndex to provide this additional information is easy enough, but > it means that the other index classes will never be able to support this > functionality (without, essentially, reimplementing ManualDWARFIndex and > scanning all debug info). > ... > I think it might be better to do something at the > `SymbolFileDWARF::FindGlobalVariables` level, so that it is common to all > indexes. For example, this function could (in case the normal search does not > provide any information) try to strip the last component of the name > (`foo::bar::baz` => `foo::bar`) and then try to look up the result as a class > (type). If it finds the type, it could then check it for the specific > (static) variable name. This makes sense, let me try to implement this approach. A few questions before I jump into coding. Do you suggest to call `m_index->GetTypes` from `SymbolFileDWARF::FindGlobalVariables` and inspect the DIE for static members? Or use something more "high-level", e.g. `SymbolFileDWARF::FindTypes` and inspect the returned `lldb_private::Type` object? I had a brief look at `Type` and didn't find an easy way to get to static members there. Can you give me a pointer? > I am also worried about the increase to the manual index size, as this would > mean that every occurrence of the DW_TAG_member would be placed in the index, > whereas now we only place the one which has a location (which is normally > just in a single compile unit). Regarding the index size, we don't put ALL occurrences of `DW_TAG_member` in the index, only the ones with constant values. So it should be the same as if all these members had a location. ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3248 + (parent_tag == DW_TAG_compile_unit || parent_tag == DW_TAG_partial_unit || + tag == DW_TAG_member) && + (parent_context_die.Tag() == DW_TAG_class_type || ---------------- labath wrote: > Why is this needed. I would expect this to evaluate to `true` even without > this addition... In case of `DW_TAG_member` parent_tag is `DW_TAG_class_type/DW_TAG_structure_type`, so this check doesn't pass. ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3270 if ((parent_tag == DW_TAG_compile_unit || - parent_tag == DW_TAG_partial_unit) && + parent_tag == DW_TAG_partial_unit || tag == DW_TAG_member) && Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU()))) ---------------- labath wrote: > Same here. As as above, `parent_tag` for `DW_TAG_member` is `DW_TAG_class_type/DW_TAG_structure_type` ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3518 if ((tag == DW_TAG_variable) || (tag == DW_TAG_constant) || + (tag == DW_TAG_member) || (tag == DW_TAG_formal_parameter && sc.function)) { ---------------- labath wrote: > Why is this needed? I wouldn't expect a member inside a function... I'm not sure I understand, wdym by "member inside a function"? `ParseVariables` is called directly from `FindGlobalVariables`, so as far as I understand it this check applies for processing global variables as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92223/new/ https://reviews.llvm.org/D92223 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
