[PATCH] D72589: Add GDB pretty printers for llvm::ilist, llvm::simple_ilist, and llvm::ilist_node.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: debuginfo-tests/llvm-prettyprinters/gdb/prettyprinters.cpp:38
+}();
+auto SimpleIlist = [&]() {
+  llvm::simple_ilist<IlistNode, SimpleIlistTag> Result;
----------------
dblaikie wrote:
> (looks like this capture should be removed, according to the lint check)
Ping on this - please remove the capture (remove the "&" in the "[&]") since it doesn't capture anything. (global variables are accessible directly, without capturing them)


================
Comment at: llvm/utils/gdb-scripts/prettyprinters.py:380-381
+      return get_pointer_int_pair(base['PrevAndSentinel'])
+    else:
+      return base['Prev'], None
+
----------------
Drop the else-after-return, per LLVM style:

```
if x:
  return y()
return z()
```


================
Comment at: llvm/utils/gdb-scripts/prettyprinters.py:399
+    children = [('sentinel', 'yes')] + children
+  return make_printer(children=children)
+
----------------
Do you think the make_printer usage is sufficiently more readable than having a standalone pretty printer type (like IlistPrinter below)? 

I think there might be some value in just always writing these as standalone pretty printer classes rather than having some of them as factories and some as classes.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72589