Add GDB pretty printers for llvm::ilist, llvm::simple_ilist, and llvm::ilist_node.

[merge_guards_bot]

Unit tests: pass. 61771 tests passed, 0 failed and 780 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[dblaikie]

debuginfo-tests/llvm-prettyprinters/gdb/prettyprinters.cpp

64

(looks like this capture should be removed, according to the lint check)

llvm/utils/gdb-scripts/prettyprinters.py

371–374

Under what conditions does this fallback occur? Are those conditions tested?

[csigg]

Reworking things and improving after PointerIntPair printer has landed.

[csigg]

Remove capture from lambda.

[csigg]

debuginfo-tests/llvm-prettyprinters/gdb/prettyprinters.cpp

64

Indeed, don't need to capture globals.

llvm/utils/gdb-scripts/prettyprinters.py

371–374

I changed it to inspect the template parameter, instead of blindly trying both.

It's not possible to check the 'Prev' code path without rebuilding LLVM with different #defines.

[merge_guards_bot]

Unit tests: pass. 62221 tests passed, 0 failed and 816 were skipped.

clang-tidy: fail. clang-tidy found 1 errors and 0 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[dblaikie]

debuginfo-tests/llvm-prettyprinters/gdb/prettyprinters.cpp

64

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)

llvm/utils/gdb-scripts/prettyprinters.py

380–381

Drop the else-after-return, per LLVM style:

if x:
  return y()
return z()

399

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.

[csigg]

Switch from factory method to printer class.

[csigg]

Switch from factory method to printer class.

[merge_guards_bot]

Unit tests: pass. 62221 tests passed, 0 failed and 816 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[merge_guards_bot]

Unit tests: pass. 62221 tests passed, 0 failed and 816 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[csigg]

debuginfo-tests/llvm-prettyprinters/gdb/prettyprinters.cpp

64

Sorry, I think the update was in flight while you were reviewing.

llvm/utils/gdb-scripts/prettyprinters.py

399

I changed it to a class, which makes sense because initialization should not fail.

But for what it's worth, I don't really see the 'some value'. I think it was perfectly readable and concise before.

[dblaikie]

debuginfo-tests/llvm-prettyprinters/gdb/prettyprinters.gdb

48–58

Could you check these with the surrounding syntax too to give a better sense of how the printing would look to the user?

(I guess they'll print in {}, is there a type prefix like with the other pretty printers (eg: "llvm::ArrayRef of", etc)?

[csigg]

Add detailed CHECKs.

[dblaikie]

Thanks for the details! I'd be /OK/ with this, but it doesn't seem ideal to be printing the next/prev when printing the container? Do you think it's worth/there's any nice way to avoid that significant redundancy/verbosity in the printing & just print the payload?

[csigg]

debuginfo-tests/llvm-prettyprinters/gdb/prettyprinters.gdb

48–58

Done.

No, there is no prefix because the printer has no to_string method. That's intentional because gdb does not print the type for ordinary values either. There is ptype for that. CLion watches show the result of both print and ptype. I should probably go back and remove it from the PointerIntPair as well, and add individual printer classes while I'm at it.

[merge_guards_bot]

Unit tests: pass. 62221 tests passed, 0 failed and 816 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[csigg]

In D72589#1845519, @dblaikie wrote:

Thanks for the details! I'd be /OK/ with this, but it doesn't seem ideal to be printing the next/prev when printing the container? Do you think it's worth/there's any nice way to avoid that significant redundancy/verbosity in the printing & just print the payload?

Well, it's an intrusive list. I'm not aware of any way to hide the ilist_node base class when printing the list, that would be the responsibility of the derived class' printer (which probably wouldn't want to do that, in case someone prints an individual node).
children can only return a list of gdb.Value, and gdb picks the printers for those internally. And to make things a little more complicated, like in the test here, there can be multiple ilist_node base classes and you would only want to silence the one with the corresponding tag.

[dblaikie]

In D72589#1846185, @csigg wrote:

In D72589#1845519, @dblaikie wrote:

Thanks for the details! I'd be /OK/ with this, but it doesn't seem ideal to be printing the next/prev when printing the container? Do you think it's worth/there's any nice way to avoid that significant redundancy/verbosity in the printing & just print the payload?

Well, it's an intrusive list. I'm not aware of any way to hide the ilist_node base class when printing the list, that would be the responsibility of the derived class' printer (which probably wouldn't want to do that, in case someone prints an individual node).
children can only return a list of gdb.Value, and gdb picks the printers for those internally. And to make things a little more complicated, like in the test here, there can be multiple ilist_node base classes and you would only want to silence the one with the corresponding tag.

Yup. Better than nothing & no doubt something better than this would be non-trivial.

debuginfo-tests/llvm-prettyprinters/gdb/prettyprinters.cpp

 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/ADT/ilist.h"
 #include "llvm/Support/Error.h"
 
 int Array[] = {1, 2, 3};
 auto IntPtr = reinterpret_cast<int *>(0xabc);
 
 llvm::ArrayRef<int> ArrayRef(Array);
 llvm::MutableArrayRef<int> MutableArrayRef(Array);
 llvm::DenseMap<int, int> DenseMap = {{4, 5}, {6, 7}};
 llvm::Expected<int> ExpectedValue(8);
 llvm::Expected<int> ExpectedError(llvm::createStringError({}, ""));
 llvm::Optional<int> OptionalValue(9);
 llvm::Optional<int> OptionalNone(llvm::None);
 llvm::SmallVector<int, 5> SmallVector = {10, 11, 12};
 llvm::SmallString<5> SmallString("foo");
 llvm::StringRef StringRef = "bar";
 llvm::Twine Twine = llvm::Twine(SmallString) + StringRef;
 llvm::PointerIntPair<int *, 1> PointerIntPair(IntPtr, 1);
 llvm::PointerUnion<float *, int *> PointerUnion(IntPtr);
 
+using IlistTag = llvm::ilist_tag<struct A>;
+using SimpleIlistTag = llvm::ilist_tag<struct B>;
+struct IlistNode : llvm::ilist_node<IlistNode, IlistTag>,
+                   llvm::ilist_node<IlistNode, SimpleIlistTag> {
+  int Value;
+};
+auto Ilist = [] {
+  llvm::ilist<IlistNode, IlistTag> Result;
+  for (int I : {13, 14, 15}) {
+    Result.push_back(new IlistNode);
+    Result.back().Value = I;
+  }
+  return Result;
+}();
+auto SimpleIlist = []() {
+  llvm::simple_ilist<IlistNode, SimpleIlistTag> Result;
+  for (auto &Node : Ilist)
+    Result.push_front(Node);
+  return Result;
+}();
+
 int main() { return 0; }

debuginfo-tests/llvm-prettyprinters/gdb/prettyprinters.gdb

 p Twine
 
 # CHECK: llvm::PointerIntPair<int *> = {pointer = 0xabc, value = 1}
 p PointerIntPair
 
 # CHECK: llvm::PointerUnion containing int * = {pointer = 0xabc}
 p PointerUnion
 
-
+# CHECK-NOT: ilist_node_impl
+# CHECK: Value = 13
+# CHECK: Value = 14
+# CHECK: Value = 15
+p Ilist
+
+# CHECK-NOT: ilist_node_impl
+# CHECK: Value = 15
+# CHECK: Value = 14
+# CHECK: Value = 13
+p SimpleIlist

llvm/utils/gdb-scripts/prettyprinters.py

def make_pointer_union_printer(val):
   """Factory for an llvm::PointerUnion printer."""
   pointer, value = get_pointer_int_pair(val['Val'])
   if not pointer or not value:
     return None
   pointer_type = val.type.template_argument(int(value))
   string = 'llvm::PointerUnion containing %s' % pointer_type
   return make_printer(string, [('pointer', pointer.cast(pointer_type))])
 
+def make_ilist_node_printer(val):
+  """Factory for an llvm::ilist_node printer."""
+
+  impl_type = gdb.lookup_type(val.type.fields()[0].name)
+  base_type = gdb.lookup_type(impl_type.fields()[0].name)
+  derived_type = val.type.template_argument(0)
+
+  def get_prev_and_sentinel(base):
+    # One of Prev and PrevAndSentinel exists. Depending on #defines used to
+    # compile LLVM, the base_type's template argument is either true of false.
+    if base_type.template_argument(0):
+      return get_pointer_int_pair(base['PrevAndSentinel'])
+    else:
+      return base['Prev'], None
+
+  def cast_pointer(pointer):
+    """Casts a base_type pointer to the appropriate derived type."""
+    sentinel = get_prev_and_sentinel(pointer.dereference())[1]
+    pointer = pointer.cast(impl_type.pointer())
+    if sentinel:
+        return pointer
+    return pointer.cast(derived_type.pointer())
+
+  # Repeated cast becaue val.type's base_type is ambiguous when using tags.
+  base = val.cast(impl_type).cast(base_type)
+  (prev, sentinel) = get_prev_and_sentinel(base)
+  prev = prev.cast(base_type.pointer())
+  prev, next = map(cast_pointer, (prev, val['Next']))
+  children = [('prev', prev), ('next', next)]
+  if sentinel:
+    children = [('sentinel', 'yes')] + children
+  return make_printer(children=children)
+
+class IlistPrinter:
+  """Print an llvm::simple_ilist or llvm::iplist object."""
+
+  def __init__(self, val):
+    self.node_type = val.type.template_argument(0)
+    sentinel = val['Sentinel']
+    # First field is common base type of sentinel and ilist_node.
+    base_type = sentinel.type.fields()[0].type
+    self.sentinel = sentinel.address.cast(base_type.pointer())
+
+  def _pointers(self):
+    pointer = self.sentinel
+    while True:
+      pointer = pointer['Next'].cast(pointer.type)
+      if pointer == self.sentinel:
+        return
+      yield pointer.cast(self.node_type.pointer())
+
+  def children(self):
+    for k, v in enumerate(self._pointers()):
+      yield ('[%d]' % k, v.dereference())
+
+
 pp = gdb.printing.RegexpCollectionPrettyPrinter("LLVMSupport")
 pp.add_printer('llvm::SmallString', '^llvm::SmallString<.*>$', SmallStringPrinter)
 pp.add_printer('llvm::StringRef', '^llvm::StringRef$', StringRefPrinter)
 pp.add_printer('llvm::SmallVectorImpl', '^llvm::SmallVector(Impl)?<.*>$', SmallVectorPrinter)
 pp.add_printer('llvm::ArrayRef', '^llvm::(Mutable)?ArrayRef<.*>$', ArrayRefPrinter)
 pp.add_printer('llvm::Expected', '^llvm::Expected<.*>$', ExpectedPrinter)
 pp.add_printer('llvm::Optional', '^llvm::Optional<.*>$', OptionalPrinter)
 pp.add_printer('llvm::DenseMap', '^llvm::DenseMap<.*>$', DenseMapPrinter)
 pp.add_printer('llvm::Twine', '^llvm::Twine$', TwinePrinter)
 pp.add_printer('llvm::PointerIntPair', '^llvm::PointerIntPair<.*>$', make_pointer_int_pair_printer)
 pp.add_printer('llvm::PointerUnion', '^llvm::PointerUnion<.*>$', make_pointer_union_printer)
+pp.add_printer('llvm::ilist_node', '^llvm::ilist_node<.*>$', make_ilist_node_printer)
+pp.add_printer('llvm::iplist', '^llvm::iplist<.*>$', IlistPrinter)
+pp.add_printer('llvm::simple_ilist', '^llvm::simple_ilist<.*>$', IlistPrinter)
 gdb.printing.register_pretty_printer(gdb.current_objfile(), pp)