[PATCH] clang-tidy: Add initial check for "Don't use else after return".
Daniel Jasper
djasper at google.com
Wed Jan 14 10:48:17 PST 2015
It does apply to other statements and we are going to implement those in this check, but I'd like the check to have the same name as the rule in the coding standard. Also, after "return" seems to be by far the most frequent case.
================
Comment at: clang-tidy/misc/ElseAfterReturnCheck.cpp:34
@@ +33,3 @@
+ SourceLocation ElseLoc = If->getElseLoc();
+ if (ElseLoc.isInvalid()) If->dump();
+ DiagnosticBuilder Diag = diag(ElseLoc, "don't use else here");
----------------
alexfh wrote:
> clang-format?
Done.
================
Comment at: clang-tidy/misc/ElseAfterReturnCheck.cpp:35
@@ +34,3 @@
+ if (ElseLoc.isInvalid()) If->dump();
+ DiagnosticBuilder Diag = diag(ElseLoc, "don't use else here");
+ Diag << removeToken(ElseLoc);
----------------
alexfh wrote:
> Maybe explain what "here" means? How about "don't use else after a return"?
Done.
================
Comment at: clang-tidy/misc/ElseAfterReturnCheck.cpp:38
@@ +37,3 @@
+
+ // FIXME: Removing the braces isn't always safe. Do a more careful analysis.
+ if (const CompoundStmt *CS = Result.Nodes.getNodeAs<CompoundStmt>("else"))
----------------
dblaikie wrote:
> Is this the "quite a bit to do" you are referring to? Or are there other holes you know need fixing? (perhaps some could be included in the test case to give an idea of why this isn't ready for prime-time yet)
That's part of the analysis. Happy to do all that, but don't have time at the moment. I think the check is useful as is, though.
================
Comment at: test/clang-tidy/misc-else-after-return.cpp:15
@@ +14,3 @@
+ } else { // comment
+// CHECK-MESSAGES: warning: don't use else here
+// CHECK-FIXES: } // comment
----------------
alexfh wrote:
> Please add a line number to avoid incorrect matches.
Done.
http://reviews.llvm.org/D6927
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the cfe-commits
mailing list