[clang-tools-extra] r260503 - [clang-tidy] Add a check to find unintended semicolons that changes the semantics.
Gabor Horvath via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 11 01:23:34 PST 2016
Author: xazax
Date: Thu Feb 11 03:23:33 2016
New Revision: 260503
URL: http://llvm.org/viewvc/llvm-project?rev=260503&view=rev
Log:
[clang-tidy] Add a check to find unintended semicolons that changes the semantics.
Reviewers: hokein, alexfh
Differential Revision: http://reviews.llvm.org/D16535
Added:
clang-tools-extra/trunk/clang-tidy/misc/SuspiciousSemicolonCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/SuspiciousSemicolonCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-suspicious-semicolon.rst
clang-tools-extra/trunk/test/clang-tidy/misc-suspicious-semicolon.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=260503&r1=260502&r2=260503&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Thu Feb 11 03:23:33 2016
@@ -21,6 +21,7 @@ add_clang_library(clangTidyMiscModule
SizeofContainerCheck.cpp
StaticAssertCheck.cpp
StringIntegerAssignmentCheck.cpp
+ SuspiciousSemicolonCheck.cpp
SwappedArgumentsCheck.cpp
ThrowByValueCatchByReferenceCheck.cpp
UndelegatedConstructor.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=260503&r1=260502&r2=260503&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Thu Feb 11 03:23:33 2016
@@ -29,6 +29,7 @@
#include "SizeofContainerCheck.h"
#include "StaticAssertCheck.h"
#include "StringIntegerAssignmentCheck.h"
+#include "SuspiciousSemicolonCheck.h"
#include "SwappedArgumentsCheck.h"
#include "ThrowByValueCatchByReferenceCheck.h"
#include "UndelegatedConstructor.h"
@@ -81,6 +82,8 @@ public:
"misc-static-assert");
CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
"misc-string-integer-assignment");
+ CheckFactories.registerCheck<SuspiciousSemicolonCheck>(
+ "misc-suspicious-semicolon");
CheckFactories.registerCheck<SwappedArgumentsCheck>(
"misc-swapped-arguments");
CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>(
Added: clang-tools-extra/trunk/clang-tidy/misc/SuspiciousSemicolonCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/SuspiciousSemicolonCheck.cpp?rev=260503&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/SuspiciousSemicolonCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/SuspiciousSemicolonCheck.cpp Thu Feb 11 03:23:33 2016
@@ -0,0 +1,74 @@
+//===--- SuspiciousSemicolonCheck.cpp - clang-tidy-------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "../utils/LexerUtils.h"
+#include "SuspiciousSemicolonCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void SuspiciousSemicolonCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ stmt(anyOf(ifStmt(hasThen(nullStmt().bind("semi")),
+ unless(hasElse(stmt()))),
+ forStmt(hasBody(nullStmt().bind("semi"))),
+ cxxForRangeStmt(hasBody(nullStmt().bind("semi"))),
+ whileStmt(hasBody(nullStmt().bind("semi")))))
+ .bind("stmt"),
+ this);
+}
+
+void SuspiciousSemicolonCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Semicolon = Result.Nodes.getNodeAs<NullStmt>("semi");
+ SourceLocation LocStart = Semicolon->getLocStart();
+
+ if (LocStart.isMacroID())
+ return;
+
+ ASTContext &Ctxt = *Result.Context;
+ auto Token = lexer_utils::getPreviousNonCommentToken(Ctxt, LocStart);
+ auto &SM = *Result.SourceManager;
+ unsigned SemicolonLine = SM.getSpellingLineNumber(LocStart);
+
+ const auto *Statement = Result.Nodes.getNodeAs<Stmt>("stmt");
+ const bool IsIfStmt = isa<IfStmt>(Statement);
+
+ if (!IsIfStmt &&
+ SM.getSpellingLineNumber(Token.getLocation()) != SemicolonLine)
+ return;
+
+ SourceLocation LocEnd = Semicolon->getLocEnd();
+ FileID FID = SM.getFileID(LocEnd);
+ llvm::MemoryBuffer *Buffer = SM.getBuffer(FID, LocEnd);
+ Lexer Lexer(SM.getLocForStartOfFile(FID), Ctxt.getLangOpts(),
+ Buffer->getBufferStart(), SM.getCharacterData(LocEnd) + 1,
+ Buffer->getBufferEnd());
+ if (Lexer.LexFromRawLexer(Token))
+ return;
+
+ unsigned BaseIndent = SM.getSpellingColumnNumber(Statement->getLocStart());
+ unsigned NewTokenIndent = SM.getSpellingColumnNumber(Token.getLocation());
+ unsigned NewTokenLine = SM.getSpellingLineNumber(Token.getLocation());
+
+ if (!IsIfStmt && NewTokenIndent <= BaseIndent &&
+ Token.getKind() != tok::l_brace && NewTokenLine != SemicolonLine)
+ return;
+
+ diag(LocStart, "potentially unintended semicolon")
+ << FixItHint::CreateRemoval(SourceRange(LocStart, LocEnd));
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/misc/SuspiciousSemicolonCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/SuspiciousSemicolonCheck.h?rev=260503&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/SuspiciousSemicolonCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/SuspiciousSemicolonCheck.h Thu Feb 11 03:23:33 2016
@@ -0,0 +1,36 @@
+//===--- SuspiciousSemicolonCheck.h - clang-tidy-----------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SUSPICIOUS_SEMICOLON_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SUSPICIOUS_SEMICOLON_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This check finds semicolon that modifies the meaning of the program
+/// unintendedly.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-suspicious-semicolon.html
+class SuspiciousSemicolonCheck : public ClangTidyCheck {
+public:
+ SuspiciousSemicolonCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SUSPICIOUS_SEMICOLON_H
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=260503&r1=260502&r2=260503&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Thu Feb 11 03:23:33 2016
@@ -61,6 +61,7 @@ Clang-Tidy Checks
misc-sizeof-container
misc-static-assert
misc-string-integer-assignment
+ misc-suspicious-semicolon
misc-swapped-arguments
misc-throw-by-value-catch-by-reference
misc-undelegated-constructor
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-suspicious-semicolon.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-suspicious-semicolon.rst?rev=260503&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-suspicious-semicolon.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-suspicious-semicolon.rst Thu Feb 11 03:23:33 2016
@@ -0,0 +1,72 @@
+.. title:: clang-tidy - misc-suspicious-semicolon
+
+misc-suspicious-semicolon
+=========================
+
+Finds most instances of stray semicolons that unexpectedly alter the meaning of
+the code. More specifically, it looks for `if`, `while`, `for` and `for-range`
+statements whose body is a single semicolon, and then analyzes the context of
+the code (e.g. indentation) in an attempt to determine whether that is
+intentional.
+
+ .. code-block:: c++
+
+ if(x < y);
+ {
+ x++;
+ }
+
+Here the body of the `if` statement consists of only the semicolon at the end of
+the first line, and `x` will be incremented regardless of the condition.
+
+
+ .. code-block:: c++
+
+ while((line = readLine(file)) != NULL);
+ processLine(line);
+
+As a result of this code, `processLine()` will only be called once, when the
+`while` loop with the empty body exits with `line == NULL`. The indentation of
+the code indicates the intention of the programmer.
+
+
+ .. code-block:: c++
+
+ if(x >= y);
+ x -= y;
+
+While the indentation does not imply any nesting, there is simply no valid
+reason to have an `if` statement with an empty body (but it can make sense for
+a loop). So this check issues a warning for the code above.
+
+To solve the issue remove the stray semicolon or in case the empty body is
+intentional, reflect this using code indentation or put the semicolon in a new
+line. For example:
+
+ .. code-block:: c++
+
+ while(readWhitespace());
+ Token t = readNextToken();
+
+Here the second line is indented in a way that suggests that it is meant to be
+the body of the `while` loop - whose body is in fact empty, becasue of the
+semicolon at the end of the first line.
+
+Either remove the indentation from the second line:
+
+ .. code-block:: c++
+
+ while(readWhitespace());
+ Token t = readNextToken();
+
+... or move the semicolon from the end of the first line to a new line:
+
+ .. code-block:: c++
+
+ while(readWhitespace())
+ ;
+
+ Token t = readNextToken();
+
+In this case the check will assume that you know what you are doing, and will
+not raise a warning.
Added: clang-tools-extra/trunk/test/clang-tidy/misc-suspicious-semicolon.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-suspicious-semicolon.cpp?rev=260503&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-suspicious-semicolon.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-suspicious-semicolon.cpp Thu Feb 11 03:23:33 2016
@@ -0,0 +1,117 @@
+// RUN: %check_clang_tidy %s misc-suspicious-semicolon %t
+
+int x = 5;
+
+void nop();
+
+void correct1()
+{
+ if(x < 5) nop();
+}
+
+void correct2()
+{
+ if(x == 5)
+ nop();
+}
+
+void correct3()
+{
+ if(x > 5)
+ {
+ nop();
+ }
+}
+
+void fail1()
+{
+ if(x > 5); nop();
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: potentially unintended semicolon [misc-suspicious-semicolon]
+ // CHECK-FIXES: if(x > 5) nop();
+}
+
+void fail2()
+{
+ if(x == 5);
+ nop();
+ // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: potentially unintended semicolon [misc-suspicious-semicolon]
+ // CHECK-FIXES: if(x == 5){{$}}
+}
+
+void fail3()
+{
+ if(x < 5);
+ {
+ nop();
+ }
+ // CHECK-MESSAGES: :[[@LINE-4]]:11: warning: potentially unintended semicolon
+ // CHECK-FIXES: if(x < 5){{$}}
+}
+
+void correct4()
+{
+ while(x % 5 == 1);
+ nop();
+}
+
+void correct5()
+{
+ for(int i = 0; i < x; ++i)
+ ;
+}
+
+void fail4()
+{
+ for(int i = 0; i < x; ++i);
+ nop();
+ // CHECK-MESSAGES: :[[@LINE-2]]:28: warning: potentially unintended semicolon
+ // CHECK-FIXES: for(int i = 0; i < x; ++i){{$}}
+}
+
+void fail5()
+{
+ if(x % 5 == 1);
+ nop();
+ // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: potentially unintended semicolon
+ // CHECK-FIXES: if(x % 5 == 1){{$}}
+}
+
+void fail6() {
+ int a = 0;
+ if (a != 0) {
+ } else if (a != 1);
+ a = 2;
+ // CHECK-MESSAGES: :[[@LINE-2]]:21: warning: potentially unintended semicolon
+ // CHECK-FIXES: } else if (a != 1){{$}}
+}
+
+void fail7() {
+ if (true)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: potentially unintended semicolon
+}
+
+void correct6()
+{
+ do; while(false);
+}
+
+int correct7()
+{
+ int t_num = 0;
+ char c = 'b';
+ char *s = "a";
+ if (s == "(" || s != "'" || c == '"') {
+ t_num += 3;
+ return (c == ')' && c == '\'');
+ }
+
+ return 0;
+}
+
+void correct8() {
+ if (true)
+ ;
+ else {
+ }
+}
More information about the cfe-commits
mailing list