dblaikie added a comment.

In D158137#4599481 <https://reviews.llvm.org/D158137#4599481>, @MaskRay wrote:

> In D158137#4599461 <https://reviews.llvm.org/D158137#4599461>, @dblaikie 
> wrote:
>
>> In D158137#4597491 <https://reviews.llvm.org/D158137#4597491>, @dexonsmith 
>> wrote:
>>
>>> In D158137#4597009 <https://reviews.llvm.org/D158137#4597009>, @MaskRay 
>>> wrote:
>>>
>>>> In D158137#4596948 <https://reviews.llvm.org/D158137#4596948>, @dexonsmith 
>>>> wrote:
>>>>
>>>>> Can you explain the downside of leaving behind an alias?
>>>>
>>>> Two minor ones. (a) Existing `-Wno-overriding-t-option` will not notice 
>>>> that they need to migrate and (b) Clang has accrued tiny tech debt.
>>>> If we eventually remove `-Wno-overriding-t-option` for tidiness, we will 
>>>> have to break `-Werror -Wno-overriding-t-option` users.
>>>
>>> I guess it's not clear to me we'd need to remove the alias. The usual 
>>> policy (I think?) is that clang driver options don't disappear. It seems 
>>> like a small piece of debt to maintain the extra alias in this case, and if 
>>> it's kept, then users don't actually need to migrate. And then you can feel 
>>> safe updating Darwin.cpp as well.
>>
>> +1 to this, FWIW - I wouldn't consider it technical debt to keep a 
>> compatible warning flag name that's been around for a decade & isn't a name 
>> we're trying to free up for some other use or because it causes any great 
>> confusion, etc.
>
> I think my previous comment has answered this.
> Think: every Clang release may emit some new warnings. `-Werror` users has 
> actually provided a promise that they will fix reasonable toolchain changes. 
> Toolchain contributors check how disruptive a change will be, but cannot 
> promise that we never emit new warnings.
> In this case, `overriding-t-options` seems a fairly rare and LLVM/Clang side 
> has far too many other fp-model/fast-math changes to make "whether we will 
> get a new warning" a fairly minor issue.
> I am unfamiliar with Darwin.cpp though. If it turns out that want to disable 
> the warning even with `-Wno-overriding-t-option`, we can add a workaround 
> specific to Darwin.cpp, but not to fp-model/Tc.

Sure, I understand that we can break these things - like if we totally remove a 
warning, I wouldn't be averse to removing the flag/not leaving it there for 
backwards "compatibility" (when it's misleading/doesn't actually trigger the 
warning, for instance). But in this case it seems like we're keeping the 
functionality, decided on a better name, but it seems pretty harmless and 
somewhat beneficial to keep the old name around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158137

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

Reply via email to