[Lldb-commits] [PATCH] D98197: [lldb] Fix DWARF-5 DW_FORM_implicit_const (used by GCC)

Tue Mar 9 00:18:15 PST 2021

labath added a comment.

Could you simplify the test case? It sounds like all you need is a single global variable (`int foo = 47;`), without any functions or that stuff. I guess we should also delete `test/Shell/Breakpoint/implicit_const_form_support.test`, as it's not very useful (the numbers it checks come from the line table, not from the implicit constants).  Other than that, this seems fine.

================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp:30
+                             dw_offset_t attr_die_offset, dw_attr_t attr) {
+  AttributeValue attr_value = {const_cast<DWARFUnit *>(form_value.GetUnit()),
+                               attr_die_offset,
----------------
jankratochvil wrote:
> This `const_cast` is not nice. For its removal one can do either of:
>   - Just pass the `DWARFUnit *cu` additional parameter like there was before (but it is a duplication of data)
>   - de-const `DWARFUnit *` in a lot of code around: https://people.redhat.com/jkratoch/lldb-deconst.patch
> 
this is not the first const_cast of DWARFUnits, so, while definitely not ideal, I'm not particularly worried about it.

================
Comment at: lldb/test/Shell/SymbolFile/DWARF/dwarf5-implicit-const.s:3
+
+# UNSUPPORTED: system-darwin, system-windows
+# REQUIRES: x86
----------------
I hope these are not necessary, since you don't run the code here.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98197