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

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet added a comment.

In D152495#4481588 <https://reviews.llvm.org/D152495#4481588>, @aaron.ballman wrote:

> This is a bit of an odd situation -- the condition variable *is* used (e.g., its value is loaded to determine whether to go into the if branch or the else branch), so we should not be marking it as unused in `Sema::CheckConditionVariable()`. Instead, I think we need to decide what constitutes "unused" in this case. Consider:
>
>   struct RAII {
>     int &x;
>   
>     RAII(int &ref) : x(ref) {}
>     ~RAII() { x = 0; }
>   
>     operator bool() const { return true; }
>   };
>   
>   void func() {
>     int i = 12;
>     if (RAII name{i}) {
>     }
>   }
>
> I don't think `name` is unused in the same way that `i` is unused in `if (int i = 1) {}`. So I think we only want this to apply to scalars and not class types. I think the changes should be happening in `Sema::ShouldDiagnoseUnusedDecl()`  (and you might need to thread the `Scope` object through to that call -- my naive thinking is that a variable declaration in a condition scope should be diagnosed as unused if it is used but not referenced, but I could be off-base).

Thank you for the insightful feedback!
I agree that we should not be emitting Wunused warnings in condition expressions when the declaration is of class type.

About the `Scope` object, if you mean `ConditionVarScope` flag by "condition scope", it seems to be used only for the condition of for-statements. (https://github.com/llvm/llvm-project/blob/f36b0169b8821e431644cb240f081538eb2b55ef/clang/lib/Parse/ParseExprCXX.cpp#L2045-L2057) Also it does not push new scope but modifies the flag of the current scope.
So it does not seem to be usable for our purpose here.

Instead, I think used-but-not-referenced variable declarations can be diagnosed as unused regardless of whether they are declared in condition expressions. This saves us from deleting the assertion from CodeGen and modifying AST tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152495