[Lldb-commits] [PATCH] D66546: Extend FindTypes w/ CompilerContext to allow filtering by language

Thu Aug 22 00:23:10 PDT 2019

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