rjmccall added a comment.
A couple minor suggestions, then LGTM.
================
Comment at: clang/include/clang/AST/VTableBuilder.h:255
+ operator ArrayRef<T>() const { return {data(), size()}; };
+ };
+
----------------
Maybe this ought to be in LLVM as OwnedArrayRef? And the more minimal
implementation approach would be to inherit from MutableArrayRef<T> and just
add a destructor and a move constructor.
The implicit conversion to ArrayRef is dangerous, but only in ways that
ArrayRef is already dangerous.
================
Comment at: clang/include/clang/AST/VTableBuilder.h:260
+ // the vtable group contains a single vtable, an empty vector here represents
+ // the vector {0}.
+ FixedSizeVector<size_t> VTableIndices;
----------------
Ah. Yes, that's a reasonable solution.
================
Comment at: clang/lib/AST/VTableBuilder.cpp:986
+ /// vtable group.
+ SmallVector<size_t, 1> VTableIndices;
+
----------------
Same comment here. Might as well give this enough capacity for any reasonably
foreseeable case.
================
Comment at: clang/lib/CodeGen/CGVTables.cpp:641
+llvm::Type *CodeGenVTables::getVTableType(const VTableLayout &layout) {
+ SmallVector<llvm::Type *, 1> tys;
+ for (unsigned i = 0, e = layout.getNumVTables(); i != e; ++i) {
----------------
While 1 is definitely the most likely size, adding small amounts of extra
inline capacity to a temporary SmallVector has basically zero cost, and of
course those cases do happen. 4 sounds reasonable.
https://reviews.llvm.org/D22296
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
