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

38

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

llvm/utils/gdb-scripts/prettyprinters.py

370–373

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

38

Indeed, don't need to capture globals.

llvm/utils/gdb-scripts/prettyprinters.py

370–373

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

38

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

330–331

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

if x:
  return y()
return z()

349

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

38

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

llvm/utils/gdb-scripts/prettyprinters.py

349

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

41–50

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

41–50

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/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};
 
 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;
 
-int main() {
-  return 0;
+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

 # CHECK: "foo"
 p SmallString
 
 # CHECK: "bar"
 p StringRef
 
 # CHECK: "\"foo\"\"bar\""
 p Twine
 
+# CHECK: Value = 13
+# CHECK: Value = 14
+# CHECK: Value = 15
+p Ilist
+
+# CHECK: Value = 15
+# CHECK: Value = 14
+# CHECK: Value = 13
+p SimpleIlist

llvm/utils/gdb-scripts/prettyprinters.py

def string_from_twine_object(self, twine):
     lhs_str = self.string_from_child(lhs, lhs_kind)
     rhs_str = self.string_from_child(rhs, rhs_kind)
 
     return lhs_str + rhs_str
 
   def to_string(self):
     return self.string_from_twine_object(self._val)
 
+class PointerIntPairPrinter:
+  """Print an llvm::PointerIntPair object."""
+
+  def __init__(self, val):
+    pointer_type = val.type.template_argument(0)
+    try:
+      name = val.type.template_argument(3).name + '::NumLowBitsAvailable'
+      bits_available = gdb.parse_and_eval(name)
+    except gdb.error:
+      # Above fails if NumLowBitsAvailable is an enum or is optimized away.
+      bits_available = pointer_type.target().alignof.bit_length() - 1
+    value_mask = (1 << bits_available) - 1
+    value_shift = bits_available - val.type.template_argument(1)
+    value = val['Value']
+    self.pointer = (value & ~value_mask).cast(pointer_type)
+    self.value = (value & value_mask) >> value_shift
+
+  def children(self):
+    return [('pointer', self.pointer), ('value', self.value)]
+
+  def to_string(self):
+    return 'llvm::PointerIntPair'
+
+  def display_hint (self):
+    return 'array'
+
+class PointerUnionPrinter:
+  """Print an llvm::PointerUnion object."""
+
+  def __init__(self, val):
+    pair_printer = PointerIntPairPrinter(val['Val'])
+    pointer_type = val.type.template_argument(int(pair_printer.value))
+    self.pointer = pair_printer.pointer.cast(pointer_type)
+
+  def children(self):
+    return [('pointer', self.pointer)]
+
+  def to_string(self):
+    return 'llvm::PointerUnion containing %s' % self.pointer.type
+
+  def display_hint (self):
+    return 'array'
+
+class IlistNodePrinter:
+  """Print an llvm::ilist_node object."""
+
+  def __init__(self, val):
+    pointer_type = val.type.pointer()
+    try:
+      pair_printer = PointerIntPairPrinter(val['PrevAndSentinel'])
+      self.prev = pair_printer.pointer.cast(pointer_type)
+      self.sentinel = bool(pair_printer.value)
+    except gdb.error:
+      # Fall back to base node that doesn't contain sentinel flag.
+      self.prev = val['Prev'].cast(pointer_type)
+      self.sentinel = None
+    self.next = val['Next'].cast(pointer_type)
+
+  def _get_pointer(self, pointer):
+    printer = IlistNodePrinter(pointer)
+    if printer.sentinel:
+      return pointer
+    node_type = pointer.type.target().template_argument(0)
+    return pointer.cast(node_type.pointer())
+
+  def children(self):
+    if self.sentinel:
+      yield ('sentinel', 'true')
+    yield ('prev', self._get_pointer(self.prev))
+    yield ('next', self._get_pointer(self.next))
+
+  def to_string(self):
+    return 'llvm::ilist_node'
+
+  def display_hint(self):
+    return 'array'
+
+class IlistPrinter:
+  """Print an llvm::simple_ilist or llvm::iplist object."""
+
+  def __init__(self, val):
+    self.node_type = val.type.template_argument(0)
+    # First field is common base type of sentinel and ilist_node.
+    base_type = val['Sentinel'].type.fields()[0].type
+    self.sentinel = val['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())
+
+  def to_string(self):
+    return 'llvm::ilist'
+
+  def display_hint(self):
+    return 'array'
+
 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)
-gdb.printing.register_pretty_printer(gdb.current_objfile(), pp)
+pp.add_printer('llvm::PointerIntPair', '^llvm::PointerIntPair<.*>$', PointerIntPairPrinter)
+pp.add_printer('llvm::PointerUnion', '^llvm::PointerUnion<.*>$', PointerUnionPrinter)
+pp.add_printer('llvm::ilist_node', '^llvm::ilist_node<.*>$', IlistNodePrinter)
+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, True)