DavidSpickett added inline comments.
================ Comment at: lldb/source/Expression/DWARFExpressionList.cpp:93 const DWARFExpression &expr = m_exprs.GetEntryRef(0).data; - return expr.ContainsThreadLocalStorage(); + return expr.ContainsThreadLocalStorage(*m_dwarf_cu); } ---------------- pfaffe wrote: > DavidSpickett wrote: > > I'm not sure that `m_dwarf_cu` is always non null. It might be in practice > > but for example one use ends up in `UpdateValueTypeFromLocationDescription` > > which checks that it is not null before use. > > > > Maybe it is reasonable to assume it is never null (and if it is and you > > have the time, a preparatory patch to make it a ref would be great). > I think it's worth checking in general. Before > https://reviews.llvm.org/D125509, the DWARFExpression used to be assiciated > with its dwarf_cu. I was considering bringing that back, what do you think? I think you should ask the author of that change :) On the surface that change makes sense but I'm no expert. If that's the case then you'll just want to sprinkle some `if not nullptr` around wherever you use it. (nullptrs are fine if there is a reason to use the null state) ================ Comment at: lldb/unittests/Expression/DWARFExpressionTest.cpp:571 + } + + // Report the skipped distance: ---------------- pfaffe wrote: > DavidSpickett wrote: > > Is there not 3 more arguments to read off here? The `0x00 0x00 0x00` from > > above. > No, the second argument to this opcode is of type uint32_t, the zeros are the > last three bytes of that. Should I maybe use a simpler/fake opcode? Might be > dangerous if that fake opcode ever got occupied/implemented. It was the comment misleading me: ``` // Called with "arguments" 0x03, 0x04, 0x00, 0x00, 0x00 ``` So I'm thinking there is an operation that is called with arguments 3, 4, 0, 0, 0. Just reword the comment and it'll be fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137247/new/ https://reviews.llvm.org/D137247 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
