[Lldb-commits] [lldb] [lldb][NFCI] Refactor AppleObjCClassDescriptorV2 method_t parsing functions (PR #163291)

[Felipe de Azevedo Piovezan via lldb-commits]

================
@@ -266,22 +267,47 @@ bool ClassDescriptorV2::method_list_t::Read(Process *process,
   return true;
 }
 
-bool ClassDescriptorV2::method_t::Read(Process *process, lldb::addr_t addr,
-                                       lldb::addr_t relative_selector_base_addr,
-                                       bool is_small, bool has_direct_sel) {
-  size_t ptr_size = process->GetAddressByteSize();
-  size_t size = GetSize(process, is_small);
+llvm::SmallVector<ClassDescriptorV2::method_t, 0>
----------------
felipepiovezan wrote:

> Is  SmallVector worth it when returning by value?

A `SmallVector<T, 0>` is supposed to be the exact same size as an `std::vector`, but with a more llvm-friendly API.

Because the small size is set to zero here, the programmer is expressing something: this is expected to always contain a lot of elements.


> ? If not, should we take an out parameter and have a sane default for the stack allocated number of elements? 

Doing this is doing what the compiler already does for us based on most ABIs: https://godbolt.org/z/9Yacveo4o

> Either way, I think it would be helpful to make this a typedef so you don't have to think about the number of elements in the template.

I'm generally opposed to this kind of typedef, it adds overhead when reading code, as readers always have to go lookup what the typedef this, to then just find out "oh, this is just a vector, why is this a typedef?!".

I understand it can be annoying to have to spell out the SmallSize, but it really is a small price... (also worth noting that a codebase more friendly to `auto` would not have that issue 👀 )

https://github.com/llvm/llvm-project/pull/163291