[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

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.

We don't typically test versions of `-Wno-error=`, we tend to trust that the diagnostics system 'works' in this case.  Unless there is a situation that you're concerned about?

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.

While I'm sympathetic to this, I don't think there is precedent for doing something like that.  I think I'd be OK tacking an extra note onto this (and starting said precedent), but I think we'd need to hear from a code-owner to make a change like that.


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

https://reviews.llvm.org/D131307