[lld] [lld-macho] Implement ObjC category merging (-objc_category_merging) (PR #82928)

Kyungwoo Lee via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 29 09:02:56 PST 2024


kyulee-com wrote:

> Yes, I do use asserts in several places where the failure would assert-worthy. Should I just make everything asserts ? Issue with that is that it may end up with no errors / possibly corrupt data or ambiguous crashes in release builds.

Do you expected such a many corrupted data in the catlist section?
Are these error message critical vs. are they worth as warning? In either case, we need to provide each test case with (plausible) input that matches individual error or warning message, being introduced here.

Given this large change, and expected large set  of test cases, you might also consider layering this change (with different PRs). E.g, you plug in reader + merger + writer into a single place, which you might make them separate. In the first one or two (reader/merger) part which just parses the expected catlist and its contents, but not actually updating or writing merged date yet. In the following change (writer), you could actually apply your change.
With a structured format, it might be easier to handle error case with appropriate test cases as needed. 
Also, it might be easier to be extended, which you mentioned about integrating the merged categories directly into the base clasee.

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


More information about the llvm-commits mailing list