Add pretty printers for llvm::PointerIntPair and llvm::PointerUnion.

[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: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

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

[csigg]

Fix formatting.

[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.gdb

42–47

If you aren't testing the pointer value anyway, you could make it null & then you can test directly for that?

llvm/utils/gdb-scripts/prettyprinters.py

325

Any chance of avoiding stringification/parsing here? could this be written as val.type.template_argument(3)['NumLowBitsAvailable'] instead? (I'm guessing not, but figured I'd check)

Oh, looks like maybe that does work: https://sourceware.org/gdb/current/onlinedocs/gdb/Types-In-Python.html - though it seems maybe gdb.Fields can only provide their value if they're an enum (enumval), maybe not if they're static? Even if the final name has to be stringified and eval'd - it'd still be nice to avoid string concatenation to create the name, if reasonably feasible.

328–329

Might be best just to say we can't print this thing if the trait isn't available, to err on the safe side of not giving possibly incorrect/misleading information?

[dblaikie]

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

42–47

Alternatively (a bit dodgy) you could hardcode a pointer value with a reinterpret cast, to check that the specific value does round-trip through the pretty printer.

Or you could dereference the pointer value in the pretty printing test to demonstrate you can reach the thing it's meant to point to.

Given the bit twiddling, it does seem like it might be nice to verify that the specific pointer value comes out unharmed.

[csigg]

llvm/utils/gdb-scripts/prettyprinters.py

325

Do you know if gdb.lookup_symbol(<string>).value() would be any better than gdb.parse_and_eval(<string>)?

I experimented for a while (disclaimer: I'm a Python GDB newbie) but couldn't get the enum to show up in PtrTraits.fields(), no matter whether the enum is named or anonymous.
However, the named enum can be found with gdb.lookup_type(), and the fields() do contain the enumerators. That would be the third option.

[dblaikie]

llvm/utils/gdb-scripts/prettyprinters.py

325

Sorry, the mention of enums was a red herring - not related to the existing enum usage here, but just something that was in the docs.

Nah, gdb.lookup_symbol(<string>).value() V gdb.parse_and_eval(<string>) doesn't make much difference to me.

Just looking at the constexpr static int member version of the code, not the enum - is there a way to avoid stringification to build the name of the constant? Can the static member variable be found in the fields list of the class? & then evaluated?

[csigg]

Applying dblaikie's review comments.

• Checking pointer address
• Switching from parse_and_eval to lookup_symbol
• Removing the fallback requires including D72590

[merge_guards_bot]

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

clang-tidy: unknown.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

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

[csigg]

llvm/utils/gdb-scripts/prettyprinters.py

325

There is no way to get to the static members of a struct without stringification, only enumerators of an enum.

Enums are no win because getting to the nested enum requires gdb.lookup_type(<stringification>). It would require a standardized enum name.

Instantiating the traits or info class (e.g. as empty base class) to get to the static member would add the requirement that PointerTy is complete. That seems worse.

I'm out of ideas for other approaches.

[csigg]

clang-format

[merge_guards_bot]

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

clang-tidy: unknown.

clang-format: pass.

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

[csigg]

Switch from lookup_symbol of PtrTraits's 'NumBitsAvailable' to lookup_type of Info::MaskAndShiftConstants.
The latter is not (or less) part of the public interface of PointerIntPair, and requires less code changes.

[csigg]

llvm/utils/gdb-scripts/prettyprinters.py

325

On second thought, gdb.lookup_type does look a little cleaner. Switched to that.

[csigg]

Remove debug hack.

[merge_guards_bot]

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

clang-tidy: unknown.

clang-format: pass.

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

[merge_guards_bot]

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

clang-tidy: unknown.

clang-format: pass.

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

[dblaikie]

Looks good - thanks!

[csigg]

Use factory methods for printeres to catch exceptions.

[merge_guards_bot]

Unit tests: pass. 62017 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

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

[csigg]

Revert merge accident.

[csigg]

Fix base. I'm so terrible at this.

[csigg]

Hi David. Thanks a lot for you review. I did another pass to switch to factory methods that catch exceptions without spewing common errors on the GDB console. Would you mind taking another quick peek? Thanks a lot for your patience!

[merge_guards_bot]

Unit tests: pass. 62026 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

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

[merge_guards_bot]

Unit tests: pass. 62026 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

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

[dblaikie]

llvm/utils/gdb-scripts/prettyprinters.py

349

Is this an idiomatic way to handle the "function that returns a thing or nothing"? It looks a bit quirky (it's a loop that's only ever intended to iterate once) & I wouldn't mind understanding better how/if this is idiomatic python (if you've got a link to some references about this) before committing it.

[csigg]

Getting rid of for loop to handle optional return value.

llvm/utils/gdb-scripts/prettyprinters.py

349

I don't know, and I couldn't find anything online. It did look pretty clean to me, but I'm not attached to it.

There is this discussion whether std::optional could be modeled as container of 0 or 1 element. And quite a few people were concerned that using it in a for loop is confusing:
https://www.reddit.com/r/cpp/comments/6uw5ny/stdoptional_and_container_interface/

Changing it to return a tuple of None.

[merge_guards_bot]

Unit tests: fail. 62021 tests passed, 6 failed and 782 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp
failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_mutex_requirements_mutex/thread_mutex_class/try_lock.pass.cpp

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.

[dblaikie]

Hmm - coming back to why these errors occur/need to be suppressed... why do these errors occur?

If it's only because of some missing refactorings/fixes to the APIs, I think maybe we should fix the APIs (since we can maintain the pretty printers in lock step with the APIs as they're committed to the same repository - we don't need backwards or forwards compatibility, etc)? Then we can skip the error handling & let it error out if we later break these APIs or add new pointer like type traits that are missing the relevant members, etc.

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

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

 p SmallString
 
 # CHECK: "bar"
 p StringRef
 
 # CHECK: "\"foo\"\"bar\""
 p Twine
 
+# CHECK: llvm::PointerIntPair = {pointer = 0xabc, value = 1}
+p PointerIntPair
+
+# CHECK: llvm::PointerUnion containing int * = {pointer = 0xabc}
+p PointerUnion
+
+

llvm/include/llvm/ADT/PointerIntPair.h

 
 template <typename PointerT, unsigned IntBits, typename PtrTraits>
 struct PointerIntPairInfo {
   static_assert(PtrTraits::NumLowBitsAvailable <
                     std::numeric_limits<uintptr_t>::digits,
                 "cannot use a pointer type that has all bits free");
   static_assert(IntBits <= PtrTraits::NumLowBitsAvailable,
                 "PointerIntPair with integer size too large for pointer");
-  enum : uintptr_t {
+  enum MaskAndShiftConstants : uintptr_t {
     /// PointerBitMask - The bits that come from the pointer.
     PointerBitMask =
         ~(uintptr_t)(((intptr_t)1 << PtrTraits::NumLowBitsAvailable) - 1),
 
     /// IntShift - The number of low bits that we reserve for other uses, and
     /// keep zero.
     IntShift = (uintptr_t)PtrTraits::NumLowBitsAvailable - IntBits,
 

llvm/utils/gdb-scripts/prettyprinters.py

 from __future__ import print_function
 import sys
 
 import gdb.printing
+import gdb.types
 
 class Iterator:
   def __iter__(self):
     return self
 
   if sys.version_info.major == 2:
       def next(self):
         return self.__next__()
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)
+    int_type = val.type.template_argument(2)
+    info_name = val.type.template_argument(4).name
+    enum_type = gdb.lookup_type(info_name + '::MaskAndShiftConstants')
+    enum_dict = gdb.types.make_enum_dict(enum_type)
+    ptr_mask = enum_dict[info_name + '::PointerBitMask']
+    int_shift = enum_dict[info_name + '::IntShift']
+    int_mask = enum_dict[info_name + '::IntMask']
+    pair_union = val['Value']
+    self.pointer = (pair_union & ptr_mask).cast(pointer_type)
+    self.value = ((pair_union >> int_shift) & int_mask).cast(int_type)
+
+  def children(self):
+    return [('pointer', self.pointer), ('value', self.value)]
+
+  def to_string(self):
+    return 'llvm::PointerIntPair'
+
+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
+
 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)
+gdb.printing.register_pretty_printer(gdb.current_objfile(), pp, True)