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 2021/02/06 06:59:49 UTC
[kudu-CR] [gutil] update map-util to benefit from C++17
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17035
Change subject: [gutil] update map-util to benefit from C++17
......................................................................
[gutil] update map-util to benefit from C++17
This patch updates map-util to benefits from C++17 features such as
try_emplace() and insert_or_assign() for dictionary containers: see
[1], [2], [3], [4] for details. At least, with try_emplace() a new
element isn't ever constructed if the container already contains
an element with the specified key.
[1] https://en.cppreference.com/w/cpp/container/map/try_emplace
[2] https://en.cppreference.com/w/cpp/container/map/insert_or_assign
[3] https://en.cppreference.com/w/cpp/container/unordered_map/try_emplace
[4] https://en.cppreference.com/w/cpp/container/unordered_map/insert_or_assign
Change-Id: I248c03e604855434ea3a0891c311461a455b428e
---
M src/kudu/gutil/map-util.h
1 file changed, 27 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/17035/1
--
To view, visit http://gerrit.cloudera.org:8080/17035
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I248c03e604855434ea3a0891c311461a455b428e
Gerrit-Change-Number: 17035
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [gutil] update map-util to benefit from C++17
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17035 )
Change subject: [gutil] update map-util to benefit from C++17
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17035
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I248c03e604855434ea3a0891c311461a455b428e
Gerrit-Change-Number: 17035
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Feb 2021 20:31:10 +0000
Gerrit-HasComments: No
[kudu-CR] [gutil] update map-util to benefit from C++17
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17035 )
Change subject: [gutil] update map-util to benefit from C++17
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h@446
PS2, Line 446: typename std::enable_if<
: std::is_same<typename Collection::key_type,
: typename Collection::value_type>::value,
: bool>::type
> nit: maybe add a note to the above comment so future readers don't find the
Done
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h@446
PS2, Line 446: typename std::enable_if<
: std::is_same<typename Collection::key_type,
: typename Collection::value_type>::value,
: bool>::type
> > Do you have a recipe for detecting for the presence of some particular me
Yup -- this template meta-programming looks a bit hairy sometimes :) Anyways, feel free to change this to be more effective/straightforward.
Thanks!
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h@470
PS2, Line 470: insert_or_assign
> At first I found it a bit odd that we're calling insert in an emplace funct
Ack.
--
To view, visit http://gerrit.cloudera.org:8080/17035
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I248c03e604855434ea3a0891c311461a455b428e
Gerrit-Change-Number: 17035
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Feb 2021 20:19:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] [gutil] update map-util to benefit from C++17
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17035 )
Change subject: [gutil] update map-util to benefit from C++17
......................................................................
[gutil] update map-util to benefit from C++17
This patch updates map-util to benefit from C++17 features such as
try_emplace() and insert_or_assign() for dictionary containers: see
[1], [2], [3], [4] for details. At least, with try_emplace() a new
element isn't ever constructed if the container already contains
an element with the specified key.
[1] https://en.cppreference.com/w/cpp/container/map/try_emplace
[2] https://en.cppreference.com/w/cpp/container/map/insert_or_assign
[3] https://en.cppreference.com/w/cpp/container/unordered_map/try_emplace
[4] https://en.cppreference.com/w/cpp/container/unordered_map/insert_or_assign
Change-Id: I248c03e604855434ea3a0891c311461a455b428e
Reviewed-on: http://gerrit.cloudera.org:8080/17035
Reviewed-by: Bankim Bhavsar <ba...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/gutil/map-util.h
1 file changed, 35 insertions(+), 24 deletions(-)
Approvals:
Bankim Bhavsar: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/17035
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I248c03e604855434ea3a0891c311461a455b428e
Gerrit-Change-Number: 17035
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] [gutil] update map-util to benefit from C++17
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17035 )
Change subject: [gutil] update map-util to benefit from C++17
......................................................................
Patch Set 2: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h@446
PS2, Line 446: typename std::enable_if<
: std::is_same<typename Collection::key_type,
: typename Collection::value_type>::value,
: bool>::type
> > The value_type for map is always pair<const key_type, mapped_type>. So,
nit: maybe add a note to the above comment so future readers don't find themselves asking this question?
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h@470
PS2, Line 470: insert_or_assign
At first I found it a bit odd that we're calling insert in an emplace function, but seems they're sometimes equivalent. https://en.cppreference.com/w/cpp/container/map/insert
> Inserts value. The overload (2) is equivalent to emplace(std::forward<P>(value)) and only participates in overload resolution if std::is_constructible<value_type, P&&>::value == true.
--
To view, visit http://gerrit.cloudera.org:8080/17035
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I248c03e604855434ea3a0891c311461a455b428e
Gerrit-Change-Number: 17035
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Feb 2021 18:23:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] [gutil] update map-util to benefit from C++17
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17035 )
Change subject: [gutil] update map-util to benefit from C++17
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h@446
PS2, Line 446: typename std::enable_if<
: std::is_same<typename Collection::key_type,
: typename Collection::value_type>::value,
: bool>::type
> The value_type for map is always pair<const key_type, mapped_type>. So, no
Probably, there is more straightforward way of differentiating there. The ideal case might be just probing for the existence of try_emplace() method.
I'm open for suggestions here.
--
To view, visit http://gerrit.cloudera.org:8080/17035
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I248c03e604855434ea3a0891c311461a455b428e
Gerrit-Change-Number: 17035
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Sat, 06 Feb 2021 23:47:50 +0000
Gerrit-HasComments: Yes
[kudu-CR] [gutil] update map-util to benefit from C++17
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Kudu Jenkins, Andrew Wong, Bankim Bhavsar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17035
to look at the new patch set (#2).
Change subject: [gutil] update map-util to benefit from C++17
......................................................................
[gutil] update map-util to benefit from C++17
This patch updates map-util to benefit from C++17 features such as
try_emplace() and insert_or_assign() for dictionary containers: see
[1], [2], [3], [4] for details. At least, with try_emplace() a new
element isn't ever constructed if the container already contains
an element with the specified key.
[1] https://en.cppreference.com/w/cpp/container/map/try_emplace
[2] https://en.cppreference.com/w/cpp/container/map/insert_or_assign
[3] https://en.cppreference.com/w/cpp/container/unordered_map/try_emplace
[4] https://en.cppreference.com/w/cpp/container/unordered_map/insert_or_assign
Change-Id: I248c03e604855434ea3a0891c311461a455b428e
---
M src/kudu/gutil/map-util.h
1 file changed, 27 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/17035/2
--
To view, visit http://gerrit.cloudera.org:8080/17035
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I248c03e604855434ea3a0891c311461a455b428e
Gerrit-Change-Number: 17035
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] [gutil] update map-util to benefit from C++17
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17035 )
Change subject: [gutil] update map-util to benefit from C++17
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h@446
PS2, Line 446: typename std::enable_if<
: std::is_same<typename Collection::key_type,
: typename Collection::value_type>::value,
: bool>::type
> The value_type for map is always pair<const key_type, mapped_type>. So, no: for map<string, string> everything works as expected.
Thanks for pointing that out.
--
To view, visit http://gerrit.cloudera.org:8080/17035
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I248c03e604855434ea3a0891c311461a455b428e
Gerrit-Change-Number: 17035
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Feb 2021 17:55:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] [gutil] update map-util to benefit from C++17
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17035 )
Change subject: [gutil] update map-util to benefit from C++17
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h@446
PS2, Line 446: typename std::enable_if<
: std::is_same<typename Collection::key_type,
: typename Collection::value_type>::value,
: bool>::type
> Do you have a recipe for detecting for the presence of some particular method using template meta-programming?
No, I don't.
I saw some online but they look complicated.
--
To view, visit http://gerrit.cloudera.org:8080/17035
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I248c03e604855434ea3a0891c311461a455b428e
Gerrit-Change-Number: 17035
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Feb 2021 19:16:27 +0000
Gerrit-HasComments: Yes
[kudu-CR] [gutil] update map-util to benefit from C++17
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Kudu Jenkins, Andrew Wong, Bankim Bhavsar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17035
to look at the new patch set (#3).
Change subject: [gutil] update map-util to benefit from C++17
......................................................................
[gutil] update map-util to benefit from C++17
This patch updates map-util to benefit from C++17 features such as
try_emplace() and insert_or_assign() for dictionary containers: see
[1], [2], [3], [4] for details. At least, with try_emplace() a new
element isn't ever constructed if the container already contains
an element with the specified key.
[1] https://en.cppreference.com/w/cpp/container/map/try_emplace
[2] https://en.cppreference.com/w/cpp/container/map/insert_or_assign
[3] https://en.cppreference.com/w/cpp/container/unordered_map/try_emplace
[4] https://en.cppreference.com/w/cpp/container/unordered_map/insert_or_assign
Change-Id: I248c03e604855434ea3a0891c311461a455b428e
---
M src/kudu/gutil/map-util.h
1 file changed, 35 insertions(+), 24 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/17035/3
--
To view, visit http://gerrit.cloudera.org:8080/17035
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I248c03e604855434ea3a0891c311461a455b428e
Gerrit-Change-Number: 17035
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] [gutil] update map-util to benefit from C++17
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17035 )
Change subject: [gutil] update map-util to benefit from C++17
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h@446
PS2, Line 446: typename std::enable_if<
: std::is_same<typename Collection::key_type,
: typename Collection::value_type>::value,
: bool>::type
> Probably, there is more straightforward way of differentiating there. The ideal case might be just probing for the existence of try_emplace() method.
Yeah that would be good.
Since the implementation doesn't actually add much of helper code, one option is to eliminate wrapper functions EmplaceIfNotPresent/EmplaceOrUpdate and let the callers directly invoke the collection specific emplace/try_emplace function.
--
To view, visit http://gerrit.cloudera.org:8080/17035
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I248c03e604855434ea3a0891c311461a455b428e
Gerrit-Change-Number: 17035
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Feb 2021 17:54:25 +0000
Gerrit-HasComments: Yes
[kudu-CR] [gutil] update map-util to benefit from C++17
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17035 )
Change subject: [gutil] update map-util to benefit from C++17
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h@446
PS2, Line 446: typename std::enable_if<
: std::is_same<typename Collection::key_type,
: typename Collection::value_type>::value,
: bool>::type
What happens if the key and value types of a map happens to be same like map<string, string> wouldn't this emplace() function be activated instead of the desired try_empace() below?
--
To view, visit http://gerrit.cloudera.org:8080/17035
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I248c03e604855434ea3a0891c311461a455b428e
Gerrit-Change-Number: 17035
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Sat, 06 Feb 2021 23:17:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] [gutil] update map-util to benefit from C++17
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17035 )
Change subject: [gutil] update map-util to benefit from C++17
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h@446
PS2, Line 446: typename std::enable_if<
: std::is_same<typename Collection::key_type,
: typename Collection::value_type>::value,
: bool>::type
> > Probably, there is more straightforward way of differentiating there. Th
I might be an option, yes. However, currently we strive to use map-util.h for all dictionary-relation operations. So, I don't think eliminating EmplaceIfNotPresent/EmplaceOrUpdate is a path forward.
Do you have a recipe for detecting for the presence of some particular method using template meta-programming?
--
To view, visit http://gerrit.cloudera.org:8080/17035
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I248c03e604855434ea3a0891c311461a455b428e
Gerrit-Change-Number: 17035
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Feb 2021 18:19:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] [gutil] update map-util to benefit from C++17
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17035 )
Change subject: [gutil] update map-util to benefit from C++17
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:
http://gerrit.cloudera.org:8080/#/c/17035/2/src/kudu/gutil/map-util.h@446
PS2, Line 446: typename std::enable_if<
: std::is_same<typename Collection::key_type,
: typename Collection::value_type>::value,
: bool>::type
> What happens if the key and value types of a map happens to be same like ma
The value_type for map is always pair<const key_type, mapped_type>. So, no: for map<string, string> everything works as expected.
--
To view, visit http://gerrit.cloudera.org:8080/17035
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I248c03e604855434ea3a0891c311461a455b428e
Gerrit-Change-Number: 17035
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Sat, 06 Feb 2021 23:43:37 +0000
Gerrit-HasComments: Yes