[lld] Reject some lld specific one-dash long options

Project Infrastructure LLVM Dev List Archives
1

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 âš™ D79371 [ELF] Add convenience TableGen classes to enforce two dashes for options not supported by GNU ld 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?

2

Yes, please.

Joerg

3

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)

4

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.