In D111468#3066820, @jansvoboda11 wrote:

LGTM.

As a follow-up, do you think it would make sense to improve the documentation/comments around "index header maps"? Variable names refer to indexing while the documentation talks about building frameworks, which is confusing without referring back to the original Radar.

Fix windows build.

In D101835#2798031, @hubert.reinterpretcast wrote:

The tests for this tool are observed to be failing on multiple big-endian platforms:
clang-s390x-linux: https://lab.llvm.org/buildbot/#/builders/94/builds/4116
clang-ppc64be-linux: https://lab.llvm.org/buildbot/#/builders/52/builds/8035

LGTM

LGTM

Thank you!

Address dblaikie's suggestion and move specialization into YAMLTraits.

In D84770#2210049, @sameerarora101 wrote:

In D84770#2208826, @smeenai wrote:

In D84770#2204147, @sameerarora101 wrote:

I think I need to add V8 support under llvm/include/llvm/TextAPI/MachO/Architecture.def here:

///
/// ARM64 architectures sorted by cpu sub type id.
///
ARCHINFO(arm64, MachO::CPU_TYPE_ARM64, MachO::CPU_SUBTYPE_ARM64_ALL, 64)
ARCHINFO(arm64e, MachO::CPU_TYPE_ARM64, MachO::CPU_SUBTYPE_ARM64E, 64)

What should be the first element for that tuple in case of V8 (arm64_v8 or arm64v8 or maybe something else) ? Thanks.

Hmm, what necessitates the TAPI patch? arm64_v8 seems fine to me, though @ributzka and @cishida should weigh in there.

Converting a yaml file with Subtype V8 to an object file requires this patch. I was trying to add a test for V8, but my yaml was incorrectly parsed as an object file with unknown architecture.

In D85594#2207080, @gkm wrote:

@tschuett, I prefer not to do anything that involves semantics for DriverKit, since I don't have any domain knowledge. For that matter, perhaps I shouldn't even follow-through with this diff, which I only embarked upon at your suggestion because the original author of the MachO -platform_version diff (I commandeered it) had a TODO comment. I can code-monkey the TBDv4 testcase via simple substitution, but what about TBDv1 .. TBDv3? IDK, and don't want to spend time determining minimum version# for DriverKit. Do you know that?

Thanks for fixing this! one nit but LGTM

the landed patch has applied MaskRay's comments.

adopintg jhenderson's comments to increase Readablity & Coding Guidelines

Updating to address comments, add more const correctness, fix casing on FileCheck

the lint corrections will be/have been made in the revision that lands.

Updating this patch to just create inlining framework support, and I will attach lib/Object & nm support in a seperate patch

LGTM, Jürgen approved via email as well.

updating Kuba's patch with tests

LGTM, thank you for cleaning this up.

LGTM

will resolve the warning: invalid case style for variable '...' [readability-identifier-naming] in a future patch since it'll cause mismatching conventions through the whole file.

Updating to use LLVM_FALLTHROUGH for cases

This indirection seems like unnecessary work. Could you reimplement this as a direct conversion to std::string?

LGTM, thank you!

small comment, otherwise LGTM

nit: probably should update [TAPI] to [llvm][TextAPI/MachO] in the commit header.

LGTM, thanks!

Updating to fix copy/paste error