You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2020/03/14 20:54:08 UTC
[kudu-CR] [build] address compilation warnings from GCC 8.3.1
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15437
Change subject: [build] address compilation warnings from GCC 8.3.1
......................................................................
[build] address compilation warnings from GCC 8.3.1
While compiling on CentOS 8.1 with GCC 8.3.1, the following warnings
were observed:
src/kudu/util/env_posix.cc:1523:11: warning: unnecessary parentheses in declaration of ‘paths’ [-Wparentheses]
char *(paths[]) = { name_dup.get(), nullptr };
src/kudu/tablet/concurrent_btree.h: In instantiation of ‘void kudu::tablet::btree::CBTreeIterator<Traits>::SeekToLeaf(const kudu::Slice&) [with Traits = kudu::tablet::btree::SmallFanoutTraits]’:
src/kudu/tablet/concurrent_btree.h:1624:5: required from ‘bool kudu::tablet::btree::CBTreeIterator<Traits>::SeekAtOrAfter(const kudu::Slice&, bool*) [with Traits = kudu::tablet::btree::SmallFanoutTraits]’
src/kudu/tablet/cbtree-test.cc:502:5: required from here
src/kudu/tablet/concurrent_btree.h:1792:15: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘class kudu::tablet::btree::LeafNode<kudu::tablet::btree::SmallFanoutTraits>’ with no trivial copy-assignment [-Wclass-memaccess]
memcpy(&leaf_copy_, leaf, sizeof(leaf_copy_));
~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/kudu/tablet/concurrent_btree.h:677:7: note: ‘class kudu::tablet::btree::LeafNode<kudu::tablet::btree::SmallFanoutTraits>’ declared here
class LeafNode : public NodeBase<Traits> {
This patch fixes the former and squelches the latter with a pragma to
ignore the -Wclass-memaccess warning.
This patch doesn't contain any functional modifications.
Change-Id: Ifc1ff1bbba48846d91c39c2a53ff0ead1bf3f513
---
M src/kudu/tablet/concurrent_btree.h
M src/kudu/util/env_posix.cc
2 files changed, 8 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/15437/1
--
To view, visit http://gerrit.cloudera.org:8080/15437
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifc1ff1bbba48846d91c39c2a53ff0ead1bf3f513
Gerrit-Change-Number: 15437
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [build] address compilation warnings from GCC 8.3.1
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15437 )
Change subject: [build] address compilation warnings from GCC 8.3.1
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15437/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15437/1//COMMIT_MSG@24
PS1, Line 24: squelches the latter with a pragma to
: ignore the -Wclass-memaccess warning.
> Sorry for not being clear, I put double quotes around cheaper to imply LOC
np, I guess that's good that we see the same benefit from using the suggested alternative.
Thank you for the review!
--
To view, visit http://gerrit.cloudera.org:8080/15437
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc1ff1bbba48846d91c39c2a53ff0ead1bf3f513
Gerrit-Change-Number: 15437
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 16 Mar 2020 21:51:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] address compilation warnings from GCC 8.3.1
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: [build] address compilation warnings from GCC 8.3.1
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/15437
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ifc1ff1bbba48846d91c39c2a53ff0ead1bf3f513
Gerrit-Change-Number: 15437
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] address compilation warnings from GCC 8.3.1
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15437 )
Change subject: [build] address compilation warnings from GCC 8.3.1
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15437/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15437/1//COMMIT_MSG@24
PS1, Line 24: squelches the latter with a pragma to
: ignore the -Wclass-memaccess warning.
> Googling suggests that statically casting the destination to void* is also
+1
--
To view, visit http://gerrit.cloudera.org:8080/15437
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc1ff1bbba48846d91c39c2a53ff0ead1bf3f513
Gerrit-Change-Number: 15437
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 16 Mar 2020 19:53:58 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] address compilation warnings from GCC 8.3.1
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15437 )
Change subject: [build] address compilation warnings from GCC 8.3.1
......................................................................
Patch Set 2: Verified+1
Unrelated test failure in Java test org.apache.kudu.client.TestSplitKeyRange
--
To view, visit http://gerrit.cloudera.org:8080/15437
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc1ff1bbba48846d91c39c2a53ff0ead1bf3f513
Gerrit-Change-Number: 15437
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 16 Mar 2020 21:18:38 +0000
Gerrit-HasComments: No
[kudu-CR] [build] address compilation warnings from GCC 8.3.1
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15437 )
Change subject: [build] address compilation warnings from GCC 8.3.1
......................................................................
Patch Set 1: Verified+1
unrelated build fluke in IWYU:
14:14:19 CMake Error at CMakeLists.txt:782 (file):
14:14:19 file COPY cannot copy file
14:14:19 "/home/jenkins-slave/workspace/kudu-master/2/src/kudu/client/../scripts/first_argument.sh"
14:14:19 to
14:14:19 "/home/jenkins-slave/workspace/kudu-master/2/build/iwyu/bin/testdata/first_argument.sh":
14:14:19 Success.
14:14:19 Call Stack (most recent call first):
14:14:19 src/kudu/client/CMakeLists.txt:268 (ADD_KUDU_TEST)
However, I verified that IWYU is happy with this change (CentOS 6.6):
2020-03-14 15:19:15,141 INFO: Checking 2 file(s)...
IWYU would have edited 0 files on your behalf.
Built target iwyu
--
To view, visit http://gerrit.cloudera.org:8080/15437
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc1ff1bbba48846d91c39c2a53ff0ead1bf3f513
Gerrit-Change-Number: 15437
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 14 Mar 2020 22:21:06 +0000
Gerrit-HasComments: No
[kudu-CR] [build] address compilation warnings from GCC 8.3.1
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: [build] address compilation warnings from GCC 8.3.1
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/15437
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ifc1ff1bbba48846d91c39c2a53ff0ead1bf3f513
Gerrit-Change-Number: 15437
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] address compilation warnings from GCC 8.3.1
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15437 )
Change subject: [build] address compilation warnings from GCC 8.3.1
......................................................................
[build] address compilation warnings from GCC 8.3.1
While compiling on CentOS 8.1 with GCC 8.3.1, the following warnings
were observed:
src/kudu/util/env_posix.cc:1523:11: warning: unnecessary parentheses in declaration of ‘paths’ [-Wparentheses]
char *(paths[]) = { name_dup.get(), nullptr };
src/kudu/tablet/concurrent_btree.h: In instantiation of ‘void kudu::tablet::btree::CBTreeIterator<Traits>::SeekToLeaf(const kudu::Slice&) [with Traits = kudu::tablet::btree::SmallFanoutTraits]’:
src/kudu/tablet/concurrent_btree.h:1624:5: required from ‘bool kudu::tablet::btree::CBTreeIterator<Traits>::SeekAtOrAfter(const kudu::Slice&, bool*) [with Traits = kudu::tablet::btree::SmallFanoutTraits]’
src/kudu/tablet/cbtree-test.cc:502:5: required from here
src/kudu/tablet/concurrent_btree.h:1792:15: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘class kudu::tablet::btree::LeafNode<kudu::tablet::btree::SmallFanoutTraits>’ with no trivial copy-assignment [-Wclass-memaccess]
memcpy(&leaf_copy_, leaf, sizeof(leaf_copy_));
~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/kudu/tablet/concurrent_btree.h:677:7: note: ‘class kudu::tablet::btree::LeafNode<kudu::tablet::btree::SmallFanoutTraits>’ declared here
class LeafNode : public NodeBase<Traits> {
This patch doesn't contain any functional modifications, it only fixes
the warnings above.
Change-Id: Ifc1ff1bbba48846d91c39c2a53ff0ead1bf3f513
Reviewed-on: http://gerrit.cloudera.org:8080/15437
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M src/kudu/tablet/concurrent_btree.h
M src/kudu/util/env_posix.cc
2 files changed, 4 insertions(+), 4 deletions(-)
Approvals:
Alexey Serbin: Verified
Adar Dembo: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/15437
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifc1ff1bbba48846d91c39c2a53ff0ead1bf3f513
Gerrit-Change-Number: 15437
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] address compilation warnings from GCC 8.3.1
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15437 )
Change subject: [build] address compilation warnings from GCC 8.3.1
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15437/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15437/1//COMMIT_MSG@24
PS1, Line 24: unctional modifications, it only fixes
: the warnings above.
> Yes, this is also an option. I don't see much difference there, except for
Sorry for not being clear, I put double quotes around cheaper to imply LOC (rather than actual performance).
--
To view, visit http://gerrit.cloudera.org:8080/15437
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc1ff1bbba48846d91c39c2a53ff0ead1bf3f513
Gerrit-Change-Number: 15437
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 16 Mar 2020 21:41:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] address compilation warnings from GCC 8.3.1
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15437 )
Change subject: [build] address compilation warnings from GCC 8.3.1
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15437/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15437/1//COMMIT_MSG@24
PS1, Line 24: squelches the latter with a pragma to
: ignore the -Wclass-memaccess warning.
> +1
Yes, this is also an option. I don't see much difference there, except for ignored warnings directives explicitly signals there might be some dragons when reading the code.
Not sure it's 'cheaper', maybe only in number of lines of code :)
OK, let's replace with static_cast.
--
To view, visit http://gerrit.cloudera.org:8080/15437
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc1ff1bbba48846d91c39c2a53ff0ead1bf3f513
Gerrit-Change-Number: 15437
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 16 Mar 2020 20:28:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] address compilation warnings from GCC 8.3.1
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15437 )
Change subject: [build] address compilation warnings from GCC 8.3.1
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15437/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15437/1//COMMIT_MSG@24
PS1, Line 24: squelches the latter with a pragma to
: ignore the -Wclass-memaccess warning.
Googling suggests that statically casting the destination to void* is also an acceptable (and "cheaper") solution. What do you think?
--
To view, visit http://gerrit.cloudera.org:8080/15437
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc1ff1bbba48846d91c39c2a53ff0ead1bf3f513
Gerrit-Change-Number: 15437
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 16 Mar 2020 06:53:54 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] address compilation warnings from GCC 8.3.1
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15437
to look at the new patch set (#2).
Change subject: [build] address compilation warnings from GCC 8.3.1
......................................................................
[build] address compilation warnings from GCC 8.3.1
While compiling on CentOS 8.1 with GCC 8.3.1, the following warnings
were observed:
src/kudu/util/env_posix.cc:1523:11: warning: unnecessary parentheses in declaration of ‘paths’ [-Wparentheses]
char *(paths[]) = { name_dup.get(), nullptr };
src/kudu/tablet/concurrent_btree.h: In instantiation of ‘void kudu::tablet::btree::CBTreeIterator<Traits>::SeekToLeaf(const kudu::Slice&) [with Traits = kudu::tablet::btree::SmallFanoutTraits]’:
src/kudu/tablet/concurrent_btree.h:1624:5: required from ‘bool kudu::tablet::btree::CBTreeIterator<Traits>::SeekAtOrAfter(const kudu::Slice&, bool*) [with Traits = kudu::tablet::btree::SmallFanoutTraits]’
src/kudu/tablet/cbtree-test.cc:502:5: required from here
src/kudu/tablet/concurrent_btree.h:1792:15: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘class kudu::tablet::btree::LeafNode<kudu::tablet::btree::SmallFanoutTraits>’ with no trivial copy-assignment [-Wclass-memaccess]
memcpy(&leaf_copy_, leaf, sizeof(leaf_copy_));
~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/kudu/tablet/concurrent_btree.h:677:7: note: ‘class kudu::tablet::btree::LeafNode<kudu::tablet::btree::SmallFanoutTraits>’ declared here
class LeafNode : public NodeBase<Traits> {
This patch doesn't contain any functional modifications, it only fixes
the warnings above.
Change-Id: Ifc1ff1bbba48846d91c39c2a53ff0ead1bf3f513
---
M src/kudu/tablet/concurrent_btree.h
M src/kudu/util/env_posix.cc
2 files changed, 4 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/15437/2
--
To view, visit http://gerrit.cloudera.org:8080/15437
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifc1ff1bbba48846d91c39c2a53ff0ead1bf3f513
Gerrit-Change-Number: 15437
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)