njames93 marked an inline comment as done. njames93 added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp:193 if (checkConditionVarUsageInElse(If) != nullptr) { + if (!WarnOnConditionVariables) + return; ---------------- aaron.ballman wrote: > njames93 wrote: > > aaron.ballman wrote: > > > Would it make sense to hoist this into the previous `if` statement so we > > > don't bother checking the condition var use in the first place if we're > > > just going to ignore the results? > > That wouldn't work, we need to see if there is a condition variable that > > needs refactoring first before we can disregard it, Or am I missing > > something? > I was suggesting: > ``` > if (WarnOnConditionVariables && checkConditionVarUsageInElse(If)) { > ... > } > if (WarnOnConditionVariables && checkInitDeclUsageInElse(If) { > ... > } > ``` > The effect is that we don't bother checking for the situations we weren't > going to warn about anyway. But maybe I'm the one missing something? I think you may be this time. The reason being those `if` statements have a `return` at the end. If I followed how you have it layed out, those returns would never hit regardless of whether there was a condition variable that needed handling or not. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82824/new/ https://reviews.llvm.org/D82824 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits