[PATCH] D49170: [Support] Require llvm::Error passed to formatv() to be wrapped in fmt_consume()

[Sam McCall via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL336888: [Support] Require llvm::Error passed to formatv() to be wrapped in fmt_consume() (authored by sammccall, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D49170

Files:
  llvm/trunk/include/llvm/Support/FormatAdapters.h
  llvm/trunk/include/llvm/Support/FormatVariadicDetails.h
  llvm/trunk/unittests/Support/FormatVariadicTest.cpp


Index: llvm/trunk/unittests/Support/FormatVariadicTest.cpp
===================================================================
--- llvm/trunk/unittests/Support/FormatVariadicTest.cpp
+++ llvm/trunk/unittests/Support/FormatVariadicTest.cpp
@@ -8,6 +8,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FormatAdapters.h"
 #include "gtest/gtest.h"
 
@@ -680,3 +681,11 @@
   adl::X X;
   EXPECT_EQ("X", formatv("{0}", X).str());
 }
+
+TEST(FormatVariadicTest, FormatError) {
+  auto E1 = make_error<StringError>("X", inconvertibleErrorCode());
+  EXPECT_EQ("X", formatv("{0}", E1).str());
+  EXPECT_TRUE(E1.isA<StringError>()); // not consumed
+  EXPECT_EQ("X", formatv("{0}", fmt_consume(std::move(E1))).str());
+  EXPECT_FALSE(E1.isA<StringError>()); // consumed
+}
Index: llvm/trunk/include/llvm/Support/FormatVariadicDetails.h
===================================================================
--- llvm/trunk/include/llvm/Support/FormatVariadicDetails.h
+++ llvm/trunk/include/llvm/Support/FormatVariadicDetails.h
@@ -17,6 +17,7 @@
 
 namespace llvm {
 template <typename T, typename Enable = void> struct format_provider {};
+class Error;
 
 namespace detail {
 class format_adapter {
@@ -141,6 +142,12 @@
 typename std::enable_if<uses_stream_operator<T>::value,
                         stream_operator_format_adapter<T>>::type
 build_format_adapter(T &&Item) {
+  // If the caller passed an Error by value, then stream_operator_format_adapter
+  // would be responsible for consuming it.
+  // Make the caller opt into this by calling fmt_consume().
+  static_assert(
+      !std::is_same<llvm::Error, typename std::remove_cv<T>::type>::value,
+      "llvm::Error-by-value must be wrapped in fmt_consume() for formatv");
   return stream_operator_format_adapter<T>(std::forward<T>(Item));
 }
 
Index: llvm/trunk/include/llvm/Support/FormatAdapters.h
===================================================================
--- llvm/trunk/include/llvm/Support/FormatAdapters.h
+++ llvm/trunk/include/llvm/Support/FormatAdapters.h
@@ -12,14 +12,15 @@
 
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FormatCommon.h"
 #include "llvm/Support/FormatVariadicDetails.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace llvm {
 template <typename T> class FormatAdapter : public detail::format_adapter {
 protected:
-  explicit FormatAdapter(T &&Item) : Item(Item) {}
+  explicit FormatAdapter(T &&Item) : Item(std::forward<T>(Item)) {}
 
   T Item;
 };
@@ -71,6 +72,14 @@
     }
   }
 };
+
+class ErrorAdapter : public FormatAdapter<Error> {
+public:
+  ErrorAdapter(Error &&Item) : FormatAdapter(std::move(Item)) {}
+  ErrorAdapter(ErrorAdapter &&) = default;
+  ~ErrorAdapter() { consumeError(std::move(Item)); }
+  void format(llvm::raw_ostream &Stream, StringRef Style) { Stream << Item; }
+};
 }
 
 template <typename T>
@@ -88,6 +97,13 @@
 detail::RepeatAdapter<T> fmt_repeat(T &&Item, size_t Count) {
   return detail::RepeatAdapter<T>(std::forward<T>(Item), Count);
 }
+
+// llvm::Error values must be consumed before being destroyed.
+// Wrapping an error in fmt_consume explicitly indicates that the formatv_object
+// should take ownership and consume it.
+inline detail::ErrorAdapter fmt_consume(Error &&Item) {
+  return detail::ErrorAdapter(std::move(Item));
+}
 }
 
 #endif


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D49170.155121.patch
Type: text/x-patch
Size: 3499 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180712/bf0a7070/attachment.bin>