labath added a comment.
The functionality seems fine, and I like how you got rid of the extra callbacks
in the plugin manager in favour of passing the sets explicitly. I'm not really
a fan of inheriting from standard containers. And though the motivation here is
stronger than in the previous case, it is not without its problems -- for
instance half of this patch uses the new `LanguageSet` type, whereas the other
(SymbolFile::FindTypes) still uses llvm::SmallBitVector. This also nicely
demonstrates the main problem with inheriting from classes which aren't meant
to be inherited (that they can be accidentally sliced) and means that this
isn't as type safe as one might hope..
Now, I am not sure what all of that means here. This functionality is not so
widespread that it makes sense to develop a full-blown class to do what you
need here, so it may be best to just go with what you have here, and possibly
rework this if it becomes more widespread in the future.... It's a pitty that
std::bitset does not have a richer interface for accessing the bits. Otherwise,
you could just do `typedef std::bitset<eNumLanguages> LanguageSet` and be done
with it..
================
Comment at: lldb/source/Symbol/TypeSystem.cpp:36-37
+ : llvm::SmallBitVector(num_small_bitvector_bits, 0) {
+ uint32_t mask32[2];
+ memcpy(mask32, &mask, 8);
+ setBitsInMask(mask32, num_small_bitvector_bits / 8);
----------------
Will this work correctly on big endian too?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66546/new/
https://reviews.llvm.org/D66546
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
