-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix case of variant which is valueless by exception #3347
Changes from 3 commits
d671bf5
de942e7
e1c72c4
0a328c0
0b52517
00d3c6a
fbfb08e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
|
||
#include "fmt/std.h" | ||
|
||
#include <stdexcept> | ||
#include <string> | ||
#include <vector> | ||
|
||
|
@@ -95,6 +96,28 @@ TEST(std_test, optional) { | |
#endif | ||
} | ||
|
||
// this struct exists only to force a variant to be | ||
// valueless by exception | ||
// NOLINTNEXTLINE | ||
struct throws_on_move_t { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
throws_on_move_t() = default; | ||
|
||
// NOLINTNEXTLINE | ||
[[noreturn]] throws_on_move_t(throws_on_move_t&&) { | ||
throw std::runtime_error{"Thrown by throws_on_move_t"}; | ||
} | ||
|
||
throws_on_move_t(const throws_on_move_t&) = default; | ||
}; | ||
|
||
template <> struct fmt::formatter<throws_on_move_t> : formatter<string_view> { | ||
template <typename FormatContext> | ||
auto format(const throws_on_move_t&, FormatContext& ctx) const { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't need to be a template, you can use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
string_view str{"<throws_on_move_t>"}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't use brace initialization except for initializer lists, let's replace with normal initialization. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My apologies, fixed. |
||
return formatter<string_view>::format(str, ctx); | ||
} | ||
}; | ||
|
||
TEST(std_test, variant) { | ||
#ifdef __cpp_lib_variant | ||
EXPECT_EQ(fmt::format("{}", std::monostate{}), "monostate"); | ||
|
@@ -126,6 +149,20 @@ TEST(std_test, variant) { | |
|
||
volatile int i = 42; // Test compile error before GCC 11 described in #3068. | ||
EXPECT_EQ(fmt::format("{}", i), "42"); | ||
|
||
using V2 = std::variant<std::monostate, throws_on_move_t>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This alias is not needed, just use the variant type below. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
|
||
V2 v6{}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
|
||
try { | ||
throws_on_move_t thrower{}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
v6.emplace<throws_on_move_t>(std::move(thrower)); | ||
} catch (const std::runtime_error&) { | ||
} | ||
// v6 is now valueless by exception | ||
|
||
EXPECT_EQ(fmt::format("{}", v6), "variant(valueless by exception)"); | ||
|
||
#endif | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The purpose of the
NOLINTNEXTLINE
is to suppress clang-tidy warnings concerning thethrows_on_move
struct which is guaranteed to throw on move construction. I have removed it.