[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 llvm-svn: 336888
This commit is contained in:
parent
d3b69c6be9
commit
907bde1240
|
@ -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
|
||||
|
|
|
@ -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));
|
||||
}
|
||||
|
||||
|
|
|
@ -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, 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
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue