feat(common): add Options::set<>() && overload

google/cloud/options.h

@@ -108,8 +108,18 @@ class Options {
     std::swap(m_, tmp.m_);
     return *this;
   }
-  Options(Options&&) = default;
-  Options& operator=(Options&&) = default;
+  Options(Options&& rhs) noexcept = default;
+
+  // Older versions of GCC (or maybe libstdc++) crash with the default move
+  // assignment:
+  //     https://godbolt.org/z/j4EKjdrv4
+  // I suspect this is the problem addressed by:
+  //     https://github.com/gcc-mirror/gcc/commit/c2fb0a1a2e7a0fb15cf3cf876f621902ccd273f0
+  Options& operator=(Options&& rhs) noexcept {
+    Options tmp(std::move(rhs));
+    std::swap(m_, tmp.m_);
+    return *this;
+  }
 
   /**
    * Sets option `T` to the value @p v and returns a reference to `*this`.
@@ -118,18 +128,38 @@ class Options {
    * struct FooOption {
    *   using Type = int;
    * };
-   * auto opts = Options{}.set<FooOption>(123);
+   * auto opts = Options{};
+   * opts.set<FooOption>(123);
    * @endcode
    *
    * @tparam T the option type
    * @param v the value to set the option T
    */
   template <typename T>
-  Options& set(ValueTypeT<T> v) {
+  Options& set(ValueTypeT<T> v) & {
     m_[typeid(T)] = std::make_unique<Data<T>>(std::move(v));
     return *this;
   }
 
+  /**
+   * Sets option `T` to the value @p v and returns a reference to `*this`.
+   *
+   * @code
+   * struct FooOption {
+   *   using Type = int;
+   * };
+   * auto opts = Options{}.set<FooOption>(123);
+   * @endcode
+   *
+   * @tparam T the option type
+   * @param v the value to set the option T
+   */
+  template <typename T>
+  Options&& set(ValueTypeT<T> v) && {
+    set<T>(std::move(v));
+    return std::move(*this);
+  }
+
   /**
    * Returns true IFF an option with type `T` exists.
    *

google/cloud/options_benchmark.cc

@@ -27,15 +27,17 @@ namespace {
 //   L1 Instruction 32 KiB (x64)
 //   L2 Unified 512 KiB (x64)
 //   L3 Unified 16384 KiB (x16)
-// Load Average: 1.26, 1.39, 2.94
+// Load Average: 2.20, 1.69, 2.13
 // --------------------------------------------------------------------------
 // Benchmark                                Time             CPU   Iterations
 // --------------------------------------------------------------------------
-// BM_OptionsOneElementDefault           18.4 ns         18.4 ns     38006008
-// BM_OptionsOneElementPresent           45.8 ns         45.8 ns     15266572
-// BM_SimulateRpc                       10721 ns        10721 ns        64131
-// BM_SimulateStreamingRpc             891912 ns       891888 ns          774
-// BM_SimulateStreamingRpcWithSave      12547 ns        12547 ns        54804
+// BM_OptionsOneElementDefault           17.6 ns         17.6 ns     39113305
+// BM_OptionsOneElementPresent           43.2 ns         43.2 ns     16252491
+// BM_SetOnTemporary                     9975 ns         9975 ns        70792
+// BM_SetOnRef                          18376 ns        18376 ns        37871
+// BM_SimulateRpc                       10422 ns        10422 ns        67269
+// BM_SimulateStreamingRpc             866456 ns       866442 ns          809
+// BM_SimulateStreamingRpcWithSave      12277 ns        12276 ns        57041
 
 struct StringOptionDefault {
   using Type = std::string;
@@ -96,6 +98,27 @@ struct ReadAllOptions {
 };
 
 auto constexpr kOptionCount = 64;
+
+std::string ConsumeOptions(Options o) {  // NOLINT
+  return o.get<StringOptionDefault>();
+}
+
+void BM_SetOnTemporary(benchmark::State& state) {
+  for (auto _ : state) {
+    benchmark::DoNotOptimize(ConsumeOptions(
+        PopulateOptions<kOptionCount>{}().set<TestOption<0>>(42)));
+  }
+}
+BENCHMARK(BM_SetOnTemporary);
+
+void BM_SetOnRef(benchmark::State& state) {
+  for (auto _ : state) {
+    auto opts = PopulateOptions<kOptionCount>{}();
+    benchmark::DoNotOptimize(ConsumeOptions(opts.set<TestOption<0>>(42)));
+  }
+}
+BENCHMARK(BM_SetOnRef);
+
 auto constexpr kMessageCount = 100;
 
 std::string SimulateRpc(Options overrides, Options const& client) {

google/cloud/options_test.cc

@@ -98,6 +98,26 @@ TEST(Options, Set) {
   EXPECT_EQ("foo", opts.get<StringOption>());
 }
 
+TEST(Options, SetRefRef) {
+  auto opts = Options{}.set<IntOption>({});
+  EXPECT_TRUE(opts.has<IntOption>());
+  EXPECT_EQ(0, opts.get<IntOption>());
+  opts = std::move(opts).set<IntOption>(123);
+  EXPECT_EQ(123, opts.get<IntOption>());
+
+  opts = Options{}.set<BoolOption>({});
+  EXPECT_TRUE(opts.has<BoolOption>());
+  EXPECT_EQ(false, opts.get<BoolOption>());
+  opts = std::move(opts).set<BoolOption>(true);
+  EXPECT_EQ(true, opts.get<BoolOption>());
+
+  opts = Options{}.set<StringOption>({});
+  EXPECT_TRUE(opts.has<StringOption>());
+  EXPECT_EQ("", opts.get<StringOption>());
+  opts = std::move(opts).set<StringOption>("foo");
+  EXPECT_EQ("foo", opts.get<StringOption>());
+}
+
 TEST(Options, Get) {
   Options opts;