[PATCH] D42334: [NFC] Refactor Apple Accelerator Tables

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added a comment.

In https://reviews.llvm.org/D42334#985062, @labath wrote:

> I've uploaded https://reviews.llvm.org/D42420 so you can see my current work on the debug_names generation. It's still quite ugly and there are several interactions that I still have to figure out (e.g., how do I get the list of CUs I am indexing), but the thing I want to show is that there are more opportunities for code sharing here than on the parsing side: obviously the header is completely different, but the code for emission of some individual subtables can be shared as long as we can customize the top-level emit() function to control the order.  Also, the functions for gathering and processing the data (AddName, finalize) can largely be shared.


Great, this is definitely very interesting. I'm pleasantly surprised by what you managed to push down into the `AccelTable` base class.

> The one idea I think could make things cleaner, but I haven't tried out yet, is to split this code into two parts:
> 
> - one would deal purely with abstract data representation: gathering names, computing hashes, bucket counts, etc. This one would identical for both tables.
> - the other part would deal with serialization of the abstract representation. Here we would have two classes for the two table kinds plus a base class for common functionality.

That sounds great, you have my full support on that. Let me know if/how I can help. Do you have anything particular in mind to achieve this?

> I don't think any of that affects this patch directly, but it may be interesting for you to see where the common functionality is.

Definitely. This is part of the reason I want to get this change upstream now. The current implementation doesn't support all of Apple's use cases and making a change like this *after* having factored out the common parts will only make things harder. Especially because we want to support the same use cases with the DWARFv5 tables as well.


https://reviews.llvm.org/D42334