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

[Jonas Devlieghere 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>
----------------
JDevlieghere wrote:

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

I wasn't talking about RVO, I meant taking a `SmallVectorImpl` and letting the caller decide what amount of elements it expects. But that only makes sense if there's a meaningful value. If we knew that most Objective-C classes have less than say 8 methods, that would make the SmallVector an even better choice. It sounds like here you're just doing it for the llvm interface (which is a good enough reason in and by itself). 

> 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?!".

The argument in favor is that it's abstracting it away on purpose: you decided that the vector "always contain a lot of elements" or at least an unpredictable number of elements (that's how I read that `0`) and so you don't have to worry about. It's more about not having to _think_ about the size rather than having to spell it out. 

> also worth noting that a codebase more friendly to auto would not have that issue 👀

This sounds contradictory to your previous statement: unless you have an IDE that show the types of `auto` inline, this adds the same amount of (if not more) overhead that you seem to be arguing against. 

Anyway, I don't feel strongly. I would have made it a typedef. I won't stand in the way if you don't. 

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