[clang-tools-extra] 25f5406 - [clang-tidy] Extend bugprone-string-constructor-check to std::string_view.
Chris Kennelly via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 18 18:15:55 PST 2020
Author: Chris Kennelly
Date: 2020-11-18T21:16:03-05:00
New Revision: 25f5406f087579d43ca9a82dee7f3e76f0691bad
URL: https://github.com/llvm/llvm-project/commit/25f5406f087579d43ca9a82dee7f3e76f0691bad
DIFF: https://github.com/llvm/llvm-project/commit/25f5406f087579d43ca9a82dee7f3e76f0691bad.diff
LOG: [clang-tidy] Extend bugprone-string-constructor-check to std::string_view.
This allows for matching the constructors std::string has in common with
std::string_view.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D91015
Added:
Modified:
clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-string-constructor.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
index 96d93a1d0413..6b23f7cd4a9c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "StringConstructorCheck.h"
+#include "../utils/OptionsUtils.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Tooling/FixIt.h"
@@ -21,17 +22,36 @@ namespace {
AST_MATCHER_P(IntegerLiteral, isBiggerThan, unsigned, N) {
return Node.getValue().getZExtValue() > N;
}
+
+const char DefaultStringNames[] =
+ "::std::basic_string;::std::basic_string_view";
+
+static std::vector<StringRef>
+removeNamespaces(const std::vector<std::string> &Names) {
+ std::vector<StringRef> Result;
+ Result.reserve(Names.size());
+ for (StringRef Name : Names) {
+ std::string::size_type ColonPos = Name.rfind(':');
+ Result.push_back(
+ Name.substr(ColonPos == std::string::npos ? 0 : ColonPos + 1));
+ }
+ return Result;
+}
+
} // namespace
StringConstructorCheck::StringConstructorCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
WarnOnLargeLength(Options.get("WarnOnLargeLength", true)),
- LargeLengthThreshold(Options.get("LargeLengthThreshold", 0x800000)) {}
+ LargeLengthThreshold(Options.get("LargeLengthThreshold", 0x800000)),
+ StringNames(utils::options::parseStringList(
+ Options.get("StringNames", DefaultStringNames))) {}
void StringConstructorCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "WarnOnLargeLength", WarnOnLargeLength);
Options.store(Opts, "LargeLengthThreshold", LargeLengthThreshold);
+ Options.store(Opts, "StringNames", DefaultStringNames);
}
void StringConstructorCheck::registerMatchers(MatchFinder *Finder) {
@@ -80,7 +100,8 @@ void StringConstructorCheck::registerMatchers(MatchFinder *Finder) {
// parameters. [i.e. string (const char* s, size_t n);]
Finder->addMatcher(
cxxConstructExpr(
- hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
+ hasDeclaration(cxxConstructorDecl(ofClass(
+ cxxRecordDecl(hasAnyName(removeNamespaces(StringNames)))))),
hasArgument(0, hasType(CharPtrType)),
hasArgument(1, hasType(isInteger())),
anyOf(
@@ -100,11 +121,17 @@ void StringConstructorCheck::registerMatchers(MatchFinder *Finder) {
// Check the literal string constructor with char pointer.
// [i.e. string (const char* s);]
Finder->addMatcher(
- traverse(TK_AsIs,
- cxxConstructExpr(hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
- hasArgument(0, expr().bind("from-ptr")),
- hasArgument(1, unless(hasType(isInteger()))))
- .bind("constructor")),
+ traverse(TK_AsIs,
+ cxxConstructExpr(
+ hasDeclaration(cxxConstructorDecl(ofClass(cxxRecordDecl(
+ hasAnyName(removeNamespaces(StringNames)))))),
+ hasArgument(0, expr().bind("from-ptr")),
+ // do not match std::string(ptr, int)
+ // match std::string(ptr, alloc)
+ // match std::string(ptr)
+ anyOf(hasArgument(1, unless(hasType(isInteger()))),
+ argumentCountIs(1)))
+ .bind("constructor")),
this);
}
diff --git a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
index 687f3b106fb4..50338f8abead 100644
--- a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
@@ -32,6 +32,7 @@ class StringConstructorCheck : public ClangTidyCheck {
private:
const bool WarnOnLargeLength;
const unsigned int LargeLengthThreshold;
+ std::vector<std::string> StringNames;
};
} // namespace bugprone
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst
index 5deedf1f6656..c0b6e5004e8c 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst
@@ -21,6 +21,7 @@ Examples:
.. code-block:: c++
std::string("test", 200); // Will include random characters after "test".
+ std::string_view("test", 200);
Creating an empty string from constructors with parameters is considered
suspicious. The programmer should use the empty constructor instead.
@@ -30,6 +31,7 @@ Examples:
.. code-block:: c++
std::string("test", 0); // Creation of an empty string.
+ std::string_view("test", 0);
Options
-------
@@ -42,3 +44,12 @@ Options
.. option:: LargeLengthThreshold
An integer specifying the large length threshold. Default is `0x800000`.
+
+.. option:: StringNames
+
+ Default is `::std::basic_string;::std::basic_string_view`.
+
+ Semicolon-delimited list of class names to apply this check to.
+ By default `::std::basic_string` applies to ``std::string`` and
+ ``std::wstring``. Set to e.g. `::std::basic_string;llvm::StringRef;QString`
+ to perform this check on custom classes.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-constructor.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-constructor.cpp
index d3d927c27dc4..1ca4d14f85e2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-constructor.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-constructor.cpp
@@ -14,6 +14,15 @@ struct basic_string {
};
typedef basic_string<char> string;
typedef basic_string<wchar_t> wstring;
+
+template <typename C, typename T = std::char_traits<C>>
+struct basic_string_view {
+ basic_string_view();
+ basic_string_view(const C *, unsigned int size);
+ basic_string_view(const C *);
+};
+typedef basic_string_view<char> string_view;
+typedef basic_string_view<wchar_t> wstring_view;
}
const char* kText = "";
@@ -52,11 +61,35 @@ void Test() {
// CHECK-MESSAGES: [[@LINE-1]]:20: warning: constructing string from nullptr is undefined behaviour
}
+void TestView() {
+ std::string_view q0("test", 0);
+ // CHECK-MESSAGES: [[@LINE-1]]:20: warning: constructor creating an empty string
+ std::string_view q1(kText, -4);
+ // CHECK-MESSAGES: [[@LINE-1]]:20: warning: negative value used as length parameter
+ std::string_view q2("test", 200);
+ // CHECK-MESSAGES: [[@LINE-1]]:20: warning: length is bigger than string literal size
+ std::string_view q3(kText, 200);
+ // CHECK-MESSAGES: [[@LINE-1]]:20: warning: length is bigger than string literal size
+ std::string_view q4(kText2, 200);
+ // CHECK-MESSAGES: [[@LINE-1]]:20: warning: length is bigger than string literal size
+ std::string_view q5(kText3, 0x1000000);
+ // CHECK-MESSAGES: [[@LINE-1]]:20: warning: suspicious large length parameter
+ std::string_view q6(nullptr);
+ // CHECK-MESSAGES: [[@LINE-1]]:20: warning: constructing string from nullptr is undefined behaviour
+ std::string_view q7 = 0;
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: constructing string from nullptr is undefined behaviour
+}
+
std::string StringFromZero() {
return 0;
// CHECK-MESSAGES: [[@LINE-1]]:10: warning: constructing string from nullptr is undefined behaviour
}
+std::string_view StringViewFromZero() {
+ return 0;
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: constructing string from nullptr is undefined behaviour
+}
+
void Valid() {
std::string empty();
std::string str(4, 'x');
@@ -64,6 +97,11 @@ void Valid() {
std::string s1("test", 4);
std::string s2("test", 3);
std::string s3("test");
+
+ std::string_view emptyv();
+ std::string_view sv1("test", 4);
+ std::string_view sv2("test", 3);
+ std::string_view sv3("test");
}
namespace instantiation_dependent_exprs {
@@ -71,5 +109,6 @@ template<typename T>
struct S {
bool x;
std::string f() { return x ? "a" : "b"; }
+ std::string_view g() { return x ? "a" : "b"; }
};
}
More information about the cfe-commits
mailing list