[PATCH] D78342: [lld] Add archive file support to Mach-O backend

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/test/MachO/archive.s:3
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %S/Inputs/archive.s -o %t.main
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %S/Inputs/archive2.s -o %t2
----------------
int3 wrote:
> smeenai wrote:
> > MaskRay wrote:
> > > Ktwu wrote:
> > > > int3 wrote:
> > > > > Nits: I don't think having a separate `Inputs/archive.s` is necessary; could just include its contents below and reference it  via `%s`.
> > > > > 
> > > > > Also, I was looking at some of the lld-ELF tests (e.g. `archive-fetch.s`), and it seems that we can create object files / archives with symbols but no corresponding code. So we could define those files inline too via `echo '.globl _boo | llvm-mc ...`
> > > > I'll definitely get rid of archive.s (this was back when I didn't know how Filecheck worked).
> > > > 
> > > > I like having explicit test files (although knowing about passing stuff straight to llvm-mc is a neat trick), so I'd prefer to keep the archive#.s if that's OK.
> > > For a definition, just write:
> > > 
> > > `echo '.globl _bar; _bar:' | llvm-mc -filetype=obj -triple=x86_64-apple-darwin - -o %t2.o`
> > > 
> > > For a reference:
> > > 
> > > `echo '.globl _bar' | llvm-mc -filetype=obj -triple=x86_64-apple-darwin - -o %t3.o`
> > > 
> > > Use applicable file extensions.
> > I don't have a super strong preference about separate test files vs. inlining, and there's definitely a point at which inlining becomes unreadable, but for tests where we just need to either define or reference a symbol, the inlining is nice in that you don't have to open up another file to understand what the test is trying to do.
> > 
> > Note that for the input files used by this test, the only thing that should matter is defining a symbol; the contents of the symbol shouldn't matter.
> > Note that for the input files used by this test, the only thing that should matter is defining a symbol; the contents of the symbol shouldn't matter.
> 
> +1. If we're going to use section names to test in symbol-order.s then it's just 3 statements that should fit onto one line too
(and if it's a matter of making sure the resulting test executable is runnable, then all we need is a `ret`, the `mov` can be omitted)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78342/new/

https://reviews.llvm.org/D78342