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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
