From 9059514335a94c0a38ef314f343627de87b2a5b4 Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Thu, 8 Jul 2021 16:36:27 +0300 Subject: [PATCH] build, treewide: enable -Wpessimizing-move warning This warning prevents using std::move() where it can hurt - on an unnamed temporary or a named automatic variable being returned from a function. In both cases the value could be constructed directly in its final destination, but std::move() prevents it. Fix the handful of cases (all trivial), and enable the warning. Closes #8992 --- api/column_family.hh | 2 +- configure.py | 1 - cql3/constants.hh | 2 +- cql3/functions/functions.cc | 4 ++-- cql3/query_options.cc | 4 ++-- cql3/statements/select_statement.cc | 2 +- cql3/values.hh | 4 ++-- db/view/build_progress_virtual_reader.hh | 2 +- db/virtual_table.cc | 2 +- flat_mutation_reader.hh | 2 +- flat_mutation_reader_v2.hh | 2 +- hashers.cc | 2 +- locator/ec2_snitch.cc | 4 ++-- locator/production_snitch_base.cc | 2 +- message/messaging_service.cc | 2 +- querier.cc | 2 +- reader_concurrency_semaphore.cc | 2 +- repair/row_level.cc | 2 +- service/storage_proxy.cc | 2 +- test/boost/double_decker_test.cc | 18 +++++++++--------- test/boost/flat_mutation_reader_test.cc | 2 +- test/boost/mutation_reader_test.cc | 2 +- test/boost/sstable_datafile_test.cc | 6 +++--- test/raft/helpers.hh | 2 +- test/unit/bptree_compaction_test.cc | 2 +- test/unit/bptree_stress_test.cc | 4 ++-- transport/server.cc | 2 +- types.cc | 2 +- 28 files changed, 42 insertions(+), 43 deletions(-) diff --git a/api/column_family.hh b/api/column_family.hh index dc9510aa84..cb8c3b6008 100644 --- a/api/column_family.hh +++ b/api/column_family.hh @@ -77,7 +77,7 @@ struct map_reduce_column_families_locally { future> operator()(database& db) const { auto res = seastar::make_lw_shared>(std::make_unique(init)); return do_for_each(db.get_column_families(), [res, this](const std::pair>& i) { - *res = std::move(reducer(std::move(*res), mapper(*i.second.get()))); + *res = reducer(std::move(*res), mapper(*i.second.get())); }).then([res] { return std::move(*res); }); diff --git a/configure.py b/configure.py index b3c1242da8..13ee0f9e01 100755 --- a/configure.py +++ b/configure.py @@ -1289,7 +1289,6 @@ warnings = [ '-Wno-inconsistent-missing-override', '-Wno-defaulted-function-deleted', '-Wno-redeclared-class-member', - '-Wno-pessimizing-move', '-Wno-redundant-move', '-Wno-unsupported-friend', '-Wno-unused-variable', diff --git a/cql3/constants.hh b/cql3/constants.hh index cd757a44ba..81334be545 100644 --- a/cql3/constants.hh +++ b/cql3/constants.hh @@ -198,7 +198,7 @@ public: if (bytes.is_unset_value()) { return UNSET_VALUE; } - return ::make_shared(std::move(cql3::raw_value::make_value(bytes))); + return ::make_shared(cql3::raw_value::make_value(bytes)); } }; diff --git a/cql3/functions/functions.cc b/cql3/functions/functions.cc index 6bb27b96c3..d855706912 100644 --- a/cql3/functions/functions.cc +++ b/cql3/functions/functions.cc @@ -452,7 +452,7 @@ function_call::bind_and_get(const query_options& options) { if (!val) { throw exceptions::invalid_request_exception(format("Invalid null value for argument to {}", *_fun)); } - buffers.push_back(std::move(to_bytes_opt(val))); + buffers.push_back(to_bytes_opt(val)); } auto result = execute_internal(options.get_cql_serialization_format(), *_fun, std::move(buffers)); return cql3::raw_value_view::make_temporary(cql3::raw_value::make_value(result)); @@ -569,7 +569,7 @@ function_call::raw::execute(scalar_function& fun, std::vector> for (auto&& t : parameters) { assert(dynamic_cast(t.get())); auto&& param = static_cast(t.get())->get(query_options::DEFAULT); - buffers.push_back(std::move(to_bytes_opt(param))); + buffers.push_back(to_bytes_opt(param)); } return execute_internal(cql_serialization_format::internal(), fun, buffers); diff --git a/cql3/query_options.cc b/cql3/query_options.cc index 1f85076b46..a0b1b44f14 100644 --- a/cql3/query_options.cc +++ b/cql3/query_options.cc @@ -133,7 +133,7 @@ query_options::query_options(std::unique_ptr qo, lw_shared_ptr_values), std::move(qo->_value_views), qo->_skip_metadata, - std::move(query_options::specific_options{qo->_options.page_size, paging_state, qo->_options.serial_consistency, qo->_options.timestamp}), + query_options::specific_options{qo->_options.page_size, paging_state, qo->_options.serial_consistency, qo->_options.timestamp}, qo->_cql_serialization_format) { } @@ -145,7 +145,7 @@ query_options::query_options(std::unique_ptr qo, lw_shared_ptr_values), std::move(qo->_value_views), qo->_skip_metadata, - std::move(query_options::specific_options{page_size, paging_state, qo->_options.serial_consistency, qo->_options.timestamp}), + query_options::specific_options{page_size, paging_state, qo->_options.serial_consistency, qo->_options.timestamp}, qo->_cql_serialization_format) { } diff --git a/cql3/statements/select_statement.cc b/cql3/statements/select_statement.cc index 4bc40d4e8d..1988a0f1cd 100644 --- a/cql3/statements/select_statement.cc +++ b/cql3/statements/select_statement.cc @@ -1244,7 +1244,7 @@ indexed_table_select_statement::read_posting_list(service::storage_proxy& proxy, query::result_view::consume(*qr.query_result, std::move(partition_slice), cql3::selection::result_set_builder::visitor(builder, *_view_schema, *selection)); - return ::make_shared(std::move(result(builder.build()))); + return ::make_shared(result(builder.build())); }); } diff --git a/cql3/values.hh b/cql3/values.hh index c569e268f2..d152ca8cf5 100644 --- a/cql3/values.hh +++ b/cql3/values.hh @@ -201,10 +201,10 @@ class raw_value { {} public: static raw_value make_null() { - return raw_value{std::move(null_value{})}; + return raw_value{null_value{}}; } static raw_value make_unset_value() { - return raw_value{std::move(unset_value{})}; + return raw_value{unset_value{}}; } static raw_value make_value(const raw_value_view& view); static raw_value make_value(managed_bytes&& mb) { diff --git a/db/view/build_progress_virtual_reader.hh b/db/view/build_progress_virtual_reader.hh index 5d05a48ca9..f4af64b54d 100644 --- a/db/view/build_progress_virtual_reader.hh +++ b/db/view/build_progress_virtual_reader.hh @@ -143,7 +143,7 @@ class build_progress_virtual_reader { _previous_clustering_key = ck; mf = mutation_fragment(*_schema, _permit, clustering_row( std::move(ck), - std::move(scylla_in_progress_row.tomb()), + scylla_in_progress_row.tomb(), std::move(scylla_in_progress_row.marker()), std::move(legacy_in_progress_row))); } else if (mf.is_range_tombstone()) { diff --git a/db/virtual_table.cc b/db/virtual_table.cc index 9110658b70..a1e9c7a056 100644 --- a/db/virtual_table.cc +++ b/db/virtual_table.cc @@ -97,7 +97,7 @@ mutation_source memtable_filling_virtual_table::as_mutation_source() { }); } - return std::move(rd); + return rd; }); }; diff --git a/flat_mutation_reader.hh b/flat_mutation_reader.hh index 1b52c92709..eab70b6c26 100644 --- a/flat_mutation_reader.hh +++ b/flat_mutation_reader.hh @@ -557,7 +557,7 @@ public: // most close implementations are expexcted to return a ready future // so expedite prcessing it. if (f.available() && !f.failed()) { - return std::move(f); + return f; } // close must not fail return f.handle_exception([i = std::move(i)] (std::exception_ptr ep) mutable { diff --git a/flat_mutation_reader_v2.hh b/flat_mutation_reader_v2.hh index 77c875b015..022551832f 100644 --- a/flat_mutation_reader_v2.hh +++ b/flat_mutation_reader_v2.hh @@ -592,7 +592,7 @@ public: // most close implementations are expexcted to return a ready future // so expedite prcessing it. if (f.available() && !f.failed()) { - return std::move(f); + return f; } // close must not fail return f.handle_exception([i = std::move(i)] (std::exception_ptr ep) mutable { diff --git a/hashers.cc b/hashers.cc index 3477a31e50..e7f3c7e583 100644 --- a/hashers.cc +++ b/hashers.cc @@ -90,7 +90,7 @@ template bytes cryptopp_hasher::calculate(con typename cryptopp_hasher::impl::impl_type hash; unsigned char digest[size]; hash.CalculateDigest(digest, reinterpret_cast(s.data()), s.size()); - return std::move(bytes{reinterpret_cast(digest), size}); + return bytes{reinterpret_cast(digest), size}; } template class cryptopp_hasher; diff --git a/locator/ec2_snitch.cc b/locator/ec2_snitch.cc index 1c89ff520f..28a7c0220d 100644 --- a/locator/ec2_snitch.cc +++ b/locator/ec2_snitch.cc @@ -70,8 +70,8 @@ future ec2_snitch::aws_api_call(sstring addr, uint16_t port, sstring cm return connect(socket_address(inet_address{addr}, port)) .then([this, addr, cmd] (connected_socket fd) { _sd = std::move(fd); - _in = std::move(_sd.input()); - _out = std::move(_sd.output()); + _in = _sd.input(); + _out = _sd.output(); _zone_req = sstring("GET ") + cmd + sstring(" HTTP/1.1\r\nHost: ") +addr + sstring("\r\n\r\n"); diff --git a/locator/production_snitch_base.cc b/locator/production_snitch_base.cc index d8dba6a71d..625798c506 100644 --- a/locator/production_snitch_base.cc +++ b/locator/production_snitch_base.cc @@ -144,7 +144,7 @@ future<> production_snitch_base::load_property_file() { return f.dma_read_exactly(0, s); }); }).then([this] (temporary_buffer tb) { - _prop_file_contents = std::move(std::string(tb.get(), _prop_file_size)); + _prop_file_contents = std::string(tb.get(), _prop_file_size); parse_property_file(); return make_ready_future<>(); diff --git a/message/messaging_service.cc b/message/messaging_service.cc index 56a8515637..842ff1361a 100644 --- a/message/messaging_service.cc +++ b/message/messaging_service.cc @@ -879,7 +879,7 @@ messaging_service::make_sink_and_source_for_stream_mutation_fragments(utils::UUI auto rpc_handler = rpc()->make_client (utils::UUID, utils::UUID, utils::UUID, uint64_t, streaming::stream_reason, rpc::sink)>(messaging_verb::STREAM_MUTATION_FRAGMENTS); return rpc_handler(*rpc_client , plan_id, schema_id, cf_id, estimated_partitions, reason, sink).then_wrapped([sink, rpc_client] (future> source) mutable { return (source.failed() ? sink.close() : make_ready_future<>()).then([sink = std::move(sink), source = std::move(source)] () mutable { - return make_ready_future(value_type(std::move(sink), std::move(source.get0()))); + return make_ready_future(value_type(std::move(sink), source.get0())); }); }); }); diff --git a/querier.cc b/querier.cc index 19f84ba329..4fb147bddf 100644 --- a/querier.cc +++ b/querier.cc @@ -208,7 +208,7 @@ static std::unique_ptr find_querier(querier_cache::index& index, u tracing::trace(trace_state, "Found cached querier for key {} and range(s) {}", key, ranges); auto ptr = std::move(it->second); index.erase(it); - return std::move(ptr); + return ptr; } querier_cache::querier_cache(std::chrono::seconds entry_ttl) diff --git a/reader_concurrency_semaphore.cc b/reader_concurrency_semaphore.cc index d21e9e5a7b..208349b8ad 100644 --- a/reader_concurrency_semaphore.cc +++ b/reader_concurrency_semaphore.cc @@ -532,7 +532,7 @@ flat_mutation_reader reader_concurrency_semaphore::detach_inactive_reader(inacti break; } --_stats.inactive_reads; - return std::move(reader); + return reader; } void reader_concurrency_semaphore::evict(inactive_read& ir, evict_reason reason) noexcept { diff --git a/repair/row_level.cc b/repair/row_level.cc index 96bf20903c..c3693945d7 100644 --- a/repair/row_level.cc +++ b/repair/row_level.cc @@ -892,7 +892,7 @@ public: _stop_promise->set_value(); } }); - return std::move(ret); + return ret; } static std::unordered_map>& repair_meta_map(); diff --git a/service/storage_proxy.cc b/service/storage_proxy.cc index a52d96fab2..12e1e72f69 100644 --- a/service/storage_proxy.cc +++ b/service/storage_proxy.cc @@ -2628,7 +2628,7 @@ future<> storage_proxy::send_hint_to_all_replicas(frozen_mutation_and_schema fm_ return mutate_internal(std::move(ms), db::consistency_level::ALL, false, nullptr, empty_service_permit()); } - std::array ms{hint_wrapper { std::move(fm_a_s.fm.unfreeze(fm_a_s.s)) }}; + std::array ms{hint_wrapper { fm_a_s.fm.unfreeze(fm_a_s.s) }}; return mutate_internal(std::move(ms), db::consistency_level::ALL, false, nullptr, empty_service_permit()); } diff --git a/test/boost/double_decker_test.cc b/test/boost/double_decker_test.cc index f4fbaebf03..dff47c5b12 100644 --- a/test/boost/double_decker_test.cc +++ b/test/boost/double_decker_test.cc @@ -187,11 +187,11 @@ SEASTAR_THREAD_TEST_CASE(test_self_iterator) { collection c(compound_key::less_compare{}); test_data::compare cmp; - c.insert(1, std::move(test_data(1, "a")), cmp); - c.insert(1, std::move(test_data(1, "b")), cmp); - c.insert(2, std::move(test_data(2, "c")), cmp); - c.insert(3, std::move(test_data(3, "d")), cmp); - c.insert(3, std::move(test_data(3, "e")), cmp); + c.insert(1, test_data(1, "a"), cmp); + c.insert(1, test_data(1, "b"), cmp); + c.insert(2, test_data(2, "c"), cmp); + c.insert(3, test_data(3, "d"), cmp); + c.insert(3, test_data(3, "e"), cmp); auto erase_by_ptr = [&] (int key, std::string sub) { test_data* d = &*c.find(compound_key(key, sub), cmp); @@ -215,7 +215,7 @@ SEASTAR_THREAD_TEST_CASE(test_end_iterator) { collection c(compound_key::less_compare{}); test_data::compare cmp; - c.insert(1, std::move(test_data(1, "a")), cmp); + c.insert(1, test_data(1, "a"), cmp); auto i = std::prev(c.end()); BOOST_REQUIRE(*i == compound_key(1, "a")); @@ -291,7 +291,7 @@ SEASTAR_THREAD_TEST_CASE(test_insert_and_erase) { compound_key k(tests::random::get_int(100), tests::random::get_sstring(3)); if (c.find(k, cmp) == c.end()) { - auto it = c.insert(k.key, std::move(test_data(k.key, k.sub_key)), cmp); + auto it = c.insert(k.key, test_data(k.key, k.sub_key), cmp); BOOST_REQUIRE(*it == k); nr++; } @@ -331,7 +331,7 @@ SEASTAR_THREAD_TEST_CASE(test_compaction) { compound_key k(tests::random::get_int(400), tests::random::get_sstring(3)); if (c.find(k, cmp) == c.end()) { - auto it = c.insert(k.key, std::move(test_data(k.key, k.sub_key)), cmp); + auto it = c.insert(k.key, test_data(k.key, k.sub_key), cmp); BOOST_REQUIRE(*it == k); s.insert(std::move(k)); nr++; @@ -368,7 +368,7 @@ SEASTAR_THREAD_TEST_CASE(test_range_erase) { collection c(compound_key::less_compare{}); for (auto&& k : keys) { - c.insert(k.key, std::move(test_data(k.key, k.sub_key)), cmp); + c.insert(k.key, test_data(k.key, k.sub_key), cmp); } auto iter_at = [&c] (size_t at) -> collection::iterator { diff --git a/test/boost/flat_mutation_reader_test.cc b/test/boost/flat_mutation_reader_test.cc index 8425dae03b..0fbf6a9a83 100644 --- a/test/boost/flat_mutation_reader_test.cc +++ b/test/boost/flat_mutation_reader_test.cc @@ -770,7 +770,7 @@ SEASTAR_THREAD_TEST_CASE(test_mutation_reader_from_fragments_as_mutation_source) fragments.emplace_back(std::move(mf)); return stop_iteration::no; }, db::no_timeout).get(); - return std::move(fragments); + return fragments; }; auto rd = make_flat_mutation_reader_from_fragments(schema, permit, get_fragments(), range, slice); diff --git a/test/boost/mutation_reader_test.cc b/test/boost/mutation_reader_test.cc index 8e61046af6..71194f9ae0 100644 --- a/test/boost/mutation_reader_test.cc +++ b/test/boost/mutation_reader_test.cc @@ -3158,7 +3158,7 @@ flat_mutation_reader create_evictable_reader_and_evict_after_first_buffer( while(permit.semaphore().try_evict_one_inactive_read()); - return std::move(rd); + return rd; } flat_mutation_reader create_evictable_reader_and_evict_after_first_buffer( diff --git a/test/boost/sstable_datafile_test.cc b/test/boost/sstable_datafile_test.cc index 22315984fb..1ddff54762 100644 --- a/test/boost/sstable_datafile_test.cc +++ b/test/boost/sstable_datafile_test.cc @@ -4999,7 +4999,7 @@ SEASTAR_THREAD_TEST_CASE(test_scrub_segregate_stack) { size_t i = 0; for (const auto& segregated_fragment_stream : segregated_fragment_streams) { testlog.debug("Checking position monotonicity of segregated stream #{}", i++); - assert_that(make_flat_mutation_reader_from_fragments(schema, permit, std::move(copy_fragments(segregated_fragment_stream)))) + assert_that(make_flat_mutation_reader_from_fragments(schema, permit, copy_fragments(segregated_fragment_stream))) .has_monotonic_positions(); } } @@ -5010,7 +5010,7 @@ SEASTAR_THREAD_TEST_CASE(test_scrub_segregate_stack) { readers.reserve(segregated_fragment_streams.size()); for (const auto& segregated_fragment_stream : segregated_fragment_streams) { - readers.emplace_back(make_flat_mutation_reader_from_fragments(schema, permit, std::move(copy_fragments(segregated_fragment_stream)))); + readers.emplace_back(make_flat_mutation_reader_from_fragments(schema, permit, copy_fragments(segregated_fragment_stream))); } assert_that(make_combined_reader(schema, permit, std::move(readers))).has_monotonic_positions(); @@ -5022,7 +5022,7 @@ SEASTAR_THREAD_TEST_CASE(test_scrub_segregate_stack) { readers.reserve(segregated_fragment_streams.size()); for (const auto& segregated_fragment_stream : segregated_fragment_streams) { - readers.emplace_back(make_flat_mutation_reader_from_fragments(schema, permit, std::move(copy_fragments(segregated_fragment_stream)))); + readers.emplace_back(make_flat_mutation_reader_from_fragments(schema, permit, copy_fragments(segregated_fragment_stream))); } auto rd = assert_that(make_combined_reader(schema, permit, std::move(readers))); diff --git a/test/raft/helpers.hh b/test/raft/helpers.hh index 4298f4f5b0..cc4d3f5e90 100644 --- a/test/raft/helpers.hh +++ b/test/raft/helpers.hh @@ -92,7 +92,7 @@ raft::command create_command(T val) { raft::command command; ser::serialize(command, val); - return std::move(command); + return command; } raft::fsm_config fsm_cfg{.append_request_threshold = 1, .enable_prevoting = false}; diff --git a/test/unit/bptree_compaction_test.cc b/test/unit/bptree_compaction_test.cc index f48428bbd8..02d6ab92dc 100644 --- a/test/unit/bptree_compaction_test.cc +++ b/test/unit/bptree_compaction_test.cc @@ -75,7 +75,7 @@ int main(int argc, char **argv) { stress_compact_collection(cfg, /* insert */ [&] (int key) { test_key k(key); - auto ti = t->emplace(std::move(copy_key(k)), k); + auto ti = t->emplace(copy_key(k), k); assert(ti.second); }, /* erase */ [&] (int key) { diff --git a/test/unit/bptree_stress_test.cc b/test/unit/bptree_stress_test.cc index a93074fb27..234e692951 100644 --- a/test/unit/bptree_stress_test.cc +++ b/test/unit/bptree_stress_test.cc @@ -91,11 +91,11 @@ int main(int argc, char **argv) { test_key k(key); if (rep % 2 != 1) { - auto ir = t->emplace(std::move(copy_key(k)), k); + auto ir = t->emplace(copy_key(k), k); assert(ir.second); } else { auto ir = t->lower_bound(k); - ir.emplace_before(std::move(copy_key(k)), test_key_compare{}, k); + ir.emplace_before(copy_key(k), test_key_compare{}, k); } oracle[key] = key + 10; diff --git a/transport/server.cc b/transport/server.cc index e12757ef78..8ef9ff50d8 100644 --- a/transport/server.cc +++ b/transport/server.cc @@ -664,7 +664,7 @@ future<> cql_server::connection::process_request() { (void)_process_request_stage(this, istream, op, stream, seastar::ref(_client_state), tracing_requested, mem_permit) .then_wrapped([this, buf = std::move(buf), mem_permit, leave = std::move(leave)] (future>> response_f) mutable { try { - write_response(std::move(response_f.get0()), std::move(mem_permit), _compression); + write_response(response_f.get0(), std::move(mem_permit), _compression); _ready_to_respond = _ready_to_respond.finally([leave = std::move(leave)] {}); } catch (...) { clogger.error("request processing failed: {}", std::current_exception()); diff --git a/types.cc b/types.cc index 760e92aae4..34cd213efb 100644 --- a/types.cc +++ b/types.cc @@ -2697,7 +2697,7 @@ struct from_string_visitor { field_len[i] = fields[i].size(); } } - return std::move(concat_fields(fields, field_len)); + return concat_fields(fields, field_len); } bytes operator()(const collection_type_impl&) { // FIXME: