[CodeHealth] Replace absl::optional with std::* (#21156)
This change replaces all uses of `absl::optional` with `std::*` variants
for optional. This is in line with upstream recent changes making
`absl::optional` a `typedef` to the `std` type.
This change has been done using an automated script:
#!/bin/bash
function replace {
echo "Replacing $1 by $2"
git grep -l "$1" \
| cut -f1 -d: \
| sort \
| uniq \
| grep \
-e "\.h" \
-e "\.cc" \
-e "\.mm" \
-e "\.py" \
| xargs sed -i "s/$1/$2/g"
}
function delete_line_with {
echo "Deleting lines with $1"
git grep -l "$1" \
| cut -f1 -d: \
| sort \
| uniq \
| grep \
-e "\.h" \
-e "\.cc" \
-e "\.mm" \
-e "\.py" \
| xargs sed -i "/$1/d"
}
function add_header {
echo "Adding header $1"
git diff --name-only HEAD \
| xargs ../tools/add_header.py --header "$1"
}
replace "absl::make_optional" "std::make_optional"
replace "absl::optional" "std::optional"
replace "absl::nullopt" "std::nullopt"
replace "absl::in_place" "std::in_place"
replace "absl::in_place_t" "std::in_place_t"
add_header "<optional>"
delete_line_with "\"third_party\/abseil-cpp\/absl\/types\/optional.h\""
git cl format
Chromium change:
https://chromium.googlesource.com/chromium/src/+/d9d21aa16829a7d471a4f3b3a493a31170ed8271
commit d9d21aa16829a7d471a4f3b3a493a31170ed8271
Author: David Benjamin <davidben@chromium.org>
Date: Mon Oct 2 23:29:57 2023 +0000
Make absl::optional a typedef for std::optional
This only changes the types around. It doesn't rewrite existing uses
to std::optional, which we can do incrementally.
absl::optional to std::optional seems to have two visible impacts.
First, the field order is different (bool first vs bool last).
std::optional's order (bool last) seems to be better overall, decreasing
binary size. Second, absl::optional's assertions crash with
__builtin_trap, while std::optional calls __libcpp_verbose_abort which
calls base::ImmediateCrash. __builtin_trap permits the compiler to
combine crash sites within a function but leads to worse crash
debugging. In base::ImmediateCrash, we'd made a conscious decision to
prefer debuggability and pay some binary size for it. The net size
increase brings our optional type in line with that preference.
For more details see the discussion and document below:
https://groups.google.com/a/chromium.org/g/cxx/c/XG3G85_ZF1k/m/_QN8adIJBQAJ
https://docs.google.com/document/d/1AW7q9HCLOk738OCj8Z2U_AKVUC0YIFZWuyRvv09XTHk/edit
Binary-Size: See discussion above.
Fuchsia-Binary-Size: See discussion above.
Bug: 1373619
This commit is contained in:
@@ -8,6 +8,7 @@
|
||||
|
||||
#include <list>
|
||||
#include <memory>
|
||||
#include <optional>
|
||||
#include <string>
|
||||
#include <string_view>
|
||||
|
||||
@@ -22,7 +23,6 @@
|
||||
#include "services/data_decoder/public/cpp/data_decoder.h"
|
||||
#include "services/network/public/cpp/simple_url_loader.h"
|
||||
#include "services/network/public/cpp/simple_url_loader_stream_consumer.h"
|
||||
#include "third_party/abseil-cpp/absl/types/optional.h"
|
||||
#include "url/gurl.h"
|
||||
|
||||
namespace network {
|
||||
@@ -83,7 +83,7 @@ struct APIRequestOptions {
|
||||
bool auto_retry_on_network_change = false;
|
||||
bool enable_cache = false;
|
||||
size_t max_body_size = -1u;
|
||||
absl::optional<base::TimeDelta> timeout;
|
||||
std::optional<base::TimeDelta> timeout;
|
||||
};
|
||||
|
||||
// Anyone is welcome to use APIRequestHelper to reduce boilerplate
|
||||
@@ -96,7 +96,7 @@ class APIRequestHelper {
|
||||
data_decoder::DataDecoder::ValueOrError result)>;
|
||||
using ResultCallback = base::OnceCallback<void(APIRequestResult)>;
|
||||
using ResponseConversionCallback =
|
||||
base::OnceCallback<absl::optional<std::string>(
|
||||
base::OnceCallback<std::optional<std::string>(
|
||||
const std::string& raw_response)>;
|
||||
|
||||
class URLLoaderHandler : public network::SimpleURLLoaderStreamConsumer {
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
#include "brave/components/api_request_helper/api_request_helper.h"
|
||||
|
||||
#include <memory>
|
||||
#include <optional>
|
||||
#include <string_view>
|
||||
#include <utility>
|
||||
|
||||
@@ -33,9 +34,9 @@ MATCHER_P(MatchesAPIRequestResult, request_result, "") {
|
||||
return arg == *request_result;
|
||||
}
|
||||
|
||||
absl::optional<std::string> ConversionCallback(
|
||||
std::optional<std::string> ConversionCallback(
|
||||
const std::string& expected_raw_response,
|
||||
const absl::optional<std::string>& converted_response,
|
||||
const std::optional<std::string>& converted_response,
|
||||
const std::string& raw_response) {
|
||||
EXPECT_EQ(expected_raw_response, raw_response);
|
||||
return converted_response;
|
||||
@@ -111,7 +112,7 @@ class ApiRequestHelperUnitTest : public testing::Test {
|
||||
SetInterceptor("POST", network_url, server_raw_response, enable_cache);
|
||||
api_request_helper_->Request(
|
||||
"POST", network_url, "", "application/json", callback.Get(), {},
|
||||
APIRequestOptions(false, enable_cache, -1u, absl::nullopt),
|
||||
APIRequestOptions(false, enable_cache, -1u, std::nullopt),
|
||||
std::move(conversion_callback));
|
||||
base::RunLoop().RunUntilIdle();
|
||||
}
|
||||
@@ -177,11 +178,11 @@ TEST_F(ApiRequestHelperUnitTest, RequestWithConversion) {
|
||||
SendRequest(server_raw_response, "", base::Value(), 200, net::OK,
|
||||
base::BindOnce(&ConversionCallback, server_raw_response, ""));
|
||||
|
||||
// Returning absl::nullopt in conversion callback results in empty response
|
||||
// Returning std::nullopt in conversion callback results in empty response
|
||||
server_raw_response = "{}";
|
||||
SendRequest(
|
||||
server_raw_response, "", base::Value(), 422, net::OK,
|
||||
base::BindOnce(&ConversionCallback, server_raw_response, absl::nullopt));
|
||||
base::BindOnce(&ConversionCallback, server_raw_response, std::nullopt));
|
||||
}
|
||||
|
||||
TEST_F(ApiRequestHelperUnitTest, Is2XXResponseCode) {
|
||||
|
||||
Reference in New Issue
Block a user