[PATCH] D139717: Revert "[Driver] Remove Joined -X"

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 16 09:39:31 PST 2022


MaskRay added a comment.

In D139717#4001704 <https://reviews.llvm.org/D139717#4001704>, @davide wrote:

> In D139717#4001702 <https://reviews.llvm.org/D139717#4001702>, @MaskRay wrote:
>
>> In D139717#4001688 <https://reviews.llvm.org/D139717#4001688>, @davide wrote:
>>
>>> In D139717#4001685 <https://reviews.llvm.org/D139717#4001685>, @MaskRay wrote:
>>>
>>>> In D139717#3998077 <https://reviews.llvm.org/D139717#3998077>, @manojgupta wrote:
>>>>
>>>>> Xlinker still works. Xcompiler is failing.
>>>>>
>>>>> A google search will show that Xcompiler is a wide-spread option used by many packages. Whether or not GCC supports it is not relevant. Please do not remove options just because you do not use them.
>>>>
>>>> Can you give an example how they use `-Xcompiler`?
>>>>
>>>>   % gcc -Xcompiler,-fpic -c a.c
>>>>   gcc: error: unrecognized command-line option ‘-Xcompiler,-fpic’
>>>>   % gcc -Xcompiler -fpic -c a.c
>>>>   gcc: error: unrecognized command-line option ‘-Xcompiler’; did you mean ‘--compile’?
>>>>
>>>> My commit message clearly says why the joined form is awkward and should be removed.
>>>> It seems that the many occurrences you found are likely for GNU libtool (`-Xcompiler foo` `-Wc,foo`), not for Clang Driver.
>>>
>>> This is not about the philosophical correctness of the patch, it's about the transition period and allowing consumers to migrate.
>>> If you want remove options, provide a deprecation window, and then remove. Noone is objecting about that.
>>
>> `-Xparser` has always been leading to such a warning: `warning: argument unused during compilation: '-Xparser' [-Wunused-command-line-argument]`, perhaps since forever when the option was added in the first place?
>> The message is different from `... deprecation ...` but isn't it sufficient as well?
>
> Yes. it is. "unused" doesn't mean "will go away".
> Sometimes if you pass linker flags to the compiler you get the same "unused" warning. Noone expects them to go away.
>
> Hope this helps.

Sorry, it doesn't help. A compiler option used a linker option leading to a "unused" warning is very different from this case.
I can understand that many `CFLAGS` options may end up in linking with a `-Wunused-command-line-argument` warning.
For practicality we must support them, as they are used in other phases.

In your case, at least as stated in the commit message, `This change is breaking internal builds.`
This is only for `-Xparser`. Then why can't you just add a `def : Flag<["-"], "Xparser">`?

It seems that GCC < 4.6 reports `g++: unrecognized option '-Xfoo'` but exit with 0 while GCC >= 4.6 reports `g++: error: unrecognized option '-Xaaa'` and exits with 1.
I don't think GCC ever supports `-Xcompiler` or `-Xparser`, so `IgnoredGCCCompat` is not justified.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139717/new/

https://reviews.llvm.org/D139717



More information about the cfe-commits mailing list