[PATCH] D48750: Use PointerIntPair in llvm::Error instead of hand-rolling it

[Jordan Rose via Phabricator via llvm-commits]

jordan_rose created this revision.
jordan_rose added a reviewer: lhames.

Also, tweak the application of LLVM_ENABLE_ABI_BREAKING_CHECKS so that a stray `setChecked(false)` call won't actually break ABI. And make sure the death tests don't run under NDEBUG.


Repository:
  rL LLVM

https://reviews.llvm.org/D48750

Files:
  include/llvm/Support/Error.h
  unittests/Support/ErrorTest.cpp


Index: unittests/Support/ErrorTest.cpp
===================================================================
--- unittests/Support/ErrorTest.cpp
+++ unittests/Support/ErrorTest.cpp
@@ -379,7 +379,7 @@
 
 // Test that handleAllUnhandledErrors crashes if an error is not caught.
 // Test runs in debug mode only.
-#if LLVM_ENABLE_ABI_BREAKING_CHECKS
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS && !NDEBUG
 TEST(Error, FailureToHandle) {
   auto FailToHandle = []() {
     handleAllErrors(make_error<CustomError>(7), [&](const CustomSubError &SE) {
@@ -398,7 +398,7 @@
 // Test that handleAllUnhandledErrors crashes if an error is returned from a
 // handler.
 // Test runs in debug mode only.
-#if LLVM_ENABLE_ABI_BREAKING_CHECKS
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS && !NDEBUG
 TEST(Error, FailureFromHandler) {
   auto ReturnErrorFromHandler = []() {
     handleAllErrors(make_error<CustomError>(7),
Index: include/llvm/Support/Error.h
===================================================================
--- include/llvm/Support/Error.h
+++ include/llvm/Support/Error.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_SUPPORT_ERROR_H
 #define LLVM_SUPPORT_ERROR_H
 
+#include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
@@ -264,35 +265,31 @@
   }
 
   ErrorInfoBase *getPtr() const {
-    return reinterpret_cast<ErrorInfoBase*>(
-             reinterpret_cast<uintptr_t>(Payload) &
-             ~static_cast<uintptr_t>(0x1));
+    return Payload.getPointer();
   }
 
   void setPtr(ErrorInfoBase *EI) {
 #if LLVM_ENABLE_ABI_BREAKING_CHECKS
-    Payload = reinterpret_cast<ErrorInfoBase*>(
-                (reinterpret_cast<uintptr_t>(EI) &
-                 ~static_cast<uintptr_t>(0x1)) |
-                (reinterpret_cast<uintptr_t>(Payload) & 0x1));
+    Payload.setPointer(EI);
 #else
-    Payload = EI;
+    // This #else isn't strictly necessary, but should be more optimizable.
+    Payload.setPointerAndInt(EI, false);
 #endif
   }
 
   bool getChecked() const {
 #if LLVM_ENABLE_ABI_BREAKING_CHECKS
-    return (reinterpret_cast<uintptr_t>(Payload) & 0x1) == 0;
+    return !Payload.getInt();
 #else
+    // This #else isn't strictly necessary, but should be more optimizable.
     return true;
 #endif
   }
 
   void setChecked(bool V) {
-    Payload = reinterpret_cast<ErrorInfoBase*>(
-                (reinterpret_cast<uintptr_t>(Payload) &
-                  ~static_cast<uintptr_t>(0x1)) |
-                  (V ? 0 : 1));
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
+    Payload.setInt(!V);
+#endif
   }
 
   std::unique_ptr<ErrorInfoBase> takePayload() {
@@ -302,7 +299,7 @@
     return Tmp;
   }
 
-  ErrorInfoBase *Payload = nullptr;
+  PointerIntPair<ErrorInfoBase *, 1, bool> Payload;
 };
 
 /// Subclass of Error for the sole purpose of identifying the success path in


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D48750.153409.patch
Type: text/x-patch
Size: 2851 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180628/ecf7dcbf/attachment.bin>