From 151183d35ee809dc3f96d1328a9437da44b544f9 Mon Sep 17 00:00:00 2001 From: Laramie Leavitt Date: Wed, 18 Sep 2024 13:55:06 -0700 Subject: [PATCH] prefer *std::move(std::optional{...}) to std::move(*std::optional{...}) PiperOrigin-RevId: 676119820 Change-Id: Iebc54df1c6880c52149c8913185c59a96ac701aa --- python/tensorstore/batch.cc | 2 +- python/tensorstore/context.cc | 4 +-- python/tensorstore/kvstore.cc | 2 +- python/tensorstore/tensorstore_class.cc | 2 +- tensorstore/context_impl_base.h | 4 +-- tensorstore/driver/array/array.cc | 2 +- tensorstore/driver/image/driver_impl.h | 8 ++--- tensorstore/driver/json/driver.cc | 4 +-- tensorstore/driver/kvs_backed_chunk_driver.cc | 20 +++++------ .../driver/neuroglancer_precomputed/driver.cc | 2 +- tensorstore/driver/zarr/spec.cc | 2 +- tensorstore/driver/zarr3/codec/blosc.cc | 2 +- tensorstore/index_space/index_transform.h | 2 +- tensorstore/index_space/transformed_array.h | 2 +- tensorstore/internal/cache/BUILD | 6 ++++ tensorstore/internal/cache/kvs_backed_cache.h | 4 +-- .../internal/cache/kvs_backed_chunk_cache.cc | 14 ++++++-- .../internal/compression/blosc_compressor.cc | 2 +- .../internal/http/curl_factory_hook.cc | 4 +-- tensorstore/internal/http/http_request.h | 2 +- tensorstore/internal/json/value_as.h | 2 +- tensorstore/internal/metrics/registry.cc | 2 +- .../internal/oauth2/google_auth_provider.cc | 3 +- tensorstore/kvstore/gcs_grpc/gcs_grpc.cc | 2 +- .../kvstore/gcs_http/gcs_key_value_store.cc | 4 +-- tensorstore/kvstore/http/driver.cc | 2 +- .../kvstore/memory/memory_key_value_store.cc | 4 +-- tensorstore/kvstore/mock_kvstore.h | 1 + .../neuroglancer_uint64_sharded.cc | 4 +-- tensorstore/kvstore/ocdbt/distributed/BUILD | 4 +++ .../kvstore/ocdbt/distributed/btree_writer.cc | 4 +-- .../cooperator_commit_mutations.cc | 35 +++++++++++++++++-- tensorstore/kvstore/ocdbt/io/node_cache.h | 2 +- .../ocdbt/non_distributed/btree_writer.cc | 4 +-- tensorstore/kvstore/ocdbt/python/BUILD | 1 + tensorstore/kvstore/ocdbt/python/bindings.cc | 4 ++- .../kvstore/s3/aws_credentials_resource.cc | 2 +- .../credentials/file_credential_provider.cc | 2 +- tensorstore/kvstore/s3/s3_key_value_store.cc | 4 +-- tensorstore/kvstore/transaction.cc | 3 +- tensorstore/util/result.h | 4 +-- tensorstore/util/result_test.cc | 2 +- tensorstore/util/status.cc | 2 +- 43 files changed, 122 insertions(+), 65 deletions(-) diff --git a/python/tensorstore/batch.cc b/python/tensorstore/batch.cc index c2d3d9afc..f6aa78d5d 100644 --- a/python/tensorstore/batch.cc +++ b/python/tensorstore/batch.cc @@ -35,7 +35,7 @@ namespace py = ::pybind11; Batch ValidateOptionalBatch(std::optional batch) { if (!batch) return no_batch; if (!*batch) throw py::value_error("batch was already submitted"); - return std::move(*batch); + return *std::move(batch); } namespace { diff --git a/python/tensorstore/context.cc b/python/tensorstore/context.cc index 00fec70ed..cd25005da 100644 --- a/python/tensorstore/context.cc +++ b/python/tensorstore/context.cc @@ -103,7 +103,7 @@ Constructs a default context. std::optional parent) { if (!parent) parent.emplace(); return Access::impl(Context(WrapImpl(std::move(spec)), - WrapImpl(std::move(*parent)))); + WrapImpl(*std::move(parent)))); }), R"( Constructs a context from a parsed spec. @@ -122,7 +122,7 @@ Constructs a context from a parsed spec. py::init([](::nlohmann::json json, std::optional parent) { if (!parent) parent.emplace(); return Access::impl(Context(ValueOrThrow(Context::Spec::FromJson(json)), - WrapImpl(std::move(*parent)))); + WrapImpl(*std::move(parent)))); }), R"( Constructs a context from its :json:schema:`JSON representation`. diff --git a/python/tensorstore/kvstore.cc b/python/tensorstore/kvstore.cc index 9380da9b2..fb8c31dac 100644 --- a/python/tensorstore/kvstore.cc +++ b/python/tensorstore/kvstore.cc @@ -71,7 +71,7 @@ py::bytes CordToPython(const absl::Cord& value) { std::optional OptionalCordFromPython( std::optional value) { if (!value) return std::nullopt; - return absl::Cord(std::move(*value)); + return absl::Cord(*value); } namespace kvstore_spec_setters { diff --git a/python/tensorstore/tensorstore_class.cc b/python/tensorstore/tensorstore_class.cc index 24aff168e..3e6786931 100644 --- a/python/tensorstore/tensorstore_class.cc +++ b/python/tensorstore/tensorstore_class.cc @@ -1389,7 +1389,7 @@ Associated transaction used for read/write operations. if (!transaction) transaction.emplace(); return ValueOrThrow( self.value | - internal::TransactionState::ToTransaction(std::move(*transaction))); + internal::TransactionState::ToTransaction(*std::move(transaction))); }, R"(Returns a transaction-bound view of this TensorStore. diff --git a/tensorstore/context_impl_base.h b/tensorstore/context_impl_base.h index 2d3cb0a0e..bbc3b7489 100644 --- a/tensorstore/context_impl_base.h +++ b/tensorstore/context_impl_base.h @@ -410,7 +410,7 @@ class ResourceProviderImpl : public ResourceProviderImplBase { return std::move(result).status(); } return ResourceImplStrongPtr( - new ResourceImpl(ResourceSpecImplPtr(this), std::move(*result))); + new ResourceImpl(ResourceSpecImplPtr(this), *std::move(result))); } Result<::nlohmann::json> ToJson(JsonSerializationOptions options) override { @@ -448,7 +448,7 @@ class ResourceProviderImpl : public ResourceProviderImplBase { auto result = internal_json_binding::FromJson(j, traits_.JsonBinder(), options); if (!result) return std::move(result).status(); - return ResourceSpecImplPtr(new SpecImpl(std::move(*result))); + return ResourceSpecImplPtr(new SpecImpl(*std::move(result))); } void AcquireContextReference(ResourceImplBase& resource) const override { diff --git a/tensorstore/driver/array/array.cc b/tensorstore/driver/array/array.cc index cb2bf6990..442a9fa00 100644 --- a/tensorstore/driver/array/array.cc +++ b/tensorstore/driver/array/array.cc @@ -539,7 +539,7 @@ Result SpecFromArray(SharedOffsetArrayView array, if (!zero_origin_array->valid() && zero_origin_array->num_elements() != 0) { return absl::InvalidArgumentError("Array is not valid"); } - driver_spec->array = std::move(*zero_origin_array); + driver_spec->array = *std::move(zero_origin_array); impl.driver_spec = std::move(driver_spec); return spec; } diff --git a/tensorstore/driver/image/driver_impl.h b/tensorstore/driver/image/driver_impl.h index a53d081ec..00538daf4 100644 --- a/tensorstore/driver/image/driver_impl.h +++ b/tensorstore/driver/image/driver_impl.h @@ -220,14 +220,14 @@ class ImageCache : public internal::KvsBackedCache, } auto options = GetOwningCache(*this).specialization_; GetOwningCache(*this).executor()( - [value = std::move(value), receiver = std::move(receiver), + [value = *std::move(value), receiver = std::move(receiver), options = std::move(options)]() mutable { - auto decode_result = options.DecodeImage(std::move(*value)); + auto decode_result = options.DecodeImage(std::move(value)); if (!decode_result.ok()) { execution::set_error(receiver, decode_result.status()); } else { execution::set_value(receiver, std::make_shared( - std::move(*decode_result))); + *std::move(decode_result))); } }); } @@ -239,7 +239,7 @@ class ImageCache : public internal::KvsBackedCache, if (!encode_result.ok()) { execution::set_error(receiver, encode_result.status()); } else { - execution::set_value(receiver, std::move(*encode_result)); + execution::set_value(receiver, *std::move(encode_result)); } } }; diff --git a/tensorstore/driver/json/driver.cc b/tensorstore/driver/json/driver.cc index ebf58cbc7..bd7fbf4d5 100644 --- a/tensorstore/driver/json/driver.cc +++ b/tensorstore/driver/json/driver.cc @@ -117,7 +117,7 @@ class JsonCache return; } execution::set_value(receiver, std::make_shared<::nlohmann::json>( - std::move(*decode_result))); + *std::move(decode_result))); }); } void DoEncode(std::shared_ptr data, @@ -173,7 +173,7 @@ class JsonCache ? *existing_json : ::nlohmann::json(::nlohmann::json::value_t::discarded)); if (result.ok()) { - new_json = std::move(*result); + new_json = *std::move(result); } else { execution::set_error(receiver, std::move(result).status()); return; diff --git a/tensorstore/driver/kvs_backed_chunk_driver.cc b/tensorstore/driver/kvs_backed_chunk_driver.cc index 10a79fb99..5db8aa023 100644 --- a/tensorstore/driver/kvs_backed_chunk_driver.cc +++ b/tensorstore/driver/kvs_backed_chunk_driver.cc @@ -621,7 +621,7 @@ struct ResolveBoundsForDeleteAndResizeContinuation { std::shared_ptr new_metadata; if (auto result = ValidateNewMetadata(*state->driver, state->transaction); result.ok()) { - new_metadata = std::move(*result); + new_metadata = *std::move(result); } else { promise.SetResult(std::move(result).status()); return; @@ -702,7 +702,7 @@ Future> KvsChunkedDriverBase::Resize( /*.transaction=*/std::move(request.transaction), /*.component_index=*/component_index(), /*.transform=*/std::move(request.transform), - /*.resize_parameters=*/std::move(*resize_parameters), + /*.resize_parameters=*/*std::move(resize_parameters), }; if ((request.options.mode & resize_metadata_only) == resize_metadata_only || (request.options.mode & expand_only) == expand_only) { @@ -861,7 +861,7 @@ Result CreateTensorStoreFromMetadata( return nullptr; } DataCacheInitializer initializer; - initializer.store = std::move(*store_result); + initializer.store = *std::move(store_result); initializer.metadata_cache_entry = base.metadata_cache_entry_; initializer.metadata = metadata; initializer.cache_pool = state->cache_pool(); @@ -978,7 +978,7 @@ struct HandleReadMetadata { if (auto result = base.metadata_cache_entry_->GetMetadata(base.transaction_); result.ok()) { - metadata = std::move(*result); + metadata = *std::move(result); } else { promise.SetResult(std::move(result).status()); return; @@ -1151,10 +1151,10 @@ void MetadataCache::Entry::DoDecode(std::optional value, receiver = std::move(receiver)]() mutable { MetadataPtr new_metadata; if (value) { - if (auto result = - GetOwningCache(*this).DecodeMetadata(this->key(), *value); + if (auto result = GetOwningCache(*this).DecodeMetadata(this->key(), + *std::move(value)); result.ok()) { - new_metadata = std::move(*result); + new_metadata = *std::move(result); } else { execution::set_error( receiver, internal::ConvertInvalidArgumentToFailedPrecondition( @@ -1188,7 +1188,7 @@ void MetadataCache::TransactionNode::DoApply(ApplyOptions options, auto read_state = AsyncCache::ReadLock(*this).read_state(); std::shared_ptr new_data; if (auto result = this->GetUpdatedMetadata(read_state.data); result.ok()) { - new_data = std::move(*result); + new_data = *std::move(result); } else { execution::set_error(receiver, std::move(result).status()); return; @@ -1219,7 +1219,7 @@ void MetadataCache::Entry::DoEncode(std::shared_ptr data, auto& cache = GetOwningCache(entry); if (auto encoded_result = cache.EncodeMetadata(entry.key(), data.get()); encoded_result.ok()) { - execution::set_value(receiver, std::move(*encoded_result)); + execution::set_value(receiver, *std::move(encoded_result)); } else { execution::set_error(receiver, std::move(encoded_result).status()); } @@ -1272,7 +1272,7 @@ internal::CachePtr GetOrCreateMetadataCache( if (auto result = state->GetMetadataKeyValueStore( metadata_cache->base_store_); result.ok()) { - metadata_cache->SetKvStoreDriver(std::move(*result)); + metadata_cache->SetKvStoreDriver(*std::move(result)); } else { metadata_cache_promise.SetResult(std::move(result).status()); } diff --git a/tensorstore/driver/neuroglancer_precomputed/driver.cc b/tensorstore/driver/neuroglancer_precomputed/driver.cc index 79af8faae..0b4ff5e3f 100644 --- a/tensorstore/driver/neuroglancer_precomputed/driver.cc +++ b/tensorstore/driver/neuroglancer_precomputed/driver.cc @@ -323,7 +323,7 @@ class DataCacheBase : public internal_kvs_backed_chunk_driver::DataCache { chunk_indices, metadata(), scale_index_, chunk_layout_czyx_, std::move(data))) { absl::InlinedVector, 1> components; - components.emplace_back(std::move(*result)); + components.emplace_back(*std::move(result)); return components; } else { return absl::FailedPreconditionError(result.status().message()); diff --git a/tensorstore/driver/zarr/spec.cc b/tensorstore/driver/zarr/spec.cc index 6fcdd5d83..62ee08b85 100644 --- a/tensorstore/driver/zarr/spec.cc +++ b/tensorstore/driver/zarr/spec.cc @@ -232,7 +232,7 @@ Result GetNewMetadata( } TENSORSTORE_RETURN_IF_ERROR(codec_spec->MergeFrom(schema.codec())); if (codec_spec->compressor) { - metadata->compressor = std::move(*codec_spec->compressor); + metadata->compressor = *std::move(codec_spec->compressor); } else { TENSORSTORE_ASSIGN_OR_RETURN(metadata->compressor, Compressor::FromJson({{"id", "blosc"}})); diff --git a/tensorstore/driver/zarr3/codec/blosc.cc b/tensorstore/driver/zarr3/codec/blosc.cc index 1443ad94f..c530d7ac4 100644 --- a/tensorstore/driver/zarr3/codec/blosc.cc +++ b/tensorstore/driver/zarr3/codec/blosc.cc @@ -105,7 +105,7 @@ class BloscCodec : public ZarrBytesToBytesCodec { return *std::move(output); }); auto reader = std::make_unique>( - output.ok() ? riegeli::Chain(std::move(*output)) : riegeli::Chain()); + output.ok() ? riegeli::Chain(*std::move(output)) : riegeli::Chain()); if (!output.ok()) { reader->Fail(std::move(output).status()); } diff --git a/tensorstore/index_space/index_transform.h b/tensorstore/index_space/index_transform.h index 1d94142a7..7bb1409d0 100644 --- a/tensorstore/index_space/index_transform.h +++ b/tensorstore/index_space/index_transform.h @@ -1103,7 +1103,7 @@ MakeCopy(const Array& array, if (auto result = TransformArray(array, transform, {constraints, must_allocate})) { return ConstDataTypeCast>( - std::move(*result)); + *std::move(result)); } else { return std::move(result).status(); } diff --git a/tensorstore/index_space/transformed_array.h b/tensorstore/index_space/transformed_array.h index 4b21620b9..8d72d20d2 100644 --- a/tensorstore/index_space/transformed_array.h +++ b/tensorstore/index_space/transformed_array.h @@ -856,7 +856,7 @@ inline Result> TryConvertToArrayImpl( array.element_pointer() = ElementPointer( StaticDataTypeCast::Element, unchecked>( - ConstDataTypeCast(std::move(*element_pointer_result)))); + ConstDataTypeCast(*std::move(element_pointer_result)))); return array; } diff --git a/tensorstore/internal/cache/BUILD b/tensorstore/internal/cache/BUILD index fd90c166a..da13a3dec 100644 --- a/tensorstore/internal/cache/BUILD +++ b/tensorstore/internal/cache/BUILD @@ -207,12 +207,18 @@ tensorstore_cc_library( srcs = ["kvs_backed_chunk_cache.cc"], hdrs = ["kvs_backed_chunk_cache.h"], deps = [ + ":async_cache", + ":cache", ":chunk_cache", ":kvs_backed_cache", "//tensorstore:array", "//tensorstore:index", + "//tensorstore/internal:memory", "//tensorstore/util:result", "//tensorstore/util:span", + "//tensorstore/util:status", + "//tensorstore/util/execution", + "@com_google_absl//absl/container:fixed_array", "@com_google_absl//absl/container:inlined_vector", "@com_google_absl//absl/strings:cord", ], diff --git a/tensorstore/internal/cache/kvs_backed_cache.h b/tensorstore/internal/cache/kvs_backed_cache.h index 11d41fffe..904c23b6e 100644 --- a/tensorstore/internal/cache/kvs_backed_cache.h +++ b/tensorstore/internal/cache/kvs_backed_cache.h @@ -305,7 +305,7 @@ class KvsBackedCache : public Parent { void set_cancel() { ABSL_UNREACHABLE(); } // COV_NF_LINE void set_value(std::optional value) { kvstore::ReadResult read_result = - value ? kvstore::ReadResult::Value(std::move(*value), + value ? kvstore::ReadResult::Value(*std::move(value), std::move(update_stamp_)) : kvstore::ReadResult::Missing(std::move(update_stamp_)); execution::set_value(receiver_, std::move(read_result)); @@ -403,7 +403,7 @@ class KvsBackedCache : public Parent { void KvsWritebackSuccess(TimestampedStorageGeneration new_stamp) override { if (new_data_) { this->WritebackSuccess( - AsyncCache::ReadState{std::move(*new_data_), std::move(new_stamp)}); + AsyncCache::ReadState{*std::move(new_data_), std::move(new_stamp)}); } else { // Unmodified. this->WritebackSuccess(AsyncCache::ReadState{}); diff --git a/tensorstore/internal/cache/kvs_backed_chunk_cache.cc b/tensorstore/internal/cache/kvs_backed_chunk_cache.cc index b3f93475a..05ce87109 100644 --- a/tensorstore/internal/cache/kvs_backed_chunk_cache.cc +++ b/tensorstore/internal/cache/kvs_backed_chunk_cache.cc @@ -14,18 +14,28 @@ #include "tensorstore/internal/cache/kvs_backed_chunk_cache.h" +#include +#include +#include #include #include #include +#include +#include "absl/container/fixed_array.h" #include "absl/container/inlined_vector.h" #include "absl/strings/cord.h" #include "tensorstore/array.h" #include "tensorstore/index.h" +#include "tensorstore/internal/cache/async_cache.h" +#include "tensorstore/internal/cache/cache.h" #include "tensorstore/internal/cache/chunk_cache.h" #include "tensorstore/internal/cache/kvs_backed_cache.h" +#include "tensorstore/internal/memory.h" +#include "tensorstore/util/execution/execution.h" #include "tensorstore/util/result.h" #include "tensorstore/util/span.h" +#include "tensorstore/util/status.h" namespace tensorstore { namespace internal { @@ -45,7 +55,7 @@ void KvsBackedChunkCache::Entry::DoDecode(std::optional value, } auto& cache = GetOwningCache(*this); auto decoded_result = - cache.DecodeChunk(this->cell_indices(), std::move(*value)); + cache.DecodeChunk(this->cell_indices(), *std::move(value)); if (!decoded_result.ok()) { execution::set_error(receiver, internal::ConvertInvalidArgumentToFailedPrecondition( @@ -92,7 +102,7 @@ void KvsBackedChunkCache::Entry::DoEncode(std::shared_ptr data, execution::set_error(receiver, std::move(encoded_result).status()); return; } - execution::set_value(receiver, std::move(*encoded_result)); + execution::set_value(receiver, *std::move(encoded_result)); } } // namespace internal diff --git a/tensorstore/internal/compression/blosc_compressor.cc b/tensorstore/internal/compression/blosc_compressor.cc index c015b4c5b..2cc95fba9 100644 --- a/tensorstore/internal/compression/blosc_compressor.cc +++ b/tensorstore/internal/compression/blosc_compressor.cc @@ -86,7 +86,7 @@ std::unique_ptr BloscCompressor::GetReader( return *std::move(output); }); auto reader = std::make_unique>( - output.ok() ? riegeli::Chain(std::move(*output)) : riegeli::Chain()); + output.ok() ? riegeli::Chain(*std::move(output)) : riegeli::Chain()); if (!output.ok()) { reader->Fail(std::move(output).status()); } diff --git a/tensorstore/internal/http/curl_factory_hook.cc b/tensorstore/internal/http/curl_factory_hook.cc index 80008d613..d57984ea2 100644 --- a/tensorstore/internal/http/curl_factory_hook.cc +++ b/tensorstore/internal/http/curl_factory_hook.cc @@ -102,11 +102,11 @@ void CurlPtrHook(CurlPtr& handle) { // Return either "SSL_CERT_FILE" or "SSL_CERT_DIR" if set. if (auto default_cert_env = internal::GetEnv("SSL_CERT_FILE"); default_cert_env.has_value()) { - return CertFile{std::move(*default_cert_env)}; + return CertFile{*std::move(default_cert_env)}; } if (auto default_cert_env = internal::GetEnv("SSL_CERT_DIR"); default_cert_env.has_value()) { - return CertDirectory{std::move(*default_cert_env)}; + return CertDirectory{*std::move(default_cert_env)}; } // This check only happens on startup so that all the curl handles use diff --git a/tensorstore/internal/http/http_request.h b/tensorstore/internal/http/http_request.h index 0ff5b55f1..086ee3822 100644 --- a/tensorstore/internal/http/http_request.h +++ b/tensorstore/internal/http/http_request.h @@ -114,7 +114,7 @@ class HttpRequestBuilder { return AddHeader(std::string_view(header)); } HttpRequestBuilder& AddHeader(std::optional header) { - return header ? AddHeader(std::move(*header)) : *this; + return header ? AddHeader(*std::move(header)) : *this; } /// Adds a `range` header to the http request if the byte_range diff --git a/tensorstore/internal/json/value_as.h b/tensorstore/internal/json/value_as.h index ebc5a64df..e657e3593 100644 --- a/tensorstore/internal/json/value_as.h +++ b/tensorstore/internal/json/value_as.h @@ -151,7 +151,7 @@ JsonRequireValueAs(const ::nlohmann::json& j, T* result, ValidateFn is_valid, j, internal_json::GetTypeName(internal::type_identity{})); } if (result != nullptr) { - *result = std::move(*value); + *result = *std::move(value); } return absl::OkStatus(); } diff --git a/tensorstore/internal/metrics/registry.cc b/tensorstore/internal/metrics/registry.cc index 619709cab..1198a962b 100644 --- a/tensorstore/internal/metrics/registry.cc +++ b/tensorstore/internal/metrics/registry.cc @@ -51,7 +51,7 @@ std::vector MetricRegistry::CollectWithPrefix( if (prefix.empty() || absl::StartsWith(kv.first, prefix)) { auto opt_metric = kv.second.poly(CollectMetricTag{}); if (opt_metric.has_value()) { - all.emplace_back(std::move(*opt_metric)); + all.emplace_back(*std::move(opt_metric)); assert(all.back().metric_name == kv.first); } } diff --git a/tensorstore/internal/oauth2/google_auth_provider.cc b/tensorstore/internal/oauth2/google_auth_provider.cc index 35a2320e6..4701ad6b3 100644 --- a/tensorstore/internal/oauth2/google_auth_provider.cc +++ b/tensorstore/internal/oauth2/google_auth_provider.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -133,7 +134,7 @@ Result> GetDefaultGoogleAuthProvider( auto var = GetEnv(kGoogleAuthTokenForTesting); if (var) { ABSL_LOG(INFO) << "Using GOOGLE_AUTH_TOKEN_FOR_TESTING"; - result.reset(new FixedTokenAuthProvider(std::move(*var))); + result.reset(new FixedTokenAuthProvider(*std::move(var))); return std::move(result); } diff --git a/tensorstore/kvstore/gcs_grpc/gcs_grpc.cc b/tensorstore/kvstore/gcs_grpc/gcs_grpc.cc index e0d5d8ad8..1ef3e435a 100644 --- a/tensorstore/kvstore/gcs_grpc/gcs_grpc.cc +++ b/tensorstore/kvstore/gcs_grpc/gcs_grpc.cc @@ -1085,7 +1085,7 @@ Future GcsGrpcKeyValueStore::Write( task->driver_ = internal::IntrusivePtr(this); task->options_ = std::move(options); task->promise_ = std::move(op.promise); - task->Start(key, std::move(*value)); + task->Start(key, *std::move(value)); } return std::move(op.future); } diff --git a/tensorstore/kvstore/gcs_http/gcs_key_value_store.cc b/tensorstore/kvstore/gcs_http/gcs_key_value_store.cc index 6a509c985..482746889 100644 --- a/tensorstore/kvstore/gcs_http/gcs_key_value_store.cc +++ b/tensorstore/kvstore/gcs_http/gcs_key_value_store.cc @@ -357,7 +357,7 @@ class GcsKeyValueStore auth_provider_ = nullptr; } else { TENSORSTORE_RETURN_IF_ERROR(result); - auth_provider_ = std::move(*result); + auth_provider_ = *std::move(result); } } if (!*auth_provider_) return std::nullopt; @@ -996,7 +996,7 @@ Future GcsKeyValueStore::Write( if (value) { auto state = internal::MakeIntrusivePtr( IntrusivePtr(this), std::move(encoded_object_name), - std::move(*value), std::move(options), std::move(op.promise)); + *std::move(value), std::move(options), std::move(op.promise)); intrusive_ptr_increment(state.get()); // adopted by WriteTask::Start. write_rate_limiter().Admit(state.get(), &WriteTask::Start); diff --git a/tensorstore/kvstore/http/driver.cc b/tensorstore/kvstore/http/driver.cc index 073fdc53d..c06113dda 100644 --- a/tensorstore/kvstore/http/driver.cc +++ b/tensorstore/kvstore/http/driver.cc @@ -301,7 +301,7 @@ struct ReadTask { auto response = owner->transport_->IssueRequest(request, {}).result(); if (!response.ok()) return response.status(); - httpresponse = std::move(*response); + httpresponse = *std::move(response); http_bytes_read.IncrementBy(httpresponse.payload.size()); ABSL_LOG_IF(INFO, http_logging.Level(1)) << "[http] Read response: " << httpresponse; diff --git a/tensorstore/kvstore/memory/memory_key_value_store.cc b/tensorstore/kvstore/memory/memory_key_value_store.cc index 5fa9057b8..89a6626ea 100644 --- a/tensorstore/kvstore/memory/memory_key_value_store.cc +++ b/tensorstore/kvstore/memory/memory_key_value_store.cc @@ -417,7 +417,7 @@ Future MemoryDriver::Write( // generation number. it = values .emplace(std::move(key), - ValueWithGenerationNumber{std::move(*value), + ValueWithGenerationNumber{*std::move(value), data.next_generation_number++}) .first; return GenerationNow(it->second.generation()); @@ -436,7 +436,7 @@ Future MemoryDriver::Write( // Set the generation number to the next unused generation number. it->second.generation_number = data.next_generation_number++; // Update the value. - it->second.value = std::move(*value); + it->second.value = *std::move(value); return GenerationNow(it->second.generation()); } diff --git a/tensorstore/kvstore/mock_kvstore.h b/tensorstore/kvstore/mock_kvstore.h index b1faee981..6042b3ae5 100644 --- a/tensorstore/kvstore/mock_kvstore.h +++ b/tensorstore/kvstore/mock_kvstore.h @@ -25,6 +25,7 @@ #include "tensorstore/kvstore/driver.h" #include "tensorstore/kvstore/generation.h" #include "tensorstore/kvstore/key_range.h" +#include "tensorstore/kvstore/operations.h" #include "tensorstore/kvstore/spec.h" #include "tensorstore/kvstore/supported_features.h" #include "tensorstore/util/future.h" diff --git a/tensorstore/kvstore/neuroglancer_uint64_sharded/neuroglancer_uint64_sharded.cc b/tensorstore/kvstore/neuroglancer_uint64_sharded/neuroglancer_uint64_sharded.cc index 5ca189c41..d13a60424 100644 --- a/tensorstore/kvstore/neuroglancer_uint64_sharded/neuroglancer_uint64_sharded.cc +++ b/tensorstore/kvstore/neuroglancer_uint64_sharded/neuroglancer_uint64_sharded.cc @@ -401,7 +401,7 @@ class MinishardIndexCache if (auto result = DecodeMinishardIndexAndAdjustByteRanges( *value, GetOwningCache(*this).sharding_spec()); result.ok()) { - read_data = std::make_shared(std::move(*result)); + read_data = std::make_shared(*std::move(result)); } else { execution::set_error(receiver, ConvertInvalidArgumentToFailedPrecondition( @@ -497,7 +497,7 @@ class ShardedKeyValueStoreWriteCache if (auto result = SplitShard(GetOwningCache(*this).sharding_spec(), *value); result.ok()) { - chunks = std::move(*result); + chunks = *std::move(result); } else { execution::set_error(receiver, ConvertInvalidArgumentToFailedPrecondition( diff --git a/tensorstore/kvstore/ocdbt/distributed/BUILD b/tensorstore/kvstore/ocdbt/distributed/BUILD index dab492a76..b0a002719 100644 --- a/tensorstore/kvstore/ocdbt/distributed/BUILD +++ b/tensorstore/kvstore/ocdbt/distributed/BUILD @@ -198,6 +198,7 @@ tensorstore_cc_library( "//tensorstore/internal/log:verbose_flag", "//tensorstore/kvstore", "//tensorstore/kvstore:generation", + "//tensorstore/kvstore:key_range", "//tensorstore/kvstore/ocdbt:io_handle", "//tensorstore/kvstore/ocdbt/format", "//tensorstore/kvstore/ocdbt/non_distributed:create_new_manifest", @@ -209,6 +210,9 @@ tensorstore_cc_library( "//tensorstore/util:future", "//tensorstore/util:quote_string", "//tensorstore/util:result", + "//tensorstore/util:span", + "//tensorstore/util:status", + "//tensorstore/util:str_cat", "@com_github_grpc_grpc//:grpc++", "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/container:flat_hash_map", diff --git a/tensorstore/kvstore/ocdbt/distributed/btree_writer.cc b/tensorstore/kvstore/ocdbt/distributed/btree_writer.cc index 3715bb26a..df6108e72 100644 --- a/tensorstore/kvstore/ocdbt/distributed/btree_writer.cc +++ b/tensorstore/kvstore/ocdbt/distributed/btree_writer.cc @@ -736,10 +736,10 @@ Future DistributedBtreeWriter::Write( needs_inline_value_pass = true; } // Config not yet known or value to be written inline. - value_ref = std::move(*value); + value_ref = *std::move(value); } else { request.flush_future = writer.io_handle_->WriteData( - IndirectDataKind::kValue, std::move(*value), + IndirectDataKind::kValue, *std::move(value), value_ref.emplace()); } } diff --git a/tensorstore/kvstore/ocdbt/distributed/cooperator_commit_mutations.cc b/tensorstore/kvstore/ocdbt/distributed/cooperator_commit_mutations.cc index 3e8fac69d..bfc9783f2 100644 --- a/tensorstore/kvstore/ocdbt/distributed/cooperator_commit_mutations.cc +++ b/tensorstore/kvstore/ocdbt/distributed/cooperator_commit_mutations.cc @@ -15,15 +15,46 @@ #include "tensorstore/kvstore/ocdbt/distributed/cooperator.h" // Part of the Cooperator interface +#include +#include +#include +#include +#include +#include +#include +#include +#include + #include "absl/base/attributes.h" #include "absl/log/absl_log.h" +#include "absl/status/status.h" +#include "absl/synchronization/mutex.h" +#include "absl/time/clock.h" +#include "absl/time/time.h" #include "tensorstore/internal/grpc/utils.h" +#include "tensorstore/internal/intrusive_ptr.h" #include "tensorstore/internal/log/verbose_flag.h" +#include "tensorstore/internal/mutex.h" +#include "tensorstore/kvstore/generation.h" +#include "tensorstore/kvstore/key_range.h" +#include "tensorstore/kvstore/ocdbt/distributed/btree_node_identifier.h" +#include "tensorstore/kvstore/ocdbt/distributed/btree_node_write_mutation.h" #include "tensorstore/kvstore/ocdbt/distributed/cooperator_impl.h" +#include "tensorstore/kvstore/ocdbt/format/btree.h" +#include "tensorstore/kvstore/ocdbt/format/btree_node_encoder.h" +#include "tensorstore/kvstore/ocdbt/format/manifest.h" +#include "tensorstore/kvstore/ocdbt/format/version_tree.h" +#include "tensorstore/kvstore/ocdbt/io_handle.h" #include "tensorstore/kvstore/ocdbt/non_distributed/create_new_manifest.h" #include "tensorstore/kvstore/ocdbt/non_distributed/storage_generation.h" #include "tensorstore/kvstore/ocdbt/non_distributed/write_nodes.h" +#include "tensorstore/util/executor.h" +#include "tensorstore/util/future.h" #include "tensorstore/util/quote_string.h" +#include "tensorstore/util/result.h" +#include "tensorstore/util/span.h" +#include "tensorstore/util/status.h" +#include "tensorstore/util/str_cat.h" namespace tensorstore { namespace internal_ocdbt_cooperator { @@ -670,7 +701,7 @@ void NodeCommitOperation::UpdateParent( mutation->mode = new_entries->empty() ? BtreeNodeWriteMutation::kDeleteExisting : BtreeNodeWriteMutation::kAddNew; - mutation->new_entries = std::move(*new_entries); + mutation->new_entries = *std::move(new_entries); } else { mutation->mode = BtreeNodeWriteMutation::kRetainExisting; } @@ -727,7 +758,7 @@ void NodeCommitOperation::UpdateRoot( new_generation, internal_ocdbt::WriteRootNode( *commit_op->server->io_handle_, commit_op->flush_promise, - commit_op->height, std::move(*new_entries)), + commit_op->height, *std::move(new_entries)), commit_op->SetError(_)); } CreateNewManifest(std::move(commit_op), std::move(new_generation)); diff --git a/tensorstore/kvstore/ocdbt/io/node_cache.h b/tensorstore/kvstore/ocdbt/io/node_cache.h index 26ec89952..07fc59a93 100644 --- a/tensorstore/kvstore/ocdbt/io/node_cache.h +++ b/tensorstore/kvstore/ocdbt/io/node_cache.h @@ -83,7 +83,7 @@ class DecodedIndirectDataCache ABSL_CHECK(ref.DecodeCacheKey(this->key())); GetOwningCache(*this).executor()( - [value = std::move(*value), base_path = ref.file_id.base_path, + [value = *std::move(value), base_path = ref.file_id.base_path, receiver = std::move(receiver)]() mutable { auto read_data = std::make_shared(); TENSORSTORE_ASSIGN_OR_RETURN( diff --git a/tensorstore/kvstore/ocdbt/non_distributed/btree_writer.cc b/tensorstore/kvstore/ocdbt/non_distributed/btree_writer.cc index 85e9422ca..1dddc113a 100644 --- a/tensorstore/kvstore/ocdbt/non_distributed/btree_writer.cc +++ b/tensorstore/kvstore/ocdbt/non_distributed/btree_writer.cc @@ -291,10 +291,10 @@ Future NonDistributedBtreeWriter::Write( writer.io_handle_->config_state->GetAssumedOrExistingConfig(); !config || value->size() <= config->max_inline_value_bytes) { // Config not yet known or value to be written inline. - value_ref = std::move(*value); + value_ref = *std::move(value); } else { value_future = writer.io_handle_->WriteData( - IndirectDataKind::kValue, std::move(*value), + IndirectDataKind::kValue, *std::move(value), value_ref.emplace()); } } diff --git a/tensorstore/kvstore/ocdbt/python/BUILD b/tensorstore/kvstore/ocdbt/python/BUILD index 210e67655..3e068f5f0 100644 --- a/tensorstore/kvstore/ocdbt/python/BUILD +++ b/tensorstore/kvstore/ocdbt/python/BUILD @@ -30,6 +30,7 @@ pybind11_cc_library( "//tensorstore/util:executor", "//tensorstore/util:future", "//tensorstore/util/garbage_collection:json", + "@com_github_nlohmann_json//:json", "@com_github_pybind_pybind11//:pybind11", "@com_google_absl//absl/strings:cord", ], diff --git a/tensorstore/kvstore/ocdbt/python/bindings.cc b/tensorstore/kvstore/ocdbt/python/bindings.cc index f6569fadc..5fff2dc66 100644 --- a/tensorstore/kvstore/ocdbt/python/bindings.cc +++ b/tensorstore/kvstore/ocdbt/python/bindings.cc @@ -21,9 +21,11 @@ #include #include +#include #include #include "absl/strings/cord.h" +#include #include "python/tensorstore/context.h" #include "python/tensorstore/future.h" #include "python/tensorstore/json_type_caster.h" @@ -96,7 +98,7 @@ void RegisterOcdbtBindings(py::module m, Executor defer) { } auto read_and_dump_future = internal_ocdbt::ReadAndDump( base.value, node_identifier, - context ? WrapImpl(std::move(*context)) : Context()); + context ? WrapImpl(*std::move(context)) : Context()); return PythonFutureWrapper<::nlohmann::json>( MapFutureValue( InlineExecutor{}, diff --git a/tensorstore/kvstore/s3/aws_credentials_resource.cc b/tensorstore/kvstore/s3/aws_credentials_resource.cc index b5d6bda0b..b9f1ba3b1 100644 --- a/tensorstore/kvstore/s3/aws_credentials_resource.cc +++ b/tensorstore/kvstore/s3/aws_credentials_resource.cc @@ -58,7 +58,7 @@ Result AwsCredentialsResource::Create( return Resource{spec, nullptr}; } TENSORSTORE_RETURN_IF_ERROR(result); - return Resource{spec, std::move(*result)}; + return Resource{spec, *std::move(result)}; } Result> diff --git a/tensorstore/kvstore/s3/credentials/file_credential_provider.cc b/tensorstore/kvstore/s3/credentials/file_credential_provider.cc index 44f1f037f..1175af2b0 100644 --- a/tensorstore/kvstore/s3/credentials/file_credential_provider.cc +++ b/tensorstore/kvstore/s3/credentials/file_credential_provider.cc @@ -77,7 +77,7 @@ FileCredentialProvider::FileCredentialProvider(std::string_view filename, : filename_(filename), profile_(profile) { if (filename_.empty()) { if (auto credentials_file = GetAwsCredentialsFileName(); credentials_file) { - filename_ = std::move(*credentials_file); + filename_ = *std::move(credentials_file); } } diff --git a/tensorstore/kvstore/s3/s3_key_value_store.cc b/tensorstore/kvstore/s3/s3_key_value_store.cc index 0a887c95f..652fef35a 100644 --- a/tensorstore/kvstore/s3/s3_key_value_store.cc +++ b/tensorstore/kvstore/s3/s3_key_value_store.cc @@ -896,7 +896,7 @@ Future S3KeyValueStore::Write( } if (value && value->size() > kMaxS3PutSize) { // TODO: Support multi-part uploads of files larger than 5GB. - // Generally, asws-cli splits uploads which exceed ~8MB into multiple + // Generally, aws-cli splits uploads which exceed ~8MB into multiple // parts. return absl::InvalidArgumentError(absl::StrCat( "Object size ", value->size(), " exceeds S3 limit of ", kMaxS3PutSize)); @@ -931,7 +931,7 @@ Future S3KeyValueStore::Write( auto state = internal::MakeIntrusivePtr( std::move(self), std::move(options), std::move(ready), - std::move(object_url), std::move(*value), std::move(promise)); + std::move(object_url), *std::move(value), std::move(promise)); intrusive_ptr_increment(state.get()); // adopted by WriteTask::Admit. state->owner->write_rate_limiter().Admit(state.get(), diff --git a/tensorstore/kvstore/transaction.cc b/tensorstore/kvstore/transaction.cc index f8a6e28d0..680c42316 100644 --- a/tensorstore/kvstore/transaction.cc +++ b/tensorstore/kvstore/transaction.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -1661,7 +1662,7 @@ Future WriteViaExistingTransaction( node.reset(new Node); node->promise_ = promise; node->read_result_ = - value ? ReadResult::Value(std::move(*value), std::move(stamp)) + value ? ReadResult::Value(*std::move(value), std::move(stamp)) : ReadResult::Missing(std::move(stamp)); node->if_equal_no_value_ = if_equal_no_value; diff --git a/tensorstore/util/result.h b/tensorstore/util/result.h index d34a0805d..22d478b8d 100644 --- a/tensorstore/util/result.h +++ b/tensorstore/util/result.h @@ -859,7 +859,7 @@ internal_result::ChainResultType ChainResult( ::tensorstore::MaybeAddSourceLocation(_); \ return (error_expr); \ } \ - decl = std::move(*temp); \ + decl = *std::move(temp); \ /**/ /// Convenience macro for propagating errors when calling a function that @@ -894,7 +894,7 @@ internal_result::ChainResultType ChainResult( if (ABSL_PREDICT_FALSE(!temp)) { \ TENSORSTORE_CHECK_OK(temp.status()); \ } \ - decl = std::move(*temp); + decl = *std::move(temp); /// Convenience macro for checking errors when calling a function that returns a /// `tensorstore::Result`. diff --git a/tensorstore/util/result_test.cc b/tensorstore/util/result_test.cc index 89567ec18..c8460bdd9 100644 --- a/tensorstore/util/result_test.cc +++ b/tensorstore/util/result_test.cc @@ -546,7 +546,7 @@ TEST(ResultTest, Emplace) { Result> result = absl::UnknownError(""); result.emplace({3, 4, 5}); EXPECT_TRUE(result); - std::vector x = std::move(*result); + std::vector x = *std::move(result); EXPECT_EQ(3, x.size()); } } diff --git a/tensorstore/util/status.cc b/tensorstore/util/status.cc index 00fca3753..8f59d456a 100644 --- a/tensorstore/util/status.cc +++ b/tensorstore/util/status.cc @@ -50,7 +50,7 @@ void MaybeAddSourceLocationImpl(absl::Status& status, SourceLocation loc) { "%s:%d", filename, loc.line()))); } else { payload->Append(absl::StrFormat("\n%s:%d", filename, loc.line())); - status.SetPayload(kSourceLocationKey, std::move(*payload)); + status.SetPayload(kSourceLocationKey, *std::move(payload)); } #endif }