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

hazohelet added a comment. Thank you for the feedback and the revert! Distinguishing condition variables from other unused variables in Wunused warnings was something I was trying initially, and I agree that it's the best way forward. The difficulty is in how to know whether an unused

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

aaron.ballman added a comment. In D152495#4631829 , @nickdesaulniers wrote: > Here's a more blatant regression caused by this patch. > > https://godbolt.org/z/q19Ge64G3 > > Causing breakage for the Linux kernel: >

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

nickdesaulniers added a comment. Here's a more blatant regression caused by this patch. https://godbolt.org/z/q19Ge64G3 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152495/new/ https://reviews.llvm.org/D152495

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

pranavk added a comment. I agree -Wunused-condition-variable sounds like a good idea. There are still numerous instances of this warning/error showing up when doing a self-build of LLVM, let alone warnings/errors that are showing up in internal code bases who are using LLVM HEAD for compiling

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

MaskRay added a comment. In D152495#4628903 , @smeenai wrote: > In D152495#4628877 , @hans wrote: > >> In D152495#4628870 , @goncharov >> wrote: >> >>> due to this

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

hans added a comment. In D152495#4628887 , @aaron.ballman wrote: > That might not be a bad idea in this case -- perhaps > `-Wunused-condition-variable` and group it under `-Wunused-variable`? Sounds god to me conceptually. (I don't know the code, so

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

smeenai added a comment. In D152495#4628877 , @hans wrote: > In D152495#4628870 , @goncharov > wrote: > >> due to this change we have a enourmous number of new warnings, on the other >> hand -Wunused-variable

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

aaron.ballman added a comment. In D152495#4628877 , @hans wrote: > In D152495#4628785 , @cor3ntin > wrote: > >> It is used, but only in an assert. Seems like the right fix! > > I suppose it is technically, but

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

hans added a comment. In D152495#4628785 , @cor3ntin wrote: > It is used, but only in an assert. Seems like the right fix! I suppose it is technically, but I'm not sure the fix reads like an improvement to me... I guess that's often the case with

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

goncharov added a comment. 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

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

cor3ntin added a comment. It is used, but only in an assert. Seems like the right fix! In D152495#4628778 , @smeenai wrote: > In D152495#4628028 , @goncharov > wrote: > >> there is a number of unused vaiables

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

smeenai added a comment. In D152495#4628028 , @goncharov wrote: > there is a number of unused vaiables in conditional loops that are firing > now, I have submitted > https://github.com/llvm/llvm-project/commit/74f4daef0412be33002bd4e24a30cb47d0187ecf

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

goncharov added a comment. there is a number of unused vaiables in conditional loops there are firing now, I have submitted https://github.com/llvm/llvm-project/commit/74f4daef0412be33002bd4e24a30cb47d0187ecf but I suspect it's just a start. How did you checked the project code for that?

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG92023b150990: Reland [Clang][SemaCXX] Add unused warning for variables declared in condition… (authored by hazohelet). Changed prior to commit: https://reviews.llvm.org/D152495?vs=550443=554539#toc

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

hazohelet updated this revision to Diff 550443. hazohelet added a comment. Removed warning fixes that are now in D158016 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152495/new/ https://reviews.llvm.org/D152495 Files: clang/docs/ReleaseNotes.rst

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

nikic added a comment. Please split the warning fixes off into a separate patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152495/new/ https://reviews.llvm.org/D152495 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

hazohelet updated this revision to Diff 550433. hazohelet added a comment. Herald added subscribers: llvm-commits, wangpc, hoy, wlei, steakhal, abrachet, ormris, martong, MaskRay, hiraditya. Herald added a reviewer: NoQ. Herald added projects: LLVM, lld-macho. Herald added a reviewer: lld-macho.

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

hazohelet added a comment. In D152495#4587804 , @chapuni wrote: > Would this cause many warnings in Clang/LLVM tree? > https://lab.llvm.org/buildbot/#/builders/36/builds/36560 > > I hope you to fix possible warnings at first. Thanks. I'll revert this

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

chapuni added a comment. Would this cause many warnings in Clang/LLVM tree? https://lab.llvm.org/buildbot/#/builders/36/builds/36560 I hope you to fix possible warnings at first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152495/new/

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

hazohelet added a comment. In D152495#4566634 , @aaron.ballman wrote: > In D152495#4563520 , @hazohelet > wrote: > >> I reverted this change because it broke sanitizer buildbots. >>

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

aaron.ballman added a comment. In D152495#4563520 , @hazohelet wrote: > I reverted this change because it broke sanitizer buildbots. > https://lab.llvm.org/buildbot/#/builders/19/builds/18369 > The cause of the errors here are relatively clear and I can

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

tbaeder added a comment. The fuzzer timeouts fail all the time; I usually just assume it's unrelated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152495/new/ https://reviews.llvm.org/D152495 ___

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

hazohelet added a comment. I reverted this change because it broke sanitizer buildbots. https://lab.llvm.org/buildbot/#/builders/19/builds/18369 The cause of the errors here are relatively clear and I can speculatively replace `if (std::error_code EC = makeCanonical(Path))` with `if

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbd0ed0abc31f: [Clang][SemaCXX] Add unused warning for variables declared in condition… (authored by hazohelet). Changed prior to commit:

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

cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. LGTM, thanks. Make sure you rebase the changes to the release notes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152495/new/ https://reviews.llvm.org/D152495

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

tbaeder added a comment. @cor3ntin It's next week :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152495/new/ https://reviews.llvm.org/D152495 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

hazohelet added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:4016-4019 + // Ensure that `-Wunused-variable` will be emitted for condition variables + // that are not referenced later. e.g.: if (int var = init()); + if (!T->isAggregateType()) +

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

cor3ntin added a comment. This looks reasonable to me now. I'll wait next week before approving it so other get a chance to look at it :) Comment at: clang/lib/Sema/SemaExprCXX.cpp:4016-4019 + // Ensure that `-Wunused-variable` will be emitted for condition variables + //

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

hazohelet added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:4016-4019 + // Ensure that `-Wunused-variable` will be emitted for condition variables + // that are not referenced later. e.g.: if (int var = init()); + if (!T->isAggregateType()) +

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

hazohelet updated this revision to Diff 540479. hazohelet marked 6 inline comments as done. hazohelet added a comment. Address comments from Aaron and Corentin - Call `ConditionVar->setReferenced(false)` without any checks - Added some more tests CHANGES SINCE LAST ACTION

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

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:4016-4019 + // Ensure that `-Wunused-variable` will be emitted for condition variables + // that are not referenced later. e.g.: if (int var = init()); + if (!T->isAggregateType()) +

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

hazohelet marked an inline comment as done. hazohelet added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:4016-4019 + // Ensure that `-Wunused-variable` will be emitted for condition variables + // that are not referenced later. e.g.: if (int var = init()); + if

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

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:4016-4019 + // Ensure that `-Wunused-variable` will be emitted for condition variables + // that are not referenced later. e.g.: if (int var = init()); + if (!T->isAggregateType()) +

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

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:4016-4019 + // Ensure that `-Wunused-variable` will be emitted for condition variables + // that are not referenced later. e.g.: if (int var = init()); + if (!T->isAggregateType()) +

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

hazohelet updated this revision to Diff 538591. hazohelet marked 5 inline comments as done. hazohelet added a comment. - Stop marking condition variable declarations as unused and diagnose all `VarDecl` that is marked used but not referenced - Added test not to warn when the declaration is not

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

hazohelet added a comment. In 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

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

aaron.ballman added a comment. 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

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

shafik added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:4003-4004 + // that are not referenced or used later. e.g.: if (int var = init()); + ConditionVar->setReferenced(false); + ConditionVar->setIsUsed(false); + tbaeder wrote: > shafik

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

tbaeder added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:4003-4004 + // that are not referenced or used later. e.g.: if (int var = init()); + ConditionVar->setReferenced(false); + ConditionVar->setIsUsed(false); + shafik wrote: > That seem a

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

shafik added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:4003-4004 + // that are not referenced or used later. e.g.: if (int var = init()); + ConditionVar->setReferenced(false); + ConditionVar->setIsUsed(false); + Repository: rG LLVM

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

hazohelet added a comment. Ping 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

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

tbaeder added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:2852 - "Should not use decl without marking it used!"); - if (ND->hasAttr()) { Question for other reviewers: If we want to keep this assertion, would it instead make sense to

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

hazohelet created this revision. hazohelet added reviewers: aaron.ballman, tbaeder, shafik. Herald added a project: All. hazohelet requested review of this revision. Herald added a project: clang. This patch marks the declarations with initializations in condition expressions such as `if (int