Fangrui Song via llvm-dev
2020-May-05 23:01 UTC
[llvm-dev] [lld] Reject some lld specific one-dash long options
GNU ld supports most long options with either one or two dashes. For compatibility, lld has to support both. For newer and lld specific options, we don't have such compatibility problem. I'd suggest we reject one-dash long options to avoid collision with short options. For example, * -lto-emit-obj can be read as -l to-emit-obj * -thinlto-cache-dir= means -t -h inlto-cache-dir= in GNU ld I created a patch https://reviews.llvm.org/D79371 to disallow some one-dash long options. These options are carefully chosen: I can't find anything depending on their one-dash forms. lld has a spell corrector for option names so a misspelled -lto-emit-obj can be identified quickly. Thoughts?
Joerg Sonnenberger via llvm-dev
2020-May-07 20:59 UTC
[llvm-dev] [lld] Reject some lld specific one-dash long options
On Tue, May 05, 2020 at 04:01:52PM -0700, Fangrui Song via llvm-dev wrote:> For newer and lld specific options, we don't have such compatibility > problem. I'd suggest we reject one-dash long options to avoid collision > with short options.Yes, please. Joerg
David Blaikie via llvm-dev
2020-Jul-29 17:21 UTC
[llvm-dev] [lld] Reject some lld specific one-dash long options
This email and the review didn't seem to mention a clause included in the final commit: "Some changed options are also used by gold, but I haven't seen their one-dash use cases outside of lld's testsuite." & I think there's a couple of issues with this particular patch, FWIW - backwards compatibility with lld is still a thing. Deprecation might've been more suitable - giving folks time to opt-out (usually it's easier for a build system to add a flag (such as from global LD_FLAGS, etc) to turn off warnings temporarily than it is to change the flags used inside a build) for any existing spellings. While certainly having a policy for all new flags going forward seems great. But compatibility with gold seems valuable too & wasn't mentioned & wasn't in the review - but added post-review. That seems like a slightly different discussion (I'd say three groups: New flags, existing lld-only flags, existing cross-linker flags (& I guess you've split that in two groups - bfd+lld flags and gold+lld flags, which is a fair point - gold, I believe, has fewer users than bfd)) (just came across this when I went to use a newer lld and had to change my -Wl,-gdb-index to -Wl,--gdb-index) On Tue, May 5, 2020 at 4:02 PM Fangrui Song via llvm-dev <llvm-dev at lists.llvm.org> wrote:> > GNU ld supports most long options with either one or two dashes. For > compatibility, lld has to support both. > > For newer and lld specific options, we don't have such compatibility > problem. I'd suggest we reject one-dash long options to avoid collision > with short options. For example, > > * -lto-emit-obj can be read as -l to-emit-obj > * -thinlto-cache-dir= means -t -h inlto-cache-dir= in GNU ld > > I created a patch https://reviews.llvm.org/D79371 to disallow some > one-dash long options. These options are carefully chosen: I can't find > anything depending on their one-dash forms. > > lld has a spell corrector for option names so a misspelled -lto-emit-obj can be > identified quickly. > > > Thoughts? > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Fangrui Song via llvm-dev
2020-Jul-29 18:44 UTC
[llvm-dev] [lld] Reject some lld specific one-dash long options
On 2020-07-29, David Blaikie wrote:>This email and the review didn't seem to mention a clause included in >the final commit: > >"Some changed options are also used by gold, but I haven't seen their >one-dash use cases outside of lld's testsuite." > >& I think there's a couple of issues with this particular patch, FWIW >- backwards compatibility with lld is still a thing. Deprecation >might've been more suitable - giving folks time to opt-out (usually >it's easier for a build system to add a flag (such as from global >LD_FLAGS, etc) to turn off warnings temporarily than it is to change >the flags used inside a build) for any existing spellings. While >certainly having a policy for all new flags going forward seems great. > >But compatibility with gold seems valuable too & wasn't mentioned & >wasn't in the review - but added post-review. That seems like a >slightly different discussion (I'd say three groups: New flags, >existing lld-only flags, existing cross-linker flags (& I guess you've >split that in two groups - bfd+lld flags and gold+lld flags, which is >a fair point - gold, I believe, has fewer users than bfd)) > >(just came across this when I went to use a newer lld and had to >change my -Wl,-gdb-index to -Wl,--gdb-index)I'll not interpret gold's support of --gdb-index that way. The documented form and the form used in projects is --gdb-index, never -gdb-index. gold accepting -gdb-index is merely an accidental implementation detail, partly due to how it mimicked GNU ld's lax option parsing. (If you remove some characters from --gdb-index, GNU ld will happily allow -gdndex. That is the grouped short options syntax.) The decision not having a deprecation warning step was a practical choice, with consideration of usage. FWIW I cannot find any occurrence "Wl,-gdb-index". My employer has a few temporary use cases of -Wl,-lto-*. We fixed it. Additionally, ld.lld has a suggestion feature of a misspelled option: ld.lld: error: unknown argument '-gdb-index', did you mean '--gdb-index' ld.lld: error: unknown argument '-thinlto-jobs=1', did you mean '--thinlto-jobs=1' You felt inconvenienced by the change. I feel sorry about that, but that is it. I don't think there was/is more action item we need to do.>On Tue, May 5, 2020 at 4:02 PM Fangrui Song via llvm-dev ><llvm-dev at lists.llvm.org> wrote: >> >> GNU ld supports most long options with either one or two dashes. For >> compatibility, lld has to support both. >> >> For newer and lld specific options, we don't have such compatibility >> problem. I'd suggest we reject one-dash long options to avoid collision >> with short options. For example, >> >> * -lto-emit-obj can be read as -l to-emit-obj >> * -thinlto-cache-dir= means -t -h inlto-cache-dir= in GNU ld >> >> I created a patch https://reviews.llvm.org/D79371 to disallow some >> one-dash long options. These options are carefully chosen: I can't find >> anything depending on their one-dash forms. >> >> lld has a spell corrector for option names so a misspelled -lto-emit-obj can be >> identified quickly. >> >> >> Thoughts? >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reasonably Related Threads
- [klibc:update-dash] dash: builtin: Reject malformed printf specifications with digits after '*'
- [klibc:update-dash] builtin: Reject malformed printf specifications with digits after '*'
- how to add different type of lines (short dash, long dash) into current plot)
- usr/dash/eval.c:277:19: warning: logical not is only applied to the left hand side of comparison
- The state of IRPGO (3 remaining work items)