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 (!T->isAggregateType()) + ConditionVar->setReferenced(/*R=*/false); ---------------- cor3ntin wrote: > aaron.ballman wrote: > > `isAggregateType()` includes arrays and I think we still want to diagnose > > an unused array. > Should it not be `Condition->hasSideEffects()` ? > Hopefully we want to be consistent with existing behavior > https://godbolt.org/z/6abbPhn4G The condition here only applies to condition variables. C++ does not allow array type in condition variable, correct? I think it's OK to use `isAggregateType` as well then. ================ Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:5348 NewVar->getDeclContext()->isFunctionOrMethod() && - OldVar->getType()->isDependentType()) + OldVar->getType()->isDependentType() && !OldVar->isImplicit()) DiagnoseUnusedDecl(NewVar); ---------------- aaron.ballman wrote: > Which test case covers this change? I wanted to suppress warnings against implicitly declared variables in range statements like `__range2`. I'll add a test for this. 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
