labath added a comment. Just a couple of questions inline.
In D110571#3035814 <https://reviews.llvm.org/D110571#3035814>, @jarin wrote: > - thread #1, name = 'a.out', stop reason = breakpoint 1.2 frame #0: > 0x0000000000401151 a.out`main [inlined] f(used=4, unused=<unavailable>) at > a.cc:5:3 > > (lldb) frame var > (int) used = 4 > (int) local = 5 <--- HERE, a local variables got between the parameters > because we append unused parameters at the end. > (int) unused = <no location, value may have been optimized out> > > Let me try to rewrite the code so that the trailing unused parameters are > inserted after the last concrete parameter (or at the beginning of variable > list if there are no concrete parameters). Let me know if you think it is > unnecessary. TBH, I have no idea. The function description comes out right, so it seems at least some parts of lldb are prepared to handle this. I suppose it would be nicer if they were grouped together, but if it makes the code significantly more complex, then I probably wouldn't bother. ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3561-3562 + const lldb::addr_t func_low_pc, VariableList &variable_list, + llvm::SmallVectorImpl<DWARFDIE> &abstract_formal_parameters, + size_t &abstract_parameter_index) { size_t vars_added = 0; ---------------- I'm wondering if one could pass a single `ArrayRef<DWARFDIE> &` argument instead of the array+index pair. FWICS, you're processing the list in a left-to-right fashion, which seems ideal for `ArrayRef::drop_front`. WDYT? ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3575-3576 + bool found = false; + while (parameter) { + parameter = parameter.GetReferencedDIE(DW_AT_abstract_origin); + if (parameter == abstract_parameter) { ---------------- All of these abstract origin loops make me uneasy. They make it very easy to hang (whether deliberately or not) the debugger with a bit of incorrect dwarf (self-referencing DIEs). Do we actually know of any abstract_origin chains? It's not really clear to me what would be the right interpretation of that, so I can't even say whether this algorithm would be correct there. Maybe just stick to a single "dereference" ? ================ Comment at: lldb/test/API/functionalities/unused-inlined-parameters/TestUnusedInlinedParameters.py:17 + + # For the unused parameters, only check the types. + self.assertEqual("void *", self.frame().FindVariable("unused1").GetType().GetName()) ---------------- Maybe we could check something else as well... Do `GetDescription` or `str(value)` return something reasonable here? ================ Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test:1 +# UNSUPPORTED: system-darwin, system-windows + ---------------- You should be able to drop this now. ================ Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test:33 +# Note: image lookup does not show variables outside of their +# location, so |partial| is missing here. +# CHECK: name = "unused3", type = "int", location = <empty> ---------------- You could add `CHECK-NOT: partial` here Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110571/new/ https://reviews.llvm.org/D110571 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
