[ADT] Fix PointerEmbeddedInt when the underlying type is uintptr_t.

[jordan_rose]

Summary

...and when you try to store negative values in it. Seems pretty straightforward.

(Background: I'm replacing uses of Swift's similar type Fixnum with PointerEmbeddedInt.)

[chandlerc]

include/llvm/ADT/PointerEmbeddedInt.h

48–50

Rather than this, use an empty struct? That's a better tag type as it is literally not passed in the ABI, just changes the overload resolution at compile time.

The _t suffix is also not terribly common in LLVM. I'd go with 'RawValueTag' personally.

64–65

These isInt and isUInt predicates are... really confusingly named. "Of course its an integer! Oh, it's asserting how many bits are in the integer..."

Anyways, if you feel like it, maybe a follow-up patch to name these predicates better? Definitely not relevant for this patch.

unittests/ADT/PointerEmbeddedIntTest.cpp

53–54

Rather than all of the scopes, just use different names for the different types? Makes it much easier to read IMO.

[jordan_rose]

include/llvm/ADT/PointerEmbeddedInt.h

48–50

Good point. I was trying to avoid the weirdness of creating a nonce value, but that's doable with a constexpr member.

[jordan_rose]

Updated to address feedback.

[chandlerc]

LGTM

include/llvm/ADT/PointerEmbeddedInt.h

48–50

Watch for buildbot complaints about this constexpr. If necessary, I'd rather the good calling convention and a nonce value with a definition in the .cpp file than using enums. But hopefully this just works. =]

[jordan_rose]

Committed in r261259 with a slightly more conservative RawValue definition. Will be watching bots.

[jordan_rose]

Scaled back even further on being clever in r261268.

include/llvm/ADT/PointerEmbeddedInt.h

 //===- llvm/ADT/PointerEmbeddedInt.h ----------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_ADT_POINTEREMBEDDEDINT_H
 #define LLVM_ADT_POINTEREMBEDDEDINT_H
 
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/Support/MathExtras.h"
 #include "llvm/Support/PointerLikeTypeTraits.h"
 #include <climits>
 
 namespace llvm {
 
 /// Utility to embed an integer into a pointer-like type. This is specifically
 /// intended to allow embedding integers where fewer bits are required than
 /// exist in a pointer, and the integer can participate in abstractions along
 /// side other pointer-like types. For example it can be placed into a \c
 /// PointerSumType or \c PointerUnion.
 ///
 /// Note that much like pointers, an integer value of zero has special utility
 /// due to boolean conversions. For example, a non-null value can be tested for
 /// in the above abstractions without testing the particular active member.
 /// Also, the default constructed value zero initializes the integer.
 template <typename IntT, int Bits = sizeof(IntT) * CHAR_BIT>
 class PointerEmbeddedInt {
   uintptr_t Value;
 
+  // Note: This '<' is correct; using '<=' would result in some shifts
+  // overflowing their storage types.
   static_assert(Bits < sizeof(uintptr_t) * CHAR_BIT,
                 "Cannot embed more bits than we have in a pointer!");
 
   enum : uintptr_t {
     // We shift as many zeros into the value as we can while preserving the
     // number of bits desired for the integer.
     Shift = sizeof(uintptr_t) * CHAR_BIT - Bits,
 
     // We also want to be able to mask out the preserved bits for asserts.
     Mask = static_cast<uintptr_t>(-1) << Bits
   };
 
+  enum RawValue_t {
+    RawValue
+  };
+
   friend class PointerLikeTypeTraits<PointerEmbeddedInt>;
 
-  explicit PointerEmbeddedInt(uintptr_t Value) : Value(Value) {}
+  explicit PointerEmbeddedInt(uintptr_t Value, RawValue_t) : Value(Value) {}
 
 public:
   PointerEmbeddedInt() : Value(0) {}
 
-  PointerEmbeddedInt(IntT I) : Value(static_cast<uintptr_t>(I) << Shift) {
-    assert((I & Mask) == 0 && "Integer has bits outside those preserved!");
+  PointerEmbeddedInt(IntT I) {
+    *this = I;
   }
 
   PointerEmbeddedInt &operator=(IntT I) {
-    assert((I & Mask) == 0 && "Integer has bits outside those preserved!");
+    assert((std::is_signed<IntT>::value ? llvm::isInt<Bits>(I)
+                                        : llvm::isUInt<Bits>(I)) && 
+           "Integer has bits outside those preserved!");
     Value = static_cast<uintptr_t>(I) << Shift;
     return *this;
   }
 
   // Note that this implicit conversion additionally allows all of the basic
   // comparison operators to work transparently, etc.
-  operator IntT() const { return static_cast<IntT>(Value >> Shift); }
+  operator IntT() const {
+    if (std::is_signed<IntT>::value)
+      return static_cast<IntT>(static_cast<intptr_t>(Value) >> Shift);
+    return static_cast<IntT>(Value >> Shift);
+  }
 };
 
 // Provide pointer like traits to support use with pointer unions and sum
 // types.
 template <typename IntT, int Bits>
 class PointerLikeTypeTraits<PointerEmbeddedInt<IntT, Bits>> {
   typedef PointerEmbeddedInt<IntT, Bits> T;
 
 public:
   static inline void *getAsVoidPointer(const T &P) {
     return reinterpret_cast<void *>(P.Value);
   }
   static inline T getFromVoidPointer(void *P) {
-    return T(reinterpret_cast<uintptr_t>(P));
+    return T(reinterpret_cast<uintptr_t>(P), T::RawValue);
   }
   static inline T getFromVoidPointer(const void *P) {
-    return T(reinterpret_cast<uintptr_t>(P));
+    return T(reinterpret_cast<uintptr_t>(P), T::RawValue);
   }
 
   enum { NumLowBitsAvailable = T::Shift };
 };
 
 // Teach DenseMap how to use PointerEmbeddedInt objects as keys if the Int type
 // itself can be a key.
 template <typename IntT, int Bits>

unittests/ADT/PointerEmbeddedIntTest.cpp

TEST(PointerEmbeddedIntTest, Basic) {
   EXPECT_FALSE(42 == J);
   EXPECT_TRUE(42 != J);
   EXPECT_TRUE(42 < J);
   EXPECT_FALSE(42 > J);
   EXPECT_TRUE(42 <= J);
   EXPECT_FALSE(42 >= J);
 }
 
+TEST(PointerEmbeddedIntTest, intptr_t) {
+  {
+    PointerEmbeddedInt<intptr_t, CHAR_BIT> I = 42, J = -42;
+    EXPECT_EQ(42, I);
+    EXPECT_EQ(-42, J);
+  }
+
+  {
+    PointerEmbeddedInt<uintptr_t, CHAR_BIT> I = 42, J = 255;
+    EXPECT_EQ(42U, I);
+    EXPECT_EQ(255U, J);
+  }
+
+  {
+    PointerEmbeddedInt<intptr_t, std::numeric_limits<intptr_t>::digits>
+        I = std::numeric_limits<intptr_t>::max() >> 1,
+        J = std::numeric_limits<intptr_t>::min() >> 1;
+    EXPECT_EQ(std::numeric_limits<intptr_t>::max() >> 1, I);
+    EXPECT_EQ(std::numeric_limits<intptr_t>::min() >> 1, J);
+  }
+
+  {
+    PointerEmbeddedInt<uintptr_t, std::numeric_limits<uintptr_t>::digits - 1>
+        I = std::numeric_limits<uintptr_t>::max() >> 1,
+        J = std::numeric_limits<uintptr_t>::min() >> 1;
+    EXPECT_EQ(std::numeric_limits<uintptr_t>::max() >> 1, I);
+    EXPECT_EQ(std::numeric_limits<uintptr_t>::min() >> 1, J);
+  }
+}
+
+TEST(PointerEmbeddedIntTest, PointerLikeTypeTraits) {
+  {
+    PointerEmbeddedInt<int, CHAR_BIT> I = 42;
+    using Traits = PointerLikeTypeTraits<decltype(I)>;
+    EXPECT_EQ(42, Traits::getFromVoidPointer(Traits::getAsVoidPointer(I)));
+  }
+
+  {
+    PointerEmbeddedInt<uintptr_t, std::numeric_limits<uintptr_t>::digits - 1>
+        I = std::numeric_limits<uintptr_t>::max() >> 1;
+    using Traits = PointerLikeTypeTraits<decltype(I)>;
+    EXPECT_EQ(std::numeric_limits<uintptr_t>::max() >> 1,
+              Traits::getFromVoidPointer(Traits::getAsVoidPointer(I)));
+  }
+}
+
 } // end anonymous namespace