njames93 updated this revision to Diff 274353.
njames93 added a comment.
Address doc backticks.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82824/new/
https://reviews.llvm.org/D82824
Files:
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability-else-after-return.rst
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-no-cond-var-refactor.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-no-cond-var-refactor.cpp
===================================================================
--- /dev/null
+++ 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.RefactorConditionVariables, 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;
+ }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-else-after-return.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/readability-else-after-return.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-else-after-return.rst
@@ -59,6 +59,19 @@
}
}
+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:: RefactorConditionVariables
+
+ 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.
+ Default value is `true`.
-This check helps to enforce this `LLVM Coding Standards recommendation
-<https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return>`_.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -195,6 +195,11 @@
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Improved :doc:`readability-else-after-return
+ <clang-tidy/checks/readability-else-after-return>` check now supports a
+ `RefactorConditionVariables` option to control whether to refactor condition
+ variables where possible.
+
- Improved :doc:'readability-identifier-naming
<clang-tidy/checks/readability-identifier-naming>` check.
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
@@ -28,6 +28,7 @@
private:
const bool WarnOnUnfixable;
+ const bool RefactorConditionVariables;
};
} // namespace readability
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -26,6 +26,8 @@
static const char ThrowStr[] = "throw";
static const char WarningMessage[] = "do not use 'else' after '%0'";
static const char WarnOnUnfixableStr[] = "WarnOnUnfixable";
+static const char RefactorConditionVariablesStr[] =
+ "RefactorConditionVariables";
const DeclRefExpr *findUsage(const Stmt *Node, int64_t DeclIdentifier) {
if (!Node)
@@ -138,10 +140,14 @@
ElseAfterReturnCheck::ElseAfterReturnCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- WarnOnUnfixable(Options.get(WarnOnUnfixableStr, true)) {}
+ WarnOnUnfixable(Options.get(WarnOnUnfixableStr, true)),
+ RefactorConditionVariables(
+ Options.get(RefactorConditionVariablesStr, true)) {}
void ElseAfterReturnCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, WarnOnUnfixableStr, WarnOnUnfixable);
+ Options.store(Opts, RefactorConditionVariablesStr,
+ RefactorConditionVariables);
}
void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
@@ -186,6 +192,8 @@
}
if (checkConditionVarUsageInElse(If) != nullptr) {
+ if (!RefactorConditionVariables)
+ 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 +227,8 @@
}
if (checkInitDeclUsageInElse(If) != nullptr) {
+ if (!RefactorConditionVariables)
+ 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.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
