jankratochvil marked 4 inline comments as done. jankratochvil added inline comments.
================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h:113 - size_t GetAttributes(DWARFAttributes &attributes, uint32_t depth = 0) const; + size_t GetAttributes(DWARFAttributes &attributes, int32_t depth = 0) const; ---------------- labath wrote: > Using -1 to prevent recursion is pretty unobvious. I think it would be better > to add a `bool recurse = true` argument between `attributes` and `depth`. In > fact, I'd consider even deleting the `depth` argument -- it's an internal > detail that noone except `DWARFDebugInfoEntry` should be using and the single > usage there can be easily changed to > `spec_die.GetDIE()->GetAttributes(spec_die.GetUnit(), attributes, recurse, > depth+1)` I had this idea. :-) The problem is `bool recurse` is type-compatible with `uint32_t depth` which leads to uncaught bugs. Given that non-recursive call makes no sense with `depth>0` I find the special value `depth = -1` to be suitable for non-recursive calls. Anyway implemented the `recurse` parameter in a safe way if it is worth it to separate it. ================ Comment at: lldb/test/Shell/SymbolFile/DWARF/DW_TAG_GNU_call_site-DW_AT_low_pc.s:14 +# RUN: %clang_host -o %t %s +# RUN: %lldb %t -o 'b 6' -o r -o 'p p' -o exit | FileCheck %s + ---------------- labath wrote: > Maybe place an int3 at the place where you want this to stop? Then you can > delete all of the line table directives and the break location will be more > obvious... Done, nice. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81334/new/ https://reviews.llvm.org/D81334 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
