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

[sammccall]

Summary

Someone must be responsible for handling an Error. When formatv takes
ownership of an Error, the formatv_object destructor must take care of this.

Passing an error by value to formatv() is not considered explicit enough to mark
the error as handled (see D49013), so we require callers to use a format adapter
to confirm this intent.

[sammccall]

Missing inline

include/llvm/Support/FormatAdapters.h

 //===- FormatAdapters.h - Formatters for common LLVM types -----*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_FORMATADAPTERS_H
 #define LLVM_SUPPORT_FORMATADAPTERS_H
 
 #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;
 };
 
 namespace detail {
 template <typename T> class AlignAdapter final : public FormatAdapter<T> {
   AlignStyle Where;
   size_t Amount;
public:
 
   void format(llvm::raw_ostream &Stream, StringRef Style) {
     auto Adapter = detail::build_format_adapter(std::forward<T>(this->Item));
     for (size_t I = 0; I < Count; ++I) {
       Adapter.format(Stream, Style);
     }
   }
 };
+
+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>
 detail::AlignAdapter<T> fmt_align(T &&Item, AlignStyle Where, size_t Amount,
                                   char Fill = ' ') {
   return detail::AlignAdapter<T>(std::forward<T>(Item), Where, Amount, Fill);
 }
 
 template <typename T>
 detail::PadAdapter<T> fmt_pad(T &&Item, size_t Left, size_t Right) {
   return detail::PadAdapter<T>(std::forward<T>(Item), Left, Right);
 }
 
 template <typename T>
 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

include/llvm/Support/FormatVariadicDetails.h

 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include <type_traits>
 
 namespace llvm {
 template <typename T, typename Enable = void> struct format_provider {};
+class Error;
 
 namespace detail {
 class format_adapter {
 protected:
   virtual ~format_adapter() {}
 
 public:
   virtual void format(raw_ostream &S, StringRef Options) = 0;

 build_format_adapter(T &&Item) {
   return provider_format_adapter<T>(std::forward<T>(Item));
 }
 
 template <typename T>
 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));
 }
 
 template <typename T>
 typename std::enable_if<uses_missing_provider<T>::value,
                         missing_format_adapter<T>>::type
 build_format_adapter(T &&Item) {
   return missing_format_adapter<T>();
 }
 }
 }
 
 #endif

unittests/Support/FormatVariadicTest.cpp

 //===- FormatVariadicTest.cpp - Unit tests for string formatting ----------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FormatAdapters.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 
 // Compile-time tests templates in the detail namespace.
 namespace {
 struct Format : public FormatAdapter<int> {

 namespace adl {
 struct X {};
 raw_ostream &operator<<(raw_ostream &OS, const X &) { return OS << "X"; }
 } // namespace adl
 TEST(FormatVariadicTest, FormatStreamable) {
   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
+}