Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11593 )
Change subject: Move constructor and assignment operator for Schema ...................................................................... Patch Set 1: (13 comments) http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema-test.cc File src/kudu/common/schema-test.cc: http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema-test.cc@146 PS1, Line 146: NO_FATALS(copied_schema.ToString()); > warning: 'copied_schema' used after it was moved [bugprone-use-after-move] This is intentional. I'll have to look for a suppression. http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema-test.cc@158 PS1, Line 158: NO_FATALS(copied_schema.ToString()); > warning: 'copied_schema' used after it was moved [bugprone-use-after-move] Ditto. http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema.h File src/kudu/common/schema.h: http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema.h@415 PS1, Line 415: name_to_index_(NameToIndexMapAllocator(&name_to_index_bytes_)), > error: no matching constructor for initialization of 'kudu::Schema::NameToI Hm...works fine on my macOS machine, and the docs show that the constructor is supposed to exist as of C++11. http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema.cc File src/kudu/common/schema.cc: http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema.cc@174 PS1, Line 174: Schema& Schema::operator=(Schema&& other) { > warning: move assignment operators should be marked noexcept [hicpp-noexcep Done http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema.cc@177 PS1, Line 177: num_key_columns_ = std::move(other.num_key_columns_); > warning: std::move of the expression of the trivially-copyable type 'size_t Done http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema.cc@180 PS1, Line 180: name_to_index_bytes_ = std::move(other.name_to_index_bytes_); > warning: std::move of the expression of the trivially-copyable type 'int64_ Done http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema.cc@181 PS1, Line 181: id_to_index_ = std::move(other.id_to_index_); > warning: passing result of std::move() as a const reference argument; no mo Looks like implementing move stuff for IdMapping is easy. http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema.cc@182 PS1, Line 182: has_nullables_ = std::move(other.has_nullables_); > warning: std::move of the expression of the trivially-copyable type 'bool' Done http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema.cc@197 PS1, Line 197: Schema::Schema(Schema&& other) > warning: move constructors should be marked noexcept [hicpp-noexcept-move] Done http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema.cc@199 PS1, Line 199: num_key_columns_(std::move(other.num_key_columns_)), > warning: std::move of the expression of the trivially-copyable type 'size_t Done http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema.cc@202 PS1, Line 202: name_to_index_bytes_(std::move(other.name_to_index_bytes_)), > warning: std::move of the expression of the trivially-copyable type 'int64_ Done http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema.cc@204 PS1, Line 204: id_to_index_(std::move(other.id_to_index_)), > warning: passing result of std::move() as a const reference argument; no mo Ditto. http://gerrit.cloudera.org:8080/#/c/11593/1/src/kudu/common/schema.cc@205 PS1, Line 205: has_nullables_(std::move(other.has_nullables_)) { > warning: std::move of the expression of the trivially-copyable type 'bool' Done -- To view, visit http://gerrit.cloudera.org:8080/11593 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c827c84657875be4cf2bc565db03a686849d8e2 Gerrit-Change-Number: 11593 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Fri, 05 Oct 2018 07:06:53 +0000 Gerrit-HasComments: Yes
