MaskRay added a comment.

In D135076#3831347 <https://reviews.llvm.org/D135076#3831347>, @jhuber6 wrote:

> In D135076#3831340 <https://reviews.llvm.org/D135076#3831340>, @MaskRay wrote:
>
>> In D135076#3831307 <https://reviews.llvm.org/D135076#3831307>, @jhuber6 
>> wrote:
>>
>>> In D135076#3831298 <https://reviews.llvm.org/D135076#3831298>, @MaskRay 
>>> wrote:
>>>
>>>> We really want these `--offload-*` users to stick with one canonical form, 
>>>> not `-offload-*` in some places while `--offload-*` in other places.
>>>>
>>>> Another angle is that people find `-offload-*` working with a new clang 
>>>> may try `-offload-*` on an old clang and get `-o ffload-*`.
>>>>
>>>> And `-offload-*` doesn't help misspelled options.
>>>
>>> This is fine, as long as users get more distinct feedback that 
>>> `-offload-arch` isn't doing what they think it does. Normally we'd get some 
>>> error and a helpful suggestion if the option is misspelled, but with `-o` 
>>> options we don't get anything.
>>> My only qualm with the current state is that it's not obvious that 
>>> `-offload-arch` is actually `-o ffload-arch` for most cases.
>>
>> You can make `-oxxx` an error if offloading is used (`-o xxx` is still 
>> allowed). Then no `--offload-*` needs a `-offload-*` form.
>
> The problem here is that the `--offload-*` options are used to enable the 
> offloading toolchain for some targets (e.g. OpenMP). Would a check on `-o' 
> that emits a warning if it matches closely a known option work? E.g. 
> `-offload-arch` would warn that the user may mean `--offload-arch`.

If we don't have `Joined` `-o`, we won't have the option collision problem. But 
we have `Joined` `-o`, and we should not introduce new aliases to collide with 
`-o`.
I am fine if `Joined` `-o` can go away but there are too many uses so such a 
change would be destructive.

My idea is to just disallow `Joined` `-o` when targeting a specific environment 
(e.g. when offloading toolchain is used).

>> This patch probably provides some convenience but regresses other aspects, 
>> and I think it should be reverted.
>
> If you think this should be reverted then I'm fine with it. I would like to 
> see some kind of solution to this problem however, as I have dealt with this 
> problem many times when working with users.

As mentioned, making users stick with more forms instead of all using the 
canonical form will do harm in other aspects.
I have explained these aspects in my previous comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135076

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to