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

324

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.

327–328

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

324

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

324

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

324

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

324

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

348

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

348

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/Support/PointerLikeTypeTraits.h

 };
 } // namespace detail
 
 // Provide PointerLikeTypeTraits for non-cvr pointers.
 template <typename T> struct PointerLikeTypeTraits<T *> {
   static inline void *getAsVoidPointer(T *P) { return P; }
   static inline T *getFromVoidPointer(void *P) { return static_cast<T *>(P); }
 
-  enum { NumLowBitsAvailable = detail::ConstantLog2<alignof(T)>::value };
+  static const int NumLowBitsAvailable = detail::ConstantLog2<alignof(T)>::value;
 };
 
 template <> struct PointerLikeTypeTraits<void *> {
   static inline void *getAsVoidPointer(void *P) { return P; }
   static inline void *getFromVoidPointer(void *P) { return P; }
 
   /// Note, we assume here that void* is related to raw malloc'ed memory and
   /// that malloc returns objects at least 4-byte aligned. However, this may be
   /// wrong, or pointers may be from something other than malloc. In this case,
   /// you should specify a real typed pointer or avoid this template.
   ///
   /// All clients should use assertions to do a run-time check to ensure that
   /// this is actually true.
-  enum { NumLowBitsAvailable = 2 };
+  static const int NumLowBitsAvailable = 2;
 };
 
 // Provide PointerLikeTypeTraits for const things.
 template <typename T> struct PointerLikeTypeTraits<const T> {
   typedef PointerLikeTypeTraits<T> NonConst;
 
   static inline const void *getAsVoidPointer(const T P) {
     return NonConst::getAsVoidPointer(P);
   }
   static inline const T getFromVoidPointer(const void *P) {
     return NonConst::getFromVoidPointer(const_cast<void *>(P));
   }
-  enum { NumLowBitsAvailable = NonConst::NumLowBitsAvailable };
+  static const int NumLowBitsAvailable = NonConst::NumLowBitsAvailable;
 };
 
 // Provide PointerLikeTypeTraits for const pointers.
 template <typename T> struct PointerLikeTypeTraits<const T *> {
   typedef PointerLikeTypeTraits<T *> NonConst;
 
   static inline const void *getAsVoidPointer(const T *P) {
     return NonConst::getAsVoidPointer(const_cast<T *>(P));
   }
   static inline const T *getFromVoidPointer(const void *P) {
     return NonConst::getFromVoidPointer(const_cast<void *>(P));
   }
-  enum { NumLowBitsAvailable = NonConst::NumLowBitsAvailable };
+  static const int NumLowBitsAvailable = NonConst::NumLowBitsAvailable;
 };
 
 // Provide PointerLikeTypeTraits for uintptr_t.
 template <> struct PointerLikeTypeTraits<uintptr_t> {
   static inline void *getAsVoidPointer(uintptr_t P) {
     return reinterpret_cast<void *>(P);
   }
   static inline uintptr_t getFromVoidPointer(void *P) {
     return reinterpret_cast<uintptr_t>(P);
   }
   // No bits are available!
-  enum { NumLowBitsAvailable = 0 };
+  static const int NumLowBitsAvailable = 0;
 };
 
 /// Provide suitable custom traits struct for function pointers.
 ///
 /// Function pointers can't be directly given these traits as functions can't
 /// have their alignment computed with `alignof` and we need different casting.
 ///
 /// To rely on higher alignment for a specialized use, you can provide a
 /// customized form of this template explicitly with higher alignment, and
 /// potentially use alignment attributes on functions to satisfy that.
 template <int Alignment, typename FunctionPointerT>
 struct FunctionPointerLikeTypeTraits {
-  enum { NumLowBitsAvailable = detail::ConstantLog2<Alignment>::value };
+  static const int NumLowBitsAvailable = detail::ConstantLog2<Alignment>::value;
   static inline void *getAsVoidPointer(FunctionPointerT P) {
     assert((reinterpret_cast<uintptr_t>(P) &
             ~((uintptr_t)-1 << NumLowBitsAvailable)) == 0 &&
            "Alignment not satisfied for an actual function pointer!");
     return reinterpret_cast<void *>(P);
   }
   static inline FunctionPointerT getFromVoidPointer(void *P) {
     return reinterpret_cast<FunctionPointerT>(P);

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)
+    int_type = val.type.template_argument(2)
+    name = val.type.template_argument(3).name + '::NumLowBitsAvailable'
+    bits_available = int(gdb.lookup_symbol(name)[0].value())
+    int_mask = (1 << bits_available) - 1
+    int_shift = bits_available - val.type.template_argument(1)
+    pair_union = val['Value']
+    self.pointer = (pair_union & ~int_mask).cast(pointer_type)
+    self.value = ((pair_union & int_mask) >> int_shift).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)