Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12866 )
Change subject: util: helper class for working with bitsets ...................................................................... Patch Set 4: (4 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: size_t MaxVals > I don't think so, I haven't seen a way that can do it without defining MaxV Ah, we want to make sure 'non-existent' entries in enums are not accepted. Sure, that makes sense. http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h@46 PS4, Line 46: typedef IntType key_type; > Yep, these are publicly needed for use in map-util.h Whoops, it slipped down a couple of lines. I meant asking for COMPILE_ASSERT(MaxVals < 64, bitset_size_too_large); http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h@68 PS4, Line 68: const_iterator > This is just to match the STL interface, http://www.cplusplus.com/reference Well, at least make it iterator, not const_iterator if you want to mimic the STL interface? Otherwise I think it cannot be used by InsertOr* and alike: e.g., how is it supposed to update the value in case of InsertOrUpdate()? Does it work at all? I would think of adding tests for right away if you are about introducing such entity, hoping it will fit those methods. Otherwise, why not to keep it private for a while? http://gerrit.cloudera.org:8080/#/c/12866/4/src/kudu/util/bitset.h@113 PS4, Line 113: IntType& > I put these into godbolt https://godbolt.org/z/btGPzW and it seems compiler SGTM -- 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: 4 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 18:02:24 +0000 Gerrit-HasComments: Yes
