[llvm] r336888 - [Support] Require llvm::Error passed to formatv() to be wrapped in fmt_consume()
Sam McCall via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 12 00:11:29 PDT 2018
Author: sammccall
Date: Thu Jul 12 00:11:28 2018
New Revision: 336888
URL: http://llvm.org/viewvc/llvm-project?rev=336888&view=rev
Log:
[Support] Require llvm::Error passed to formatv() to be wrapped in fmt_consume()
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.
Reviewers: zturner
Subscribers: llvm-commits, lhames
Differential Revision: https://reviews.llvm.org/D49170
Modified:
llvm/trunk/include/llvm/Support/FormatAdapters.h
llvm/trunk/include/llvm/Support/FormatVariadicDetails.h
llvm/trunk/unittests/Support/FormatVariadicTest.cpp
Modified: llvm/trunk/include/llvm/Support/FormatAdapters.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FormatAdapters.h?rev=336888&r1=336887&r2=336888&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FormatAdapters.h (original)
+++ llvm/trunk/include/llvm/Support/FormatAdapters.h Thu Jul 12 00:11:28 2018
@@ -12,6 +12,7 @@
#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"
@@ -19,7 +20,7 @@
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 @@ public:
}
}
};
+
+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 @@ 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
Modified: llvm/trunk/include/llvm/Support/FormatVariadicDetails.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FormatVariadicDetails.h?rev=336888&r1=336887&r2=336888&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FormatVariadicDetails.h (original)
+++ llvm/trunk/include/llvm/Support/FormatVariadicDetails.h Thu Jul 12 00:11:28 2018
@@ -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 @@ 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));
}
Modified: llvm/trunk/unittests/Support/FormatVariadicTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/FormatVariadicTest.cpp?rev=336888&r1=336887&r2=336888&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/FormatVariadicTest.cpp (original)
+++ llvm/trunk/unittests/Support/FormatVariadicTest.cpp Thu Jul 12 00:11:28 2018
@@ -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 @@ TEST(FormatVariadicTest, FormatStreamabl
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
+}
More information about the llvm-commits
mailing list