[clang-tools-extra] a568411 - [clang-tidy] Update bugprone-stringview-nullptr to consistently prefer the empty string when passing arguments to constructors/functions
Author: CJ Johnson
Date: 2022-01-20T18:08:40-05:00
New Revision: a5684114445a72b5c0bb5b7b68a5c6eb3486b66d
URL:
https://github.com/llvm/llvm-project/commit/a5684114445a72b5c0bb5b7b68a5c6eb3486b66d
DIFF:
https://github.com/llvm/llvm-project/commit/a5684114445a72b5c0bb5b7b68a5c6eb3486b66d.diff
LOG: [clang-tidy] Update bugprone-stringview-nullptr to consistently prefer the
empty string when passing arguments to constructors/functions
Previously, function(nullptr) would have been fixed with function({}). This
unfortunately can change overload resolution and even become ambiguous.
T(nullptr) was already being fixed with T(""), so this change just brings
function calls in line with that.
Differential Revision: https://reviews.llvm.org/D117840
Added:
Modified:
clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
clang-tools-extra/docs/clang-tidy/checks/bugprone-stringview-nullptr.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
Removed:
diff --git a/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
index a0ae262318914..b45aa93533b08 100644
--- a/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
@@ -44,6 +44,9 @@ RewriteRule StringviewNullptrCheckImpl() {
auto static_cast_warning =
cat("casting to basic_string_view from null is undefined; replace with "
"the empty string");
+ auto argument_construction_warning =
+ cat("passing null as basic_string_view is undefined; replace with the "
+ "empty string");
auto assignment_warning =
cat("assignment to basic_string_view from null is undefined; replace "
"with the default constructor");
@@ -53,9 +56,6 @@ RewriteRule StringviewNullptrCheckImpl() {
auto equality_comparison_warning =
cat("comparing basic_string_view to null is undefined; replace with the "
"emptiness query");
- auto constructor_argument_warning =
- cat("passing null as basic_string_view is undefined; replace with the "
- "empty string");
// Matches declarations and expressions of type `basic_string_view`
auto HasBasicStringViewType = hasType(hasUnqualifiedDesugaredType(recordType(
@@ -211,11 +211,12 @@ RewriteRule StringviewNullptrCheckImpl() {
remove(node("null_arg_expr")), construction_warning);
// `function(null_arg_expr)`
- auto HandleFunctionArgumentInitialization = makeRule(
- callExpr(hasAnyArgument(
- ignoringImpCasts(BasicStringViewConstructingFromNullExpr)),
- unless(cxxOperatorCallExpr())),
- changeTo(node("construct_expr"), cat("{}")), construction_warning);
+ auto HandleFunctionArgumentInitialization =
+ makeRule(callExpr(hasAnyArgument(ignoringImpCasts(
+BasicStringViewConstructingFromNullExpr)),
+unless(cxxOperatorCallExpr())),
+ changeTo(node("construct_expr"), cat("\"\"")),
+ argument_construction_warning);
// `sv = null_arg_expr`
auto HandleAssignment = makeRule(
@@ -268,7 +269,7 @@ RewriteRule StringviewNullptrCheckImpl() {
BasicStringViewConstructingFromNullExpr)),
unless(HasBasicStringViewType)),
changeTo(node("construct_expr"), cat("\"\"")),
- constructor_argument_warning);
+ argument_construction_warning);
return applyFirst(
{HandleTemporaryCXXFunctionalCastExpr,
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/bugprone-stringview-nullptr.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone-stringview-nullptr.rst
index 198ad398ec7b7..7138c97b745ae 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone-stringview-nullptr.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-stringview-nullptr.rst
@@ -43,9 +43,9 @@ is translated into...
bool is_empty = sv.empty();
bool isnt_empty = !sv.empty();
- accepts_sv({});
+ accepts_sv("");
- accepts_sv({}); // A
+ accepts_sv(""); // A
accepts_sv({nullptr, 0}); // B
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
index 322c8eeca754e..02fcab31dcf3e 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
@@ -1039,24 +1039,24 @@ void function_argument_initialization() /* f */ {
// Function Argument Initialization
{
function(nullptr) /* f1 */;
-// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: constructing{{.*}}default
-// CHECK-FIXES: {{^}}function({}) /* f1 */;
+
[clang-tools-extra] 7e29da8 - Add support for return values in bugprone-stringview-nullptr
Author: CJ Johnson
Date: 2022-01-12T16:53:13-05:00
New Revision: 7e29da875ca95cd916e7ce6668fa85b638c3bd5f
URL:
https://github.com/llvm/llvm-project/commit/7e29da875ca95cd916e7ce6668fa85b638c3bd5f
DIFF:
https://github.com/llvm/llvm-project/commit/7e29da875ca95cd916e7ce6668fa85b638c3bd5f.diff
LOG: Add support for return values in bugprone-stringview-nullptr
bugprone-stringview-nullptr was not initially written with tests for return
statements. After landing the check, the thought crossed my mind to add such
tests. After writing them, I realized they needed additional handling in the
matchers.
Differential Revision: https://reviews.llvm.org/D115121
Added:
Modified:
clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
Removed:
diff --git a/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
index 1f7dc48bbe8fb..a0ae262318914 100644
--- a/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
@@ -9,6 +9,8 @@
#include "StringviewNullptrCheck.h"
#include "../utils/TransformerClangTidyCheck.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/OperationKinds.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Tooling/Transformer/RangeSelector.h"
@@ -24,15 +26,24 @@ using namespace ::clang::ast_matchers;
using namespace ::clang::transformer;
namespace {
+
AST_MATCHER_P(InitListExpr, initCountIs, unsigned, N) {
return Node.getNumInits() == N;
}
+
+AST_MATCHER(clang::VarDecl, isDirectInitialization) {
+ return Node.getInitStyle() != clang::VarDecl::InitializationStyle::CInit;
+}
+
} // namespace
RewriteRule StringviewNullptrCheckImpl() {
auto construction_warning =
cat("constructing basic_string_view from null is undefined; replace with
"
"the default constructor");
+ auto static_cast_warning =
+ cat("casting to basic_string_view from null is undefined; replace with "
+ "the empty string");
auto assignment_warning =
cat("assignment to basic_string_view from null is undefined; replace "
"with the default constructor");
@@ -42,143 +53,247 @@ RewriteRule StringviewNullptrCheckImpl() {
auto equality_comparison_warning =
cat("comparing basic_string_view to null is undefined; replace with the "
"emptiness query");
- auto StringViewConstructingFromNullExpr =
+ auto constructor_argument_warning =
+ cat("passing null as basic_string_view is undefined; replace with the "
+ "empty string");
+
+ // Matches declarations and expressions of type `basic_string_view`
+ auto HasBasicStringViewType = hasType(hasUnqualifiedDesugaredType(recordType(
+ hasDeclaration(cxxRecordDecl(hasName("::std::basic_string_view"));
+
+ // Matches `nullptr` and `(nullptr)` binding to a pointer
+ auto NullLiteral = implicitCastExpr(
+ hasCastKind(clang::CK_NullToPointer),
+ hasSourceExpression(ignoringParens(cxxNullPtrLiteralExpr(;
+
+ // Matches `{nullptr}` and `{(nullptr)}` binding to a pointer
+ auto NullInitList = initListExpr(initCountIs(1), hasInit(0, NullLiteral));
+
+ // Matches `{}`
+ auto EmptyInitList = initListExpr(initCountIs(0));
+
+ // Matches null construction without `basic_string_view` type spelling
+ auto BasicStringViewConstructingFromNullExpr =
cxxConstructExpr(
- hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration(
- cxxRecordDecl(hasName("::std::basic_string_view")),
- argumentCountIs(1),
- hasArgument(
- 0, anyOf(ignoringParenImpCasts(cxxNullPtrLiteralExpr()),
- initListExpr(initCountIs(1),
-hasInit(0, ignoringParenImpCasts(
- cxxNullPtrLiteralExpr(,
- initListExpr(initCountIs(0,
- has(expr().bind("null_argument_expr")))
+ HasBasicStringViewType, argumentCountIs(1),
+ hasAnyArgument(/* `hasArgument` would skip over parens */ anyOf(
+ NullLiteral, NullInitList, EmptyInitList)),
+ unless(cxxTemporaryObjectExpr(/* filters out type spellings */)),
+ has(expr().bind("null_arg_expr")))
.bind("construct_expr");
+ // `std::string_view(null_arg_expr)`
auto HandleTemporaryCXXFunctionalCastExpr =
- makeRule(cxxFunctionalCastExpr(
- hasSourceExpression(StringViewConstructingFromNullExpr)),
- remove(node("null_argument_expr")), construction_warning);
+ makeRule(cxxFunctionalCastExpr(hasSourceExpression(
+ Basic
[clang-tools-extra] 81c330e - Filter string_view from the nullptr diagnosis of bugprone-string-constructor to prevent duplicate warnings with bugprone-stringview-nullptr
Author: CJ Johnson
Date: 2022-01-12T17:04:44-05:00
New Revision: 81c330e23dd4a14736b977e246bdca25be9b5770
URL:
https://github.com/llvm/llvm-project/commit/81c330e23dd4a14736b977e246bdca25be9b5770
DIFF:
https://github.com/llvm/llvm-project/commit/81c330e23dd4a14736b977e246bdca25be9b5770.diff
LOG: Filter string_view from the nullptr diagnosis of
bugprone-string-constructor to prevent duplicate warnings with
bugprone-stringview-nullptr
Updates the check and tests to not diagnose the null case for string_view (but
retains it for string). This prevents the check from giving duplicate warnings
that are caught by bugprone-stringview-nullptr ([[
https://reviews.llvm.org/D113148 | D113148 ]]).
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D114823
Added:
Modified:
clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
Removed:
diff --git a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
index beca3933bcd2..b55b282c41aa 100644
--- a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
@@ -43,6 +43,8 @@ removeNamespaces(const std::vector &Names) {
StringConstructorCheck::StringConstructorCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
+ IsStringviewNullptrCheckEnabled(
+ Context->isCheckEnabled("bugprone-stringview-nullptr")),
WarnOnLargeLength(Options.get("WarnOnLargeLength", true)),
LargeLengthThreshold(Options.get("LargeLengthThreshold", 0x80)),
StringNames(utils::options::parseStringList(
@@ -121,17 +123,20 @@ 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(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")),
+ traverse(
+ TK_AsIs,
+ cxxConstructExpr(
+ hasDeclaration(cxxConstructorDecl(ofClass(anyOf(
+ cxxRecordDecl(hasName("basic_string_view"))
+ .bind("basic_string_view_decl"),
+ 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);
}
@@ -167,6 +172,12 @@ void StringConstructorCheck::check(const
MatchFinder::MatchResult &Result) {
Ptr->EvaluateAsRValue(ConstPtr, Ctx) &&
((ConstPtr.Val.isInt() && ConstPtr.Val.getInt().isZero()) ||
(ConstPtr.Val.isLValue() && ConstPtr.Val.isNullPointer( {
+ if (IsStringviewNullptrCheckEnabled &&
+ Result.Nodes.getNodeAs("basic_string_view_decl")) {
+// Filter out `basic_string_view` to avoid conflicts with
+// `bugprone-stringview-nullptr`
+return;
+ }
diag(Loc, "constructing string from nullptr is undefined behaviour");
}
}
diff --git a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
index 50338f8abead..d4877fbaa235 100644
--- a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
@@ -30,6 +30,7 @@ class StringConstructorCheck : public ClangTidyCheck {
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
private:
+ const bool IsStringviewNullptrCheckEnabled;
const bool WarnOnLargeLength;
const unsigned int LargeLengthThreshold;
std::vector StringNames;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
