[clang-tools-extra] r265033 - [clang-tidy] Add a new checker to detect missing comma in initializer list.
Etienne Bergeron via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 31 11:12:23 PDT 2016
Author: etienneb
Date: Thu Mar 31 13:12:23 2016
New Revision: 265033
URL: http://llvm.org/viewvc/llvm-project?rev=265033&view=rev
Log:
[clang-tidy] Add a new checker to detect missing comma in initializer list.
Summary:
This checker is able to detect missing comma in
an array of string literals.
```
const char* A[] = {
"abc",
"def" // missing comma (no compiler warnings)
"ghi",
};
```
The ratio of false-positive is reduced by restricting the
size of the array considered and the ratio of missing
comma.
To validate the quantity of false positive, the checker
was tried over LLVM and chromium code and detected these
cases:
[[ http://reviews.llvm.org/D18454 | http://reviews.llvm.org/D18454 ]]
[[https://codereview.chromium.org/1807753002/ | https://codereview.chromium.org/1807753002/]]
[[https://codereview.chromium.org/1826193002/ | https://codereview.chromium.org/1826193002/]]
[[https://codereview.chromium.org/1805713002/ | https://codereview.chromium.org/1805713002/]]
Reviewers: alexfh
Subscribers: LegalizeAdulthood, szdominik, xazax.hun, cfe-commits
Differential Revision: http://reviews.llvm.org/D18457
Added:
clang-tools-extra/trunk/clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/SuspiciousMissingCommaCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-suspicious-missing-comma.rst
clang-tools-extra/trunk/test/clang-tidy/misc-suspicious-missing-comma.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=265033&r1=265032&r2=265033&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Thu Mar 31 13:12:23 2016
@@ -23,6 +23,7 @@ add_clang_library(clangTidyMiscModule
SizeofContainerCheck.cpp
StaticAssertCheck.cpp
StringIntegerAssignmentCheck.cpp
+ SuspiciousMissingCommaCheck.cpp
SuspiciousSemicolonCheck.cpp
SwappedArgumentsCheck.cpp
ThrowByValueCatchByReferenceCheck.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=265033&r1=265032&r2=265033&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Thu Mar 31 13:12:23 2016
@@ -31,6 +31,7 @@
#include "SizeofContainerCheck.h"
#include "StaticAssertCheck.h"
#include "StringIntegerAssignmentCheck.h"
+#include "SuspiciousMissingCommaCheck.h"
#include "SuspiciousSemicolonCheck.h"
#include "SwappedArgumentsCheck.h"
#include "ThrowByValueCatchByReferenceCheck.h"
@@ -88,6 +89,8 @@ public:
"misc-static-assert");
CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
"misc-string-integer-assignment");
+ CheckFactories.registerCheck<SuspiciousMissingCommaCheck>(
+ "misc-suspicious-missing-comma");
CheckFactories.registerCheck<SuspiciousSemicolonCheck>(
"misc-suspicious-semicolon");
CheckFactories.registerCheck<SwappedArgumentsCheck>(
Added: clang-tools-extra/trunk/clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/SuspiciousMissingCommaCheck.cpp?rev=265033&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/SuspiciousMissingCommaCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/SuspiciousMissingCommaCheck.cpp Thu Mar 31 13:12:23 2016
@@ -0,0 +1,80 @@
+//===--- SuspiciousMissingCommaCheck.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 "SuspiciousMissingCommaCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+namespace {
+
+AST_MATCHER(StringLiteral, isConcatenatedLiteral) {
+ return Node.getNumConcatenated() > 1;
+}
+
+} // namespace
+
+SuspiciousMissingCommaCheck::SuspiciousMissingCommaCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ SizeThreshold(Options.get("SizeThreshold", 5U)),
+ RatioThreshold(std::stod(Options.get("RatioThreshold", ".2"))) {}
+
+void SuspiciousMissingCommaCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "SizeThreshold", SizeThreshold);
+ Options.store(Opts, "RatioThreshold", std::to_string(RatioThreshold));
+}
+
+void SuspiciousMissingCommaCheck::registerMatchers(MatchFinder *Finder) {
+ const auto ConcatenatedStringLiteral =
+ stringLiteral(isConcatenatedLiteral()).bind("str");
+
+ const auto StringsInitializerList =
+ initListExpr(hasType(constantArrayType()),
+ has(expr(ignoringImpCasts(ConcatenatedStringLiteral))));
+
+ Finder->addMatcher(StringsInitializerList.bind("list"), this);
+}
+
+void SuspiciousMissingCommaCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *InitializerList = Result.Nodes.getNodeAs<InitListExpr>("list");
+ const auto *ConcatenatedLiteral = Result.Nodes.getNodeAs<Expr>("str");
+ assert(InitializerList && ConcatenatedLiteral);
+
+ // Skip small arrays as they often generate false-positive.
+ unsigned int Size = InitializerList->getNumInits();
+ if (Size < SizeThreshold) return;
+
+ // Count the number of occurence of concatenated string literal.
+ unsigned int Count = 0;
+ for (unsigned int i = 0; i < Size; ++i) {
+ const Expr *Child = InitializerList->getInit(i)->IgnoreImpCasts();
+ if (const auto *Literal = dyn_cast<StringLiteral>(Child)) {
+ if (Literal->getNumConcatenated() > 1) ++Count;
+ }
+ }
+
+ // Warn only when concatenation is not common in this initializer list.
+ // The current threshold is set to less than 1/5 of the string literals.
+ if (double(Count) / Size > RatioThreshold) return;
+
+ diag(ConcatenatedLiteral->getLocStart(),
+ "suspicious string literal, probably missing a comma");
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/misc/SuspiciousMissingCommaCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/SuspiciousMissingCommaCheck.h?rev=265033&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/SuspiciousMissingCommaCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/SuspiciousMissingCommaCheck.h Thu Mar 31 13:12:23 2016
@@ -0,0 +1,41 @@
+//===--- SuspiciousMissingCommaCheck.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_MISSING_COMMA_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SUSPICIOUS_MISSING_COMMA_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This check finds string literals which are probably concatenated accidentally.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-suspicious-missing-comma.html
+class SuspiciousMissingCommaCheck : public ClangTidyCheck {
+public:
+ SuspiciousMissingCommaCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ // Minimal size of a string literals array to be considered by the checker.
+ const unsigned SizeThreshold;
+ // Maximal threshold ratio of suspicious string literals to be considered.
+ const double RatioThreshold;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SUSPICIOUS_MISSING_COMMA_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=265033&r1=265032&r2=265033&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Thu Mar 31 13:12:23 2016
@@ -66,6 +66,7 @@ Clang-Tidy Checks
misc-sizeof-container
misc-static-assert
misc-string-integer-assignment
+ misc-suspicious-missing-comma
misc-suspicious-semicolon
misc-swapped-arguments
misc-throw-by-value-catch-by-reference
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-suspicious-missing-comma.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-suspicious-missing-comma.rst?rev=265033&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-suspicious-missing-comma.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-suspicious-missing-comma.rst Thu Mar 31 13:12:23 2016
@@ -0,0 +1,35 @@
+.. title:: clang-tidy - misc-suspicious-missing-comma
+
+misc-suspicious-missing-comma
+=============================
+
+String literals placed side-by-side are concatenated at translation phase 6
+(after the preprocessor). This feature is used to represent long string
+literal on multiple lines.
+
+For instance, these declarations are equivalent:
+ const char* A[] = "This is a test";
+ const char* B[] = "This" " is a "
+ "test";
+
+A common mistake done by programmers is to forget a comma between two string
+literals in an array initializer list.
+
+ const char* Test[] = {
+ "line 1",
+ "line 2" // Missing comma!
+ "line 3",
+ "line 4",
+ "line 5"
+ };
+
+The array contains the string "line 2line3" at offset 1 (i.e. Test[1]). Clang
+won't generate warnings at compile time.
+
+This checker may warn incorrectly on cases like:
+
+ const char* SupportedFormat[] = {
+ "Error %s",
+ "Code " PRIu64, // May warn here.
+ "Warning %s",
+ };
Added: clang-tools-extra/trunk/test/clang-tidy/misc-suspicious-missing-comma.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-suspicious-missing-comma.cpp?rev=265033&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-suspicious-missing-comma.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-suspicious-missing-comma.cpp Thu Mar 31 13:12:23 2016
@@ -0,0 +1,34 @@
+// RUN: %check_clang_tidy %s misc-suspicious-missing-comma %t
+
+const char* Cartoons[] = {
+ "Bugs Bunny",
+ "Homer Simpson",
+ "Mickey Mouse",
+ "Bart Simpson",
+ "Charlie Brown" // There is a missing comma here.
+ "Fred Flintstone",
+ "Popeye",
+};
+// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: suspicious string literal, probably missing a comma [misc-suspicious-missing-comma]
+
+const wchar_t* Colors[] = {
+ L"Red", L"Yellow", L"Blue", L"Green", L"Purple", L"Rose", L"White", L"Black"
+};
+
+// The following array should not trigger any warnings.
+const char* HttpCommands[] = {
+ "GET / HTTP/1.0\r\n"
+ "\r\n",
+
+ "GET /index.html HTTP/1.0\r\n"
+ "\r\n",
+
+ "GET /favicon.ico HTTP/1.0\r\n"
+ "header: dummy"
+ "\r\n",
+};
+
+// This array is too small to trigger a warning.
+const char* SmallArray[] = {
+ "a" "b", "c"
+};
More information about the cfe-commits
mailing list