diff --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h --- a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h +++ b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h @@ -28,6 +28,7 @@ private: const bool WarnOnUnfixable; + const bool WarnOnConditionVariables; }; } // namespace readability diff --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp --- a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp @@ -26,6 +26,7 @@ static const char ThrowStr[] = "throw"; static const char WarningMessage[] = "do not use 'else' after '%0'"; static const char WarnOnUnfixableStr[] = "WarnOnUnfixable"; +static const char WarnOnConditionVariablesStr[] = "WarnOnConditionVariables"; const DeclRefExpr *findUsage(const Stmt *Node, int64_t DeclIdentifier) { if (!Node) @@ -138,10 +139,13 @@ ElseAfterReturnCheck::ElseAfterReturnCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - WarnOnUnfixable(Options.get(WarnOnUnfixableStr, true)) {} + WarnOnUnfixable(Options.get(WarnOnUnfixableStr, true)), + WarnOnConditionVariables(Options.get(WarnOnConditionVariablesStr, true)) { +} void ElseAfterReturnCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, WarnOnUnfixableStr, WarnOnUnfixable); + Options.store(Opts, WarnOnConditionVariablesStr, WarnOnConditionVariables); } void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) { @@ -186,6 +190,8 @@ } if (checkConditionVarUsageInElse(If) != nullptr) { + if (!WarnOnConditionVariables) + return; if (IsLastInScope) { // If the if statement is the last statement its enclosing statements // scope, we can pull the decl out of the if statement. @@ -219,6 +225,8 @@ } if (checkInitDeclUsageInElse(If) != nullptr) { + if (!WarnOnConditionVariables) + return; if (IsLastInScope) { // If the if statement is the last statement its enclosing statements // scope, we can pull the decl out of the if statement. diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -195,6 +195,11 @@ Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Improved :doc:`readability-else-after-return + ` check now supports a + `WarnOnConditionVariables` option to control whether to refactor condition + variables where possible. + - Improved :doc:`readability-identifier-naming ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability-else-after-return.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-else-after-return.rst --- a/clang-tools-extra/docs/clang-tidy/checks/readability-else-after-return.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability-else-after-return.rst @@ -59,6 +59,21 @@ } } +Options +------- + +.. option:: WarnOnUnfixable + + When `true`, Emit a warning for cases where the check can't output a + Fix-It. These can occur with declarations inside the ``else`` branch that + would have an extended lifetime if the ``else`` branch was removed. + Default value is `true`. + +.. option:: WarnOnConditionVariables + + When `true`, The check will attempt to refactor a variable defined inside + the condition of the ``if`` statement that is used in the ``else`` branch + defining them just before the ``if`` statement. This can only be done if + the ``if`` statement is the last statement in its parents scope. + Default value is `true`. -This check helps to enforce this `LLVM Coding Standards recommendation -`_. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-no-cond-var-refactor.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-no-cond-var-refactor.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-no-cond-var-refactor.cpp @@ -0,0 +1,42 @@ +// RUN: %check_clang_tidy %s readability-else-after-return %t -- \ +// RUN: -config='{CheckOptions: [ \ +// RUN: {key: readability-else-after-return.WarnOnConditionVariables, value: false}, \ +// RUN: ]}' + +bool foo(int Y) { + // Excess scopes are here so that the check would have to opportunity to + // refactor the variable out of the condition. + + // Expect warnings here as we don't need to move declaration of 'X' out of the + // if condition as its not used in the else. + { + if (int X = Y) + return X < 0; + else + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return' + return false; + } + { + if (int X = Y; X) + return X < 0; + else + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return' + return false; + } + + // Expect no warnings for these cases, as even though its safe to move + // declaration of 'X' out of the if condition, that has been disabled + // by the options. + { + if (int X = Y) + return false; + else + return X < 0; + } + { + if (int X = Y; X) + return false; + else + return X < 0; + } +}