[NFC] Improve Debugging experience with SmallVector

[Tyker]

Summary

When debugging it is usefull to be able to look at the content of containers.
But many containers are templated so there member functions are not emitted if they are not used.
this patch adds the used attribute to some member functions of SmallVector, DenseMap and PointerIntPair
when asserts are enabled and the host complier supports it. this way interacting with thoses type
from the debugger is easier.

in this patch, I only updated the containers I most frequently have this issue with.
but many other could get the same treatment.

[dblaikie]

Interesting idea - but could you start an llvm-dev thread to discuss this idea, since it may become more pervasive & it'd be good to get buy-in to the general approach.

[aprantl]

llvm/include/llvm/Support/Compiler.h

501

We'd want to use LLVM_USED_FOR_DEBUG instead of LLVM_ATTRIBUTE_USED here too?

[Tyker]

for the case of SmallVector it is as easy as adding a few used
But for many containers adding used attribute may cause previously new failure because the function annotated cannot be instantiated.

llvm/include/llvm/ADT/DenseMap.h

public:
   }
   inline const_iterator end() const {
     return makeConstIterator(getBucketsEnd(), getBucketsEnd(), *this, true);
   }
 
   LLVM_NODISCARD bool empty() const {
     return getNumEntries() == 0;
   }
-  unsigned size() const { return getNumEntries(); }
+  LLVM_USED_FOR_DEBUG unsigned size() const { return getNumEntries(); }
 
   /// Grow the densemap so that it can contain at least \p NumEntries items
   /// before resizing again.
   void reserve(size_type NumEntries) {
     auto NumBuckets = getMinBucketToReserveForEntries(NumEntries);
     incrementEpoch();
     if (NumBuckets > getNumBuckets())
       grow(NumBuckets);
if (std::is_trivially_destructible<ValueT>::value) {
       }
       assert(NumEntries == 0 && "Node count imbalance!");
     }
     setNumEntries(0);
     setNumTombstones(0);
   }
 
   /// Return 1 if the specified key is in the map, 0 otherwise.
-  size_type count(const_arg_type_t<KeyT> Val) const {
+  LLVM_USED_FOR_DEBUG size_type count(const_arg_type_t<KeyT> Val) const {
     const BucketT *TheBucket;
     return LookupBucketFor(Val, TheBucket) ? 1 : 0;
   }
 
-  iterator find(const_arg_type_t<KeyT> Val) {
+  LLVM_USED_FOR_DEBUG iterator find(const_arg_type_t<KeyT> Val) {
     BucketT *TheBucket;
     if (LookupBucketFor(Val, TheBucket))
       return makeIterator(TheBucket, getBucketsEnd(), *this, true);
     return end();
   }
-  const_iterator find(const_arg_type_t<KeyT> Val) const {
+  LLVM_USED_FOR_DEBUG const_iterator find(const_arg_type_t<KeyT> Val) const {
     const BucketT *TheBucket;
     if (LookupBucketFor(Val, TheBucket))
       return makeConstIterator(TheBucket, getBucketsEnd(), *this, true);
     return end();
   }
 
   /// Alternate version of find() which allows a different, and possibly
   /// less expensive, key type.
const_iterator find_as(const LookupKeyT &Val) const {
     const BucketT *TheBucket;
     if (LookupBucketFor(Val, TheBucket))
       return makeConstIterator(TheBucket, getBucketsEnd(), *this, true);
     return end();
   }
 
   /// lookup - Return the entry for the specified key, or a default
   /// constructed value if no such entry exists.
-  ValueT lookup(const_arg_type_t<KeyT> Val) const {
+  LLVM_USED_FOR_DEBUG ValueT lookup(const_arg_type_t<KeyT> Val) const {
     const BucketT *TheBucket;
     if (LookupBucketFor(Val, TheBucket))
       return TheBucket->getSecond();
     return ValueT();
   }
 
   // Inserts key,value pair into the map if the key isn't already in the map.
   // If the key is already in the map, it returns false and doesn't update the
public:
   // for const iterator destinations so it doesn't end up as a user defined copy
   // constructor.
   template <bool IsConstSrc,
             typename = std::enable_if_t<!IsConstSrc && IsConst>>
   DenseMapIterator(
       const DenseMapIterator<KeyT, ValueT, KeyInfoT, Bucket, IsConstSrc> &I)
       : DebugEpochBase::HandleBase(I), Ptr(I.Ptr), End(I.End) {}
 
-  reference operator*() const {
+  LLVM_USED_FOR_DEBUG reference operator*() const {
     assert(isHandleInSync() && "invalid iterator access!");
     assert(Ptr != End && "dereferencing end() iterator");
     if (shouldReverseIterate<KeyT>())
       return Ptr[-1];
     return *Ptr;
   }
-  pointer operator->() const {
+  LLVM_USED_FOR_DEBUG pointer operator->() const {
     assert(isHandleInSync() && "invalid iterator access!");
     assert(Ptr != End && "dereferencing end() iterator");
     if (shouldReverseIterate<KeyT>())
       return &(Ptr[-1]);
     return Ptr;
   }
 
-  bool operator==(const ConstIterator &RHS) const {
+  LLVM_USED_FOR_DEBUG bool operator==(const ConstIterator &RHS) const {
     assert((!Ptr || isHandleInSync()) && "handle not in sync!");
     assert((!RHS.Ptr || RHS.isHandleInSync()) && "handle not in sync!");
     assert(getEpochAddress() == RHS.getEpochAddress() &&
            "comparing incomparable iterators!");
     return Ptr == RHS.Ptr;
   }
-  bool operator!=(const ConstIterator &RHS) const {
+  LLVM_USED_FOR_DEBUG bool operator!=(const ConstIterator &RHS) const {
     assert((!Ptr || isHandleInSync()) && "handle not in sync!");
     assert((!RHS.Ptr || RHS.isHandleInSync()) && "handle not in sync!");
     assert(getEpochAddress() == RHS.getEpochAddress() &&
            "comparing incomparable iterators!");
     return Ptr != RHS.Ptr;
   }
 
   inline DenseMapIterator& operator++() {  // Preincrement

llvm/include/llvm/ADT/PointerIntPair.h

public:
   constexpr PointerIntPair() = default;
 
   PointerIntPair(PointerTy PtrVal, IntType IntVal) {
     setPointerAndInt(PtrVal, IntVal);
   }
 
   explicit PointerIntPair(PointerTy PtrVal) { initWithPointer(PtrVal); }
 
-  PointerTy getPointer() const { return Info::getPointer(Value); }
+  LLVM_USED_FOR_DEBUG PointerTy getPointer() const {
+    return Info::getPointer(Value);
+  }
 
-  IntType getInt() const { return (IntType)Info::getInt(Value); }
+  LLVM_USED_FOR_DEBUG IntType getInt() const {
+    return (IntType)Info::getInt(Value);
+  }
 
   void setPointer(PointerTy PtrVal) LLVM_LVALUE_FUNCTION {
     Value = Info::updatePointer(Value, PtrVal);
   }
 
   void setInt(IntType IntVal) LLVM_LVALUE_FUNCTION {
     Value = Info::updateInt(Value, static_cast<intptr_t>(IntVal));
   }

llvm/include/llvm/ADT/SmallVector.h

protected:
   SmallVectorBase(void *FirstEl, size_t TotalCapacity)
       : BeginX(FirstEl), Capacity(TotalCapacity) {}
 
   /// This is an implementation of the grow() method which only works
   /// on POD-like data types and is out of line to reduce code duplication.
   void grow_pod(void *FirstEl, size_t MinCapacity, size_t TSize);
 
 public:
-  size_t size() const { return Size; }
+  LLVM_USED_FOR_DEBUG size_t size() const { return Size; }
   size_t capacity() const { return Capacity; }
 
   LLVM_NODISCARD bool empty() const { return !Size; }
 
   /// Set the array size to \p N, which the current array must have enough
   /// capacity for.
   ///
   /// This does not construct or destroy any elements in the vector.
public:
 
   size_t capacity_in_bytes() const { return capacity() * sizeof(T); }
 
   /// Return a pointer to the vector's buffer, even if empty().
   pointer data() { return pointer(begin()); }
   /// Return a pointer to the vector's buffer, even if empty().
   const_pointer data() const { return const_pointer(begin()); }
 
-  reference operator[](size_type idx) {
+  LLVM_USED_FOR_DEBUG reference operator[](size_type idx) {
     assert(idx < size());
     return begin()[idx];
   }
-  const_reference operator[](size_type idx) const {
+  LLVM_USED_FOR_DEBUG const_reference operator[](size_type idx) const {
     assert(idx < size());
     return begin()[idx];
   }
 
   reference front() {
     assert(!empty());
     return begin()[0];
   }

llvm/include/llvm/Support/Compiler.h

 
 /// Mark debug helper function definitions like dump() that should not be
 /// stripped from debug builds.
 /// Note that you should also surround dump() functions with
 /// `#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)` so they do always
 /// get stripped in release builds.
 // FIXME: Move this to a private config.h as it's not usable in public headers.
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 #define LLVM_DUMP_METHOD LLVM_ATTRIBUTE_NOINLINE LLVM_ATTRIBUTE_USED
 #else
 #define LLVM_DUMP_METHOD LLVM_ATTRIBUTE_NOINLINE
 #endif
 
+/// This macro will mark the function as used if the host compiler supports it
+/// and the build has asserts enabled. Functions marked with this macro can be
+/// called from the debugger during debugging. This macro should be used on
+/// function that are useful for debugging and wouldn't be emitted unless they
+/// are used, usually getter-like member function of template classes. Example:
+/// SmallVectors's operator[] is usefull for debugging, doesn't change the state
+/// of the SmallVector and will not be generated it is used because it is
+/// templated.
+#if !defined(NDEBUG)
+#define LLVM_USED_FOR_DEBUG LLVM_ATTRIBUTE_USED
+#else
+#define LLVM_USED_FOR_DEBUG
+#endif
+
 /// \macro LLVM_PRETTY_FUNCTION
 /// Gets a user-friendly looking function signature for the current scope
 /// using the best available method on each platform.  The exact format of the
 /// resulting string is implementation specific and non-portable, so this should
 /// only be used, for example, for logging or diagnostics.
 #if defined(_MSC_VER)
 #define LLVM_PRETTY_FUNCTION __FUNCSIG__
 #elif defined(__GNUC__) || defined(__clang__)