[PATCH] D26482: Prevent at compile time converting from Error::success() to Expected<T>

[Mehdi AMINI via llvm-commits]

mehdi_amini created this revision.
mehdi_amini added a reviewer: lhames.
mehdi_amini added a subscriber: llvm-commits.

This would trigger an assertion at runtime otherwise.


https://reviews.llvm.org/D26482

Files:
  llvm/include/llvm/Support/Error.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp


Index: llvm/lib/Bitcode/Reader/BitcodeReader.cpp
===================================================================
--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -4318,7 +4318,7 @@
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
-      return Error::success();
+      return "";
 
     case BitstreamEntry::SubBlock:
       if (Entry.ID == bitc::MODULE_BLOCK_ID)
@@ -4352,7 +4352,7 @@
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
-      return Error::success();
+      return "";
 
     case BitstreamEntry::SubBlock:
       if (Entry.ID == bitc::IDENTIFICATION_BLOCK_ID) {
@@ -4403,7 +4403,7 @@
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
-      return Error::success();
+      return false;
 
     case BitstreamEntry::SubBlock:
       if (Entry.ID == bitc::MODULE_BLOCK_ID)
Index: llvm/include/llvm/Support/Error.h
===================================================================
--- llvm/include/llvm/Support/Error.h
+++ llvm/include/llvm/Support/Error.h
@@ -27,6 +27,7 @@
 
 class Error;
 class ErrorList;
+class ErrorSuccess;
 
 /// Base class for error info classes. Do not extend this directly: Extend
 /// the ErrorInfo template subclass instead.
@@ -159,7 +160,7 @@
 public:
   /// Create a success value. This is equivalent to calling the default
   /// constructor, but should be preferred for readability where possible.
-  static Error success() { return Error(); }
+  static ErrorSuccess success();
 
   // Errors are not copy-constructable.
   Error(const Error &Other) = delete;
@@ -279,6 +280,13 @@
   ErrorInfoBase *Payload;
 };
 
+/// Subclass of Error for the sole purpose of identifying the success path in
+/// the type system. This allows to catch invalid conversion to Expected<T> at
+/// compile time.
+class ErrorSuccess : public Error {};
+
+inline ErrorSuccess Error::success() { return ErrorSuccess(); }
+
 /// Make a Error instance representing failure using the given error info
 /// type.
 template <typename ErrT, typename... ArgTs> Error make_error(ArgTs &&... Args) {
@@ -645,6 +653,11 @@
     new (getErrorStorage()) error_type(Err.takePayload());
   }
 
+  /// Forbid to convert from Error::success() implicitly, this avoids having
+  /// Expected<T> foo() { return Error::success(); } which compiles otherwise
+  /// but triggers the assertion above.
+  Expected(ErrorSuccess) = delete;
+
   /// Create an Expected<T> success value from the given OtherT value, which
   /// must be convertible to T.
   template <typename OtherT>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D26482.77417.patch
Type: text/x-patch
Size: 2712 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161110/fe927da9/attachment.bin>