zinovy.nis added inline comments.
================ Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp:1092 + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'isSet' [bugprone-redundant-branch-condition] + // CHECK-FIXES: {{isSet}} + } ---------------- aaron.ballman wrote: > zinovy.nis wrote: > > aaron.ballman wrote: > > > There's not a whole lot of context for FileCheck to determine if it's > > > been correctly applied or not (same below) -- for instance, won't this > > > pass even if no changes are applied because FileCheck is still going to > > > find `isSet` in the file? > > Thanks. Fixed. > Maybe it's just early in the morning for me, but... I was expecting the > transformation to be: > ``` > if (RetT::Test(isSet).Ok() && isSet) { > if (RetT::Test(isSet).Ok() && isSet) { > } > } > ``` > turns into > ``` > if (RetT::Test(isSet).Ok() && isSet) { > } > ``` > Why does it remove the `&& isSet` instead? That seems like it's changing the > logic here from `if (true && false)` to `if (true)`. IMO it's correct. `isSet` cannot change its value between `if`s while `RetT::Test(isSet).Ok()` can. So we don't need to re-evaluate `isSet` and need to re-evaluate `RetT::Test(isSet).Ok()` only. > That seems like it's changing the logic here from if (true && false) to if > (true). As I understand only the second `if` is transformed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91037/new/ https://reviews.llvm.org/D91037 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits