[PATCH] D152495: [Clang][SemaCXX] Add unused warning for variables declared in condition expressions

Wed Aug 30 11:38:06 PDT 2023

MaskRay added a comment.

In D152495#4628903 <https://reviews.llvm.org/D152495#4628903>, @smeenai wrote:

> In D152495#4628877 <https://reviews.llvm.org/D152495#4628877>, @hans wrote:
>
>> In D152495#4628870 <https://reviews.llvm.org/D152495#4628870>, @goncharov wrote:
>>
>>> due to this change we have a enourmous number of new warnings, on the other hand -Wunused-variable is a valuable warning. I am not sure what is the policy and best practices for warnings but maybe there is a possiblity to make a transition period for enabling this type of warning separetely and to allow updating existing code?
>>
>> The usual policy is to put new warnings behind new flags so users can disable them selectively. It gets trickier when it's existing warnings getting enhanced like this. Would it be possible to put this new functionality behind a flag?
>
> Yeah, this has been a recurring issue when existing warnings get enhanced. It's often not feasible to fix all new instances right away, but you don't want to disable or `-Wno-error` the warning either cos then new issues can start creeping in, leaving you with no good options.

(I haven't really read the technical side of this patch yet.)

Yes, from me observing how we internally treat such warnings.

> Maybe we should have a policy around even enhancements to existing warnings always going in their own subgroup, so that we can disable them selectively while not regressing anything? @aaron.ballman, what are your thoughts?

Sometimes an improved diagnostic just triggers in very few new places that can be easily handled.
Sometimes there are many more places so that it is difficult to fix in a satisfactory timeline (as a rolling update user doesn't want to postpone releases a long time).

I think sometimes it's just difficult for contributors to figure out whether a diagnostic is going to require substantial cleanup or just a little effort. In this case rolling update users (like we) providing feedback is useful whether a new sub `-Wxxx` is needed.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152495