jarin added a comment. Hi, could you take a look at this change?
Some discussion points: - In the ParseVariablesInFunctionContext method, we are using a lambda for the recursive parser. We could also use a function-local class or inner class of SymbolFileDWARF. Would any of these be preferable? - The variables created by ParseVariableDIE on abstract formal parameters are fairly strange, especially if a function gets inlined into two different functions. If that happens, then the parsed variable will refer to a symbol context that does not contain the variable DIE and a block can contain a variable that is not in the DIE of tree of the block. Is that a big problem? (Quick testing of this situation did not reveal any strange stack traces or `frame var` anomalies.) Unfortunately, there is no good way to provide the correct block and the correct function because LLDB does not parse functions and blocks for the abstract functions (i.e., for the DW_TAG_subroutines that are referenced by DW_AT_abstract_origin of concrete functions). - The provided test only tests the case of an inlined function where some parameters are unused/omitted. Would it make sense to also provide tests for other interesting cases or would that be too much bloat? The particularly interesting cases are: - Inlined function with all its parameters unused/omitted, - Inlined function that is called from different top-level functions. - Test correctness of the stack trace in the cases above. - We could supply a test written in C, but it needs -O1 and is fairly sensitive to the meaning of -O1 (e.g., clang started inlining and omitting unsued inlined parameters only recently, so changes to -O1 could make a C test easily meaningless). Any concerns here? - The provided test is a bit verbose, mostly because we wanted to mostly preserve the structure of the C compiler output. We could still cut the size of the test down by removing the main function in favour of _start and by removing all the file/line info. Would any of that make sense? 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
