[PATCH] D82824: [clang-tidy] Added option to readability-else-after-return

[Nathan James via Phabricator via cfe-commits]

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