Yea sure, what email would you like me to use for the commit author email?

I'm not sure if there is special sign off around this file since it's so public facing but this LGTM

spacing

Move bool into BitcodeCompiler

https://reviews.llvm.org/D145273

In D145268#4168067, @int3 wrote:

Huh, I'm surprised that setting -object_path_lto has a functional difference; I thought it was only to make debugging the intermediate object files easier. Did you figure out why?

Also, did you figure out why LLD-ELF doesn't seem to need to set DefaultTriple?

update function to mutate instead of return

remove redundent return

https://reviews.llvm.org/D140491

Improve variable naming, require originFile existance

Rename CLI arg, add file path to warning / error

Remove literal deduping

Discussion: https://discord.com/channels/636084430946959380/636732801042874388/1055235620844355595
Previous discussion: https://reviews.llvm.org/D117250

Alternatively in this patch we could not include --strict-auto-link-options and we could remove --ignore-auto-link-option

After reading all this discussion, and seeing this well timed issue https://github.com/llvm/llvm-project/issues/59627 which is caused by this as well, I feel like we should take the opportunity to mirror ld64's behavior entirely, and potentially add a flag to opt-in to the more strict behavior that exists today. My opinion changed here since we talked about this last year because at least 5 people who have tried this have asked me about this specific issue. This issue is far more wide-spread than I originally thought in the community, and I don't think there's huge value in fixing these issues if your link succeeds anyways. Overall from this conversation it doesn't seem like anyone feels strongly enough about being strict on this issue that it justifies differing from ld64 and making adoption even just slightly harder.

The benefit of per library opt outs is you can make sure new violations don't sneak in. But either way we can deal with that part separately from this if we want to add a new flag like that

If we make them warnings, you would actually be able to disable the warning today by passing --ignore-auto-link-option=Foo as well, without any new flags

Here's the ld64 mirroring option: https://reviews.llvm.org/D140244

This is one of the options being discussed on https://reviews.llvm.org/D140225

I wonder if at this point we should just implement LD64's behavior with an off-by-default flag to be more strict? Specifically they ignore these failures and collect the missing ones and only print about them in the case the link fails

Improve test naming

Make tests address independent

Fix test addrs

I landed, thanks all!

rebase to run CI

Parameterize tests and add all test cases

Yes it does, and it looks like we get that today with lld because of platform == PLATFORM_MACOS

dropping this one and went for the discussed approach in https://reviews.llvm.org/D139347 which has no overlap with this one

2 thoughts:

It feels to me like we're defeating the purpose of LLVM_HOST_TRIPLE, but I imagine any other solution to this would require a much bigger hammer, so seems worth a try.

I submitted https://reviews.llvm.org/D139215 and https://reviews.llvm.org/D139223 based on this discussion. Please check them out!

this isn't exactly what was discussed in https://reviews.llvm.org/D130221 but it seems like making LinkGraphMaterializationUnit accessible is nicer for unit tests where we only care to test its behavior vs the whole system. lmk what you think. Locally I also have a patch to use MachOPlatform::isInitializerSection instead of the local version here but I wanted to prove the tests pass before and after so I didn't include it yet.

Add x86_64 example showing -no_pie isn't rejected in that case

I'm not super familiar with the history behind LLVM_HOST_TRIPLE, but one thing that definitely worries me about this specific change is there are other consumers of that variable, but only this 1 gets this fixup. I feel like the core issue is the mismatch between having a single LLVM_HOST_TRIPLE, but having cmake build a fat binary.

In D135531#3863681, @dblaikie wrote:

Looks like this and ( D135532 ) were sent for review, but then committed before that review was attempted/completed?

Could you please wait for reviews to be completed before submitting code.

If code doesn't need pre-commit review (per the guidelines in the LLVM documentation), it's OK/correct to submit it without review - but the general idea is that if something is sent for review, it's because it needed it, and so submitting without that review is problematic (we don't want to encourage folks to submit stuff because they're frustrated with/waiting too long for review - if they felt it needed review in the first place, delayed code review doesn't change that fact)