Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12866 )

Change subject: util: helper class for working with bitsets
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h
File src/kudu/util/bitset.h:

http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h@40
PS4, Line 40:
> Ah, we want to make sure 'non-existent' entries in enums are not accepted. 
Yeah, I'll add a comment since that isn't clear.


http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h@46
PS4, Line 46:   class iterator;
> Whoops, it slipped down a couple of lines.  I meant asking for COMPILE_ASSE
Done


http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h@68
PS4, Line 68:  'val' into th
> Well, at least make it iterator, not const_iterator if you want to mimic th
AFAICT set<> types don't support InsertOrUpdate(), at least when I try to 
compile:

 std::unordered_set<int> test;
 InsertOrUpdate(&test, 0);

It's unclear what the point of InsertOrUpdate() would be for set types anyway, 
vs InsertIfNotPresent() or InsertOrDie(), which both work and are tested. Same 
with FindOr* functions, those functions don't work with sets and ContainsKey() 
is preferred anyway.

That said, I've updated to use iterator instead of const_iterator just for 
parity with STL's interface, but nothing that works from map-util uses 
const_iterator AFAICT. Also added a test to test the output of insert().



--
To view, visit http://gerrit.cloudera.org:8080/12866
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0344fe94e9f9da9651164cb1b456c92d99dbdf4
Gerrit-Change-Number: 12866
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 27 Mar 2019 22:09:22 +0000
Gerrit-HasComments: Yes

Reply via email to