aaron.ballman added a comment. In D158137#4599546 <https://reviews.llvm.org/D158137#4599546>, @dblaikie wrote:
> 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. IMO, the less friction we make for users doing an upgrade, the better. I tend to agree with @dblaikie that it might be reasonable to leave the old flag around as an alias for the new flag (we can document a preference for the new name if it makes us feel better). 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
