[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin added a comment.

Thank you for the great comments, Pavel. I took a stab at merging the parameters without interleaving them with the locals. Let me know what you think; I can certainly put this back to the original state if you think this is a change for the worse.

(I am sorry for the churn, but I feel the code is fairly subtle and would like to leave it in a state we are all happy with.)



================
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;
----------------
labath wrote:
> 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?
I have replaced the in place-merging with a separate merging function, so this should not be relevant anymore.


================
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())
----------------
labath wrote:
> Maybe we could check something else as well... Do `GetDescription` or `str(value)` return something reasonable here?
Actually, the most important thing is the type here, so this was quite deliberate.

GetDescription returns `(void *) unused1 = <no location, value may have been optimized out>\n\n`, but I am not sure if we can safely require that all future versions/platforms optimize the parameter out.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110571/new/

https://reviews.llvm.org/D110571