Merge "treewide: adjust for gcc 9" from Avi

"
gcc 9 complains a lot about pessimizing moves, narrowing conversions, and
has tighter deduction rules, plus other nice warnings. Fix problems found
by it, and make some non-problems compile without warnings.
"

* tag 'gcc9/v1' of https://github.com/avikivity/scylla:
  types: fix pessimizing moves
  thrift: fix pessimizing moves
  tests: fix pessimizing moves
  tests: cql_query_test: silence narrowing conversion warning
  test: cql_auth_syntax_test: fix ambiguity due to parser uninitialized<T>
  table: fix potentially wrong schema when reading from zero sstables
  storage_proxy: fix pessimizing moves
  memtable: fix pessimizing moves
  IDL: silence narrowing conversion in bool serializer
  compaction: fix pessimizing moves
  cache: fix pessimizing moves
  locator: fix pessimizing moves
  database: fix pessimizing moves
  cql: fix pessimizing moves
  cql parser: fix conversion from uninitalized<T> to optional<T> with gcc 9

cql3/Cql.g

@@ -125,6 +125,7 @@ struct uninitialized {
     uninitialized& operator=(uninitialized&&) = default;
     operator const T&() const & { return check(), *_val; }
     operator T&&() && { return check(), std::move(*_val); }
+    operator std::optional<T>&&() && { return check(), std::move(_val); }
     void check() const { if (!_val) { throw std::runtime_error("not intitialized"); } }
 };
 

cql3/functions/functions.cc

@@ -194,7 +194,7 @@ make_from_json_function(database& db, const sstring& keyspace, data_type t) {
         if (!json_value.isNull()) {
             parsed_json_value.emplace(t->from_json_object(json_value, sf));
         }
-        return std::move(parsed_json_value);
+        return parsed_json_value;
     });
 }
 

cql3/restrictions/single_column_primary_key_restrictions.hh

@@ -288,7 +288,7 @@ private:
                     if (def->type->is_reversed()) {
                         ranges.back().reverse();
                     }
-                    return std::move(ranges);
+                    return ranges;
                 }
 
                 ranges.reserve(cartesian_product_size(vec_of_values));
@@ -317,7 +317,7 @@ private:
                     }
                 }
 
-                return std::move(ranges);
+                return ranges;
             }
 
             auto values = r->values(options);
@@ -337,7 +337,7 @@ private:
             ranges.emplace_back(range_type::make_singular(ValueType::from_optional_exploded(*_schema, std::move(prefix))));
         }
 
-        return std::move(ranges);
+        return ranges;
     }
 
 public:

database.cc

@@ -551,7 +551,7 @@ do_parse_schema_tables(distributed<service::storage_proxy>& proxy, const sstring
             auto keyspace_name = r.template get_nonnull<sstring>("keyspace_name");
             names.emplace(keyspace_name);
         }
-        return std::move(names);
+        return names;
     }).then([&proxy, cf_name, func = std::move(func)] (std::set<sstring>&& names) mutable {
         return parallel_for_each(names.begin(), names.end(), [&proxy, cf_name, func = std::move(func)] (sstring name) mutable {
             if (is_system_keyspace(name)) {

locator/abstract_replication_strategy.cc

@@ -81,7 +81,7 @@ std::vector<inet_address> abstract_replication_strategy::get_natural_endpoints(c
         auto endpoints = calculate_natural_endpoints(search_token, _token_metadata);
         cached_endpoints.emplace(key_token, endpoints);
 
-        return std::move(endpoints);
+        return endpoints;
     }
 
     ++_cache_hits_count;

memtable.cc

@@ -678,7 +678,7 @@ memtable::make_flat_reader(schema_ptr s,
         if (fwd == streamed_mutation::forwarding::yes) {
             return make_forwardable(std::move(res));
         } else {
-            return std::move(res);
+            return res;
         }
     }
 }

partition_snapshot_reader.hh

@@ -325,7 +325,7 @@ make_partition_snapshot_flat_reader(schema_ptr s,
     if (fwd) {
         return make_forwardable(std::move(res)); // FIXME: optimize
     } else {
-        return std::move(res);
+        return res;
     }
 }
 

serializer.hh

@@ -199,7 +199,21 @@ struct integral_serializer {
     }
 };
 
-template<> struct serializer<bool> : public integral_serializer<int8_t> {};
+template<> struct serializer<bool> {
+    template <typename Input>
+    static bool read(Input& i) {
+        return deserialize_integral<uint8_t>(i);
+    }
+    template< typename Output>
+    static void write(Output& out, bool v) {
+        serialize_integral(out, uint8_t(v));
+    }
+    template <typename Input>
+    static void skip(Input& i) {
+        read(i);
+    }
+
+};
 template<> struct serializer<int8_t> : public integral_serializer<int8_t> {};
 template<> struct serializer<uint8_t> : public integral_serializer<uint8_t> {};
 template<> struct serializer<int16_t> : public integral_serializer<int16_t> {};

service/storage_proxy.cc

@@ -1107,7 +1107,7 @@ future<> storage_proxy::mutate_begin(std::vector<unique_response_handler> ids, d
         // call before send_to_live_endpoints() for the same reason as above
         auto f = response_wait(response_id, timeout);
         send_to_live_endpoints(protected_response.release(), timeout); // response is now running and it will either complete or timeout
-        return std::move(f);
+        return f;
     });
 }
 
@@ -3144,7 +3144,7 @@ std::vector<gms::inet_address> storage_proxy::get_live_endpoints(keyspace& ks, c
     std::vector<gms::inet_address> eps = rs.get_natural_endpoints(token);
     auto itend = boost::range::remove_if(eps, std::not1(std::bind1st(std::mem_fn(&gms::gossiper::is_alive), &gms::get_local_gossiper())));
     eps.erase(itend, eps.end());
-    return std::move(eps);
+    return eps;
 }
 
 std::vector<gms::inet_address> storage_proxy::get_live_sorted_endpoints(keyspace& ks, const dht::token& token) {

sstables/leveled_compaction_strategy.hh

@@ -79,7 +79,7 @@ compaction_descriptor leveled_compaction_strategy::get_sstables_for_compaction(c
 
     if (!candidate.sstables.empty()) {
         leveled_manifest::logger.debug("leveled: Compacting {} out of {} sstables", candidate.sstables.size(), cfs.get_sstables()->size());
-        return std::move(candidate);
+        return candidate;
     }
 
     // if there is no sstable to compact in standard way, try compacting based on droppable tombstone ratio

table.cc

@@ -343,7 +343,7 @@ table::make_sstable_reader(schema_ptr s,
         if (pr.is_singular() && pr.start()->value().has_key()) {
             const dht::ring_position& pos = pr.start()->value();
             if (dht::shard_of(pos.token()) != engine().cpu_id()) {
-                return mutation_source([s] (
+                return mutation_source([] (
                         schema_ptr s,
                         const dht::partition_range& pr,
                         const query::partition_slice& slice,

tests/cql_auth_syntax_test.cc

@@ -186,11 +186,11 @@ BOOST_AUTO_TEST_CASE(list_permissions) {
 BOOST_AUTO_TEST_CASE(user_or_role_name) {
     const auto test = make_tester(&cql3_parser::CqlParser::userOrRoleName);
 
-    BOOST_REQUIRE_EQUAL(static_cast<cql3::role_name>(test("SaM")).to_string(), "sam");
-    BOOST_REQUIRE_EQUAL(static_cast<cql3::role_name>(test("'SaM'")).to_string(), "SaM");
-    BOOST_REQUIRE_EQUAL(static_cast<cql3::role_name>(test("\"SaM\"")).to_string(), "SaM");
+    BOOST_REQUIRE_EQUAL(static_cast<cql3::role_name&&>(test("SaM")).to_string(), "sam");
+    BOOST_REQUIRE_EQUAL(static_cast<cql3::role_name&&>(test("'SaM'")).to_string(), "SaM");
+    BOOST_REQUIRE_EQUAL(static_cast<cql3::role_name&&>(test("\"SaM\"")).to_string(), "SaM");
     // Unreserved keyword.
-    BOOST_REQUIRE_EQUAL(static_cast<cql3::role_name>(test("LisT")).to_string(), "list");
+    BOOST_REQUIRE_EQUAL(static_cast<cql3::role_name&&>(test("LisT")).to_string(), "list");
 }
 
 BOOST_AUTO_TEST_CASE(role_options) {

tests/cql_query_test.cc

@@ -3484,7 +3484,7 @@ SEASTAR_TEST_CASE(test_select_with_mixed_order_table) {
                 test_cases.emplace_back(slice_test_type{gt_range, true, lt_range,true});
             } else {
                 for(int i=0; i<=3; i++) {
-                    test_cases.emplace_back(slice_test_type{gt_range, i&1, lt_range, i&2});
+                    test_cases.emplace_back(slice_test_type{gt_range, bool(i&1), lt_range, bool(i&2)});
                 }
             }
         };

tests/data_listeners_test.cc

@@ -96,9 +96,9 @@ results test_data_listeners(cql_test_env& e, sstring cf_name) {
                 results res{li->read, li->write};
                 testlog.info("uninstalled listener {}: rd={} wr={}", li, li->read, li->write);
                 db.data_listeners().uninstall(li);
-                return std::move(res);
+                return res;
             }
-            return std::move(results{});
+            return results{};
         },
         results{},
         [] (results res, results li_res) {

tests/limiting_data_source_test.cc

@@ -86,7 +86,7 @@ data_source prepare_test_skip() {
     BOOST_REQUIRE_EQUAL(1, buf.size());
     BOOST_REQUIRE_EQUAL(0, buf[0]);
     // At this point we have 9 chars buffered in limiting_data_source_impl
-    return std::move(tested);
+    return tested;
 }
 
 }

tests/mutation_source_test.cc

@@ -480,7 +480,7 @@ static void test_streamed_mutation_forwarding_guarantees(populate_fn populate) {
             nullptr,
             streamed_mutation::forwarding::yes));
         res.produces_partition_start(m.decorated_key());
-        return std::move(res);
+        return res;
     };
 
     auto verify_range = [&] (flat_reader_assertions& sm, int start, int end) {

tests/querier_cache.cc

@@ -165,7 +165,7 @@ public:
         , _mutation_source([this] (schema_ptr, const dht::partition_range& range) {
             auto rd = flat_mutation_reader_from_mutations(_mutations, range);
             rd.set_max_buffer_size(max_reader_buffer_size);
-            return std::move(rd);
+            return rd;
         }) {
     }
 

tests/row_cache_test.cc

@@ -2659,7 +2659,7 @@ SEASTAR_TEST_CASE(test_random_row_population) {
             auto rd = cache.make_reader(s.schema(), pr, slice ? *slice : s.schema()->full_slice());
             rd.set_max_buffer_size(1);
             rd.fill_buffer(db::no_timeout).get();
-            return std::move(rd);
+            return rd;
         };
 
         std::vector<query::clustering_range> ranges;
@@ -2774,7 +2774,7 @@ SEASTAR_TEST_CASE(test_continuity_is_populated_when_read_overlaps_with_older_ver
             auto rd = cache.make_reader(s.schema(), pr);
             rd.set_max_buffer_size(1);
             rd.fill_buffer(db::no_timeout).get();
-            return std::move(rd);
+            return rd;
         };
 
         {
@@ -2902,7 +2902,7 @@ SEASTAR_TEST_CASE(test_continuity_population_with_multicolumn_clustering_key) {
             auto rd = cache.make_reader(s, pr, slice ? *slice : s->full_slice());
             rd.set_max_buffer_size(1);
             rd.fill_buffer(db::no_timeout).get();
-            return std::move(rd);
+            return rd;
         };
 
         {
@@ -3009,7 +3009,7 @@ SEASTAR_TEST_CASE(test_concurrent_setting_of_continuity_on_read_upper_bound) {
             auto rd = cache.make_reader(s.schema(), pr, slice ? *slice : s.schema()->full_slice());
             rd.set_max_buffer_size(1);
             rd.fill_buffer(db::no_timeout).get();
-            return std::move(rd);
+            return rd;
         };
 
         {
@@ -3073,7 +3073,7 @@ SEASTAR_TEST_CASE(test_tombstone_merging_of_overlapping_tombstones_in_many_versi
             auto rd = cache.make_reader(s.schema());
             rd.set_max_buffer_size(1);
             rd.fill_buffer(db::no_timeout).get();
-            return std::move(rd);
+            return rd;
         };
 
         apply(cache, underlying, m1);
@@ -3111,7 +3111,7 @@ SEASTAR_TEST_CASE(test_concurrent_reads_and_eviction) {
             auto rd = cache.make_reader(s, pr, slice);
             rd.set_max_buffer_size(3);
             rd.fill_buffer(db::no_timeout).get();
-            return std::move(rd);
+            return rd;
         };
 
         const int n_readers = 3;

tests/sstable_3_x_test.cc

@@ -1349,7 +1349,7 @@ SEASTAR_THREAD_TEST_CASE(test_uncompressed_compound_static_row_read) {
         columns.push_back({s_text_cdef, utf8_type->from_string(text_val)});
         columns.push_back({s_inet_cdef, inet_addr_type->from_string(inet_val)});
 
-        return std::move(columns);
+        return columns;
     };
 
     assert_that(sst.read_rows_flat())
@@ -1692,7 +1692,7 @@ static void test_partition_key_with_values_of_different_types_read(const sstring
         columns.push_back({uuid_cdef, uuid_type->from_string(uuid_val)});
         columns.push_back({text_cdef, utf8_type->from_string(text_val)});
 
-        return std::move(columns);
+        return columns;
     };
 
     assert_that(sst.read_rows_flat())
@@ -1860,7 +1860,7 @@ SEASTAR_THREAD_TEST_CASE(test_uncompressed_subset_of_columns_read) {
             columns.push_back({text_cdef, utf8_type->from_string(*text_val)});
         }
 
-        return std::move(columns);
+        return columns;
     };
 
     assert_that(sst.read_rows_flat())
@@ -2866,7 +2866,7 @@ SEASTAR_THREAD_TEST_CASE(test_uncompressed_collections_read) {
             });
         });
 
-        return std::move(assertions);
+        return assertions;
     };
 
     std::vector<column_id> ids{set_cdef->id, list_cdef->id, map_cdef->id};

tests/top_k_test.cc

@@ -50,7 +50,7 @@ vector<unsigned> count(const utils::space_saving_top_k<unsigned>::results& res)
     for (auto& c : res) {
         v.push_back(c.count);
     }
-    return std::move(v);
+    return v;
 }
 
 //---------------------------------------------------------------------------------------------

thrift/handler.cc

@@ -1188,7 +1188,7 @@ private:
         }
         def.__set_cf_defs(cfs);
         def.__set_durable_writes(meta->durable_writes());
-        return std::move(def);
+        return def;
     }
     static std::optional<index_metadata> index_metadata_from_thrift(const ColumnDef& def) {
         std::optional<sstring> idx_name;

types.cc

@@ -3511,7 +3511,7 @@ static std::vector<sstring_view> split_field_strings(sstring_view v) {
         prev_ch = v[i];
     }
     result.push_back(v.substr(prev, v.size() - prev));
-    return std::move(result);
+    return result;
 }
 
 // Replace "\:" with ":" and "\@" with "@".
@@ -3521,7 +3521,7 @@ static std::string unescape(sstring_view s) {
     std::string result(s);
     result = std::regex_replace(result, escaped_colon_re, ":");
     result = std::regex_replace(result, escaped_at_re, "@");
-    return std::move(result);
+    return result;
 }
 
 // Concat list of bytes into a single bytes.
@@ -3539,7 +3539,7 @@ static bytes concat_fields(const std::vector<bytes>& fields, const std::vector<i
             it = std::copy(std::begin(fields[i]), std::end(fields[i]), it);
         }
     }
-    return std::move(result);
+    return result;
 }
 
 bytes