The patch adds support for yaml2obj parsing auxiliary symbols for XCOFF.
Question 1: Why we need std::unique_ptr<> here?
Question 2: It looks that AuxEntries is optional in the symbol entry. Can we wrap it with Optional<>.
Nit: coding style.
It would be great to print the values of NumberOfAuxEntries and AuxCount in the error message.
Why the names of these two entries are not consistent with other fields (e.g., SectionOrLengthLo, SectionOrLengthHi)?
I'm not sure that these comments are really useful. I'd just get rid of them.
Should this really be an error? Could you potentially have an AUX_EXCEPT symbol in a 32-bit object (albeit a malformed one)? Or is it fundamentally impossible to write such a thing? Same goes for the AUX_STAT symbol below.
Remember, in yaml2obj, we want to be as permissive as possible. If you could imagine something existing, it should be allowed, even if the file format techincally prohibits it. This then allows you to write testing for code that reads the file (e.g. llvm-readobj) to ensure it properly handles such cases.
Rather than have one test-case per aux entry type, is it possible to have one which contains all entry types, either with a single symbol, or multiple symbols, as needed?
In the mulitple symbol case, name the symbols after their aux entry.
Rather than needing to duplicate the YAML between this and the defaults test case, you could consider implementing support for (if needed) and using the special [[MACRO=<none>]] syntax in the YAML. For ELF, this is equivalent to omitting the field, if no value is specified for it, which allows you to test default behaviour, using the same YAML as non-default behaviour.
Maybe spin this off into another patch? Seems unrelated to getting yaml2obj to actually produce the aux symbols, and needs its own testing.
I think there is no harm in using std::unique_ptr<> since it has less overhead. While wrapping it with Optional<> will cause failure in mapOptional().
I think this should be an error.
The [[MACRO=<none>]] syntax will work well to reduce duplication when the same field is used for multiple assignments. But in this test, most of the fields only need to be tested once.
You could store this a uint32_t to save the later cast, if you want.
I think we prefer static_cast
You've got two general test cases: the default and the non default. In both cases, you need a fair amount of basic scaffolding for the YAML to work (i.e. symbol details etc). In the default case, you can omit some fields, for the aux entries, but not in the full contents cases. I'm suggesting you merge the default and non-default YAML documents, not the 32/64 cases, so that the basic scaffolding isn't duplicated across the two files.
I don't think it's any easier to review separating default and non-default cases, personally. You'll see this pattern a lot in the ELF tests.