We have a mix of substituted lld (%lld) and hard-coded lld (ld64.lld) commands.
When testing with different versions of LLD, this would require going into every place
where lld is hard-coded and changing that. If we centralize it, this'll only require us
to modify it in only one place and will make it easy to run the same test suite. Plus,
this will make it be consistent with how we write other tests.
Details
- Reviewers
int3 oontvoo - Group Reviewers
Restricted Project - Commits
- rGef764ee20746: [lld-macho][nfc] Centralize usages of ld64.lld in tests
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Or %lld-no-arg, to be a bit more consistent with %no_fatal_warnings_lld. (I like that we're using hyphens instead of underscores though; IMO %no_fatal_warnings_lld should use hyphens too)
Bikeshedding: How about %lld-noarg? It's a bit more descriptive and only two characters longer.
Unfortunately, we can't start with %lld anymore otherwise it would get replaced with the value of %lld and then just append -noarg which makes the command invalid. I'm happy to change it to %no-arg-lld.
LGTM
Bikeshedding: How about %lld-noarg? It's a bit more descriptive and only two characters longer.
(slightly) prefer lld-noarg as well. :)
On the other hand, why doesn't re-using %lld plus additional args, which would override the defalut ones provided by %lld anyways, work?
Unfortunately, we can't start with %lld anymore otherwise it would get replaced with the value of %lld and then just append -noarg which makes the command invalid. I'm happy to change it to %no-arg-lld.
I stand corrected. If we move that substitution before we define %lld then it should work. So, in theory %lld-no-arg and %lld-noarg should work. Although, I do like %no-arg-lld a bit better since it matches with how no_fatal_warnings is defined.
why doesn't re-using %lld plus additional args, which would override the defalut ones provided by %lld anyways, work?
The substitution happens in order and it sees %lld within %lld-noarg first, so it ends up replacing that whole command and the command ends up failing a whole bunch of tests with ld64.lld: error: unknown argument '-fatal_warnings-raw'. So, moving that statement up should solve the issue. If the majority feels %lld-noarg is a better fit, I'm happy to change it to that; although, isn't noarg technically two words? (So, it should be %lld-no-arg?)
ok, gotcha!
So, moving that statement up should solve the issue. If the majority feels %lld-noarg is a better fit, I'm happy to change it to that; although, isn't noarg technically two words? (So, it should be %lld-no-arg?)
On a second thought, I think you have a good point on %no_arg_lld , to be consistent with _no_fatal_warning_* .
Overall, I gave it some thought on the naming. To be consistent with a part of the naming (e.g. no_fatal_warnings_), it makes sense to have no-arg be as the prefix as well. We do have a mix of - and _ (e.g. lld-watchos and no_fatal_warnings_lld). I broke the tie and slightly prefer -. Hence, the result no-arg-lld.