llvm dev - Jul 2020 - [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?

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

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

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