aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

In D131307#3705362 <https://reviews.llvm.org/D131307#3705362>, @smeenai wrote:

> In D131307#3704709 <https://reviews.llvm.org/D131307#3704709>, @thakis wrote:
>
>> It's already an error, but it's a warning default-mapped to an error. You 
>> can -Wno-error=name to downgrade it into a warning, but that requires an 
>> explicit action. So people are unlikely to miss it.
>>
>> This is how we usually handle these breaking changes.
>>
>> Maybe there could be a test for the -Wno-error= case? But this looks roughly 
>> right to me overall. I haven't looked in detail.
>
> Right, but we also want people to understand that the downgrade possibility 
> will be going away in the next release (i.e. it'll always be an error and 
> there's nothing you can do about that), so they really do want to deal with 
> this as a priority.

FWIW, we had a similar situation (but not quite the same) during Clang 15 where 
we had some diagnostics that were strengthened to be an error but that 
allowance only exists in some language modes:

  - The ``-Wimplicit-function-declaration`` warning diagnostic now defaults to
    an error in C99 and later. Prior to C2x, it may be downgraded to a warning
    with ``-Wno-error=implicit-function-declaration``, or disabled entirely with
    ``-Wno-implicit-function-declaration``. As of C2x, support for implicit
    function declarations has been removed, and the warning options will have no
    effect.
  - The ``-Wimplicit-int`` warning diagnostic now defaults to an error in C99 
and
    later. Prior to C2x, it may be downgraded to a warning with
    ``-Wno-error=implicit-int``, or disabled entirely with 
``-Wno-implicit-int``.
    As of C2x, support for implicit int has been removed, and the warning 
options
    will have no effect. Specifying ``-Wimplicit-int`` in C89 mode will now 
issue
    warnings instead of being a noop.

I think the release note we have in this patch is reasonable.

This LGTM!


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

https://reviews.llvm.org/D131307

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

Reply via email to