Merge pull request #21691 from yashykt/optionalfix

Fix grpc_core::Optional

src/core/ext/filters/client_channel/lb_policy/xds/xds.cc

@@ -1886,7 +1886,7 @@ class XdsFactory : public LoadBalancingPolicyFactory {
     if (error_list.empty()) {
       Optional<std::string> optional_lrs_load_reporting_server_name;
       if (lrs_load_reporting_server_name != nullptr) {
-        optional_lrs_load_reporting_server_name.set(
+        optional_lrs_load_reporting_server_name.emplace(
             std::string(lrs_load_reporting_server_name));
       }
       return MakeRefCounted<ParsedXdsConfig>(

src/core/ext/filters/client_channel/resolver_result_parsing.cc

@@ -348,8 +348,8 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const grpc_json* json,
                 "field:retryThrottling field:maxTokens error:Type should be "
                 "number"));
           } else {
-            max_milli_tokens.set(gpr_parse_nonnegative_int(sub_field->value) *
-                                 1000);
+            max_milli_tokens.emplace(
+                gpr_parse_nonnegative_int(sub_field->value) * 1000);
             if (max_milli_tokens.value() <= 0) {
               error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
                   "field:retryThrottling field:maxTokens error:should be "
@@ -398,7 +398,7 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const grpc_json* json,
                   "parsing"));
               continue;
             }
-            milli_token_ratio.set(
+            milli_token_ratio.emplace(
                 static_cast<int>((whole_value * multiplier) + decimal_value));
             if (milli_token_ratio.value() <= 0) {
               error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
@@ -421,7 +421,7 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const grpc_json* json,
       } else {
         data.milli_token_ratio = milli_token_ratio.value();
       }
-      retry_throttling.set(data);
+      retry_throttling.emplace(data);
     }
     if (strcmp(field->key, "healthCheckConfig") == 0) {
       if (health_check_service_name != nullptr) {
@@ -461,9 +461,9 @@ ClientChannelServiceConfigParser::ParsePerMethodParams(const grpc_json* json,
             "field:waitForReady error:Duplicate entry"));
       }  // Duplicate, continue parsing.
       if (field->type == GRPC_JSON_TRUE) {
-        wait_for_ready.set(true);
+        wait_for_ready.emplace(true);
       } else if (field->type == GRPC_JSON_FALSE) {
-        wait_for_ready.set(false);
+        wait_for_ready.emplace(false);
       } else {
         error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
             "field:waitForReady error:Type should be true/false"));

src/core/ext/filters/client_channel/xds/xds_api.cc

@@ -397,7 +397,7 @@ grpc_error* CdsResponseParse(const envoy_api_v2_DiscoveryResponse* response,
         return GRPC_ERROR_CREATE_FROM_STATIC_STRING(
             "ConfigSource is not self.");
       }
-      cds_update.lrs_load_reporting_server_name.set("");
+      cds_update.lrs_load_reporting_server_name.emplace("");
     }
     upb_strview cluster_name = envoy_api_v2_Cluster_name(cluster);
     cds_update_map->emplace(std::string(cluster_name.data, cluster_name.size),

src/core/ext/filters/client_channel/xds/xds_client.cc

@@ -762,7 +762,7 @@ void XdsClient::ChannelState::AdsCallState::AcceptCdsUpdate(
       continue;
     }
     // Update the cluster state.
-    cluster_state.update.set(std::move(cds_update));
+    cluster_state.update.emplace(std::move(cds_update));
     // Notify all watchers.
     for (const auto& p : cluster_state.watchers) {
       p.first->OnClusterChanged(cluster_state.update.value());

src/core/lib/gprpp/optional.h

@@ -46,14 +46,11 @@ class Optional {
  public:
   Optional() : value_() {}
 
-  void set(const T& val) {
-    value_ = val;
-    set_ = true;
-  }
-
-  void set(T&& val) {
-    value_ = std::move(val);
+  template <typename... Args>
+  T& emplace(Args&&... args) {
+    value_ = T(std::forward<Args>(args)...);
     set_ = true;
+    return value_;
   }
 
   bool has_value() const { return set_; }

src/core/lib/iomgr/buffer_list.cc

@@ -66,27 +66,27 @@ void extract_opt_stats_from_tcp_info(ConnectionMetrics* metrics,
     return;
   }
   if (info->length > offsetof(grpc_core::tcp_info, tcpi_sndbuf_limited)) {
-    metrics->recurring_retrans.set(info->tcpi_retransmits);
-    metrics->is_delivery_rate_app_limited.set(
+    metrics->recurring_retrans.emplace(info->tcpi_retransmits);
+    metrics->is_delivery_rate_app_limited.emplace(
         info->tcpi_delivery_rate_app_limited);
-    metrics->congestion_window.set(info->tcpi_snd_cwnd);
-    metrics->reordering.set(info->tcpi_reordering);
-    metrics->packet_retx.set(info->tcpi_total_retrans);
-    metrics->pacing_rate.set(info->tcpi_pacing_rate);
-    metrics->data_notsent.set(info->tcpi_notsent_bytes);
+    metrics->congestion_window.emplace(info->tcpi_snd_cwnd);
+    metrics->reordering.emplace(info->tcpi_reordering);
+    metrics->packet_retx.emplace(info->tcpi_total_retrans);
+    metrics->pacing_rate.emplace(info->tcpi_pacing_rate);
+    metrics->data_notsent.emplace(info->tcpi_notsent_bytes);
     if (info->tcpi_min_rtt != UINT32_MAX) {
-      metrics->min_rtt.set(info->tcpi_min_rtt);
+      metrics->min_rtt.emplace(info->tcpi_min_rtt);
     }
-    metrics->packet_sent.set(info->tcpi_data_segs_out);
-    metrics->delivery_rate.set(info->tcpi_delivery_rate);
-    metrics->busy_usec.set(info->tcpi_busy_time);
-    metrics->rwnd_limited_usec.set(info->tcpi_rwnd_limited);
-    metrics->sndbuf_limited_usec.set(info->tcpi_sndbuf_limited);
+    metrics->packet_sent.emplace(info->tcpi_data_segs_out);
+    metrics->delivery_rate.emplace(info->tcpi_delivery_rate);
+    metrics->busy_usec.emplace(info->tcpi_busy_time);
+    metrics->rwnd_limited_usec.emplace(info->tcpi_rwnd_limited);
+    metrics->sndbuf_limited_usec.emplace(info->tcpi_sndbuf_limited);
   }
   if (info->length > offsetof(grpc_core::tcp_info, tcpi_dsack_dups)) {
-    metrics->data_sent.set(info->tcpi_bytes_sent);
-    metrics->data_retx.set(info->tcpi_bytes_retrans);
-    metrics->packet_spurious_retx.set(info->tcpi_dsack_dups);
+    metrics->data_sent.emplace(info->tcpi_bytes_sent);
+    metrics->data_retx.emplace(info->tcpi_bytes_retrans);
+    metrics->packet_spurious_retx.emplace(info->tcpi_dsack_dups);
   }
 }
 
@@ -107,79 +107,80 @@ void extract_opt_stats_from_cmsg(ConnectionMetrics* metrics,
     const void* val = data + offset + NLA_HDRLEN;
     switch (attr->nla_type) {
       case TCP_NLA_BUSY: {
-        metrics->busy_usec.set(read_unaligned<uint64_t>(val));
+        metrics->busy_usec.emplace(read_unaligned<uint64_t>(val));
         break;
       }
       case TCP_NLA_RWND_LIMITED: {
-        metrics->rwnd_limited_usec.set(read_unaligned<uint64_t>(val));
+        metrics->rwnd_limited_usec.emplace(read_unaligned<uint64_t>(val));
         break;
       }
       case TCP_NLA_SNDBUF_LIMITED: {
-        metrics->sndbuf_limited_usec.set(read_unaligned<uint64_t>(val));
+        metrics->sndbuf_limited_usec.emplace(read_unaligned<uint64_t>(val));
         break;
       }
       case TCP_NLA_PACING_RATE: {
-        metrics->pacing_rate.set(read_unaligned<uint64_t>(val));
+        metrics->pacing_rate.emplace(read_unaligned<uint64_t>(val));
         break;
       }
       case TCP_NLA_DELIVERY_RATE: {
-        metrics->delivery_rate.set(read_unaligned<uint64_t>(val));
+        metrics->delivery_rate.emplace(read_unaligned<uint64_t>(val));
         break;
       }
       case TCP_NLA_DELIVERY_RATE_APP_LMT: {
-        metrics->is_delivery_rate_app_limited.set(read_unaligned<uint8_t>(val));
+        metrics->is_delivery_rate_app_limited.emplace(
+            read_unaligned<uint8_t>(val));
         break;
       }
       case TCP_NLA_SND_CWND: {
-        metrics->congestion_window.set(read_unaligned<uint32_t>(val));
+        metrics->congestion_window.emplace(read_unaligned<uint32_t>(val));
         break;
       }
       case TCP_NLA_MIN_RTT: {
-        metrics->min_rtt.set(read_unaligned<uint32_t>(val));
+        metrics->min_rtt.emplace(read_unaligned<uint32_t>(val));
         break;
       }
       case TCP_NLA_SRTT: {
-        metrics->srtt.set(read_unaligned<uint32_t>(val));
+        metrics->srtt.emplace(read_unaligned<uint32_t>(val));
         break;
       }
       case TCP_NLA_RECUR_RETRANS: {
-        metrics->recurring_retrans.set(read_unaligned<uint8_t>(val));
+        metrics->recurring_retrans.emplace(read_unaligned<uint8_t>(val));
         break;
       }
       case TCP_NLA_BYTES_SENT: {
-        metrics->data_sent.set(read_unaligned<uint64_t>(val));
+        metrics->data_sent.emplace(read_unaligned<uint64_t>(val));
         break;
       }
       case TCP_NLA_DATA_SEGS_OUT: {
-        metrics->packet_sent.set(read_unaligned<uint64_t>(val));
+        metrics->packet_sent.emplace(read_unaligned<uint64_t>(val));
         break;
       }
       case TCP_NLA_TOTAL_RETRANS: {
-        metrics->packet_retx.set(read_unaligned<uint64_t>(val));
+        metrics->packet_retx.emplace(read_unaligned<uint64_t>(val));
         break;
       }
       case TCP_NLA_DELIVERED: {
-        metrics->packet_delivered.set(read_unaligned<uint32_t>(val));
+        metrics->packet_delivered.emplace(read_unaligned<uint32_t>(val));
         break;
       }
       case TCP_NLA_DELIVERED_CE: {
-        metrics->packet_delivered_ce.set(read_unaligned<uint32_t>(val));
+        metrics->packet_delivered_ce.emplace(read_unaligned<uint32_t>(val));
         break;
       }
       case TCP_NLA_BYTES_RETRANS: {
-        metrics->data_retx.set(read_unaligned<uint64_t>(val));
+        metrics->data_retx.emplace(read_unaligned<uint64_t>(val));
         break;
       }
       case TCP_NLA_DSACK_DUPS: {
-        metrics->packet_spurious_retx.set(read_unaligned<uint32_t>(val));
+        metrics->packet_spurious_retx.emplace(read_unaligned<uint32_t>(val));
         break;
       }
       case TCP_NLA_REORDERING: {
-        metrics->reordering.set(read_unaligned<uint32_t>(val));
+        metrics->reordering.emplace(read_unaligned<uint32_t>(val));
         break;
       }
       case TCP_NLA_SND_SSTHRESH: {
-        metrics->snd_ssthresh.set(read_unaligned<uint32_t>(val));
+        metrics->snd_ssthresh.emplace(read_unaligned<uint32_t>(val));
         break;
       }
     }

test/core/gprpp/optional_test.cc

@@ -31,7 +31,7 @@ TEST(OptionalTest, BasicTest) {
   EXPECT_FALSE(opt_val.has_value());
   const int kTestVal = 123;
 
-  opt_val.set(kTestVal);
+  opt_val.emplace(kTestVal);
   EXPECT_TRUE(opt_val.has_value());
   EXPECT_EQ(opt_val.value(), kTestVal);