CJ-Johnson updated this revision to Diff 391940.
CJ-Johnson added a comment.
Add missing tests for function arguments and fix incorrect warning message for
static_cast cases
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115121/new/
https://reviews.llvm.org/D115121
Files:
clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
@@ -88,8 +88,21 @@
} // namespace std
-void function(std::string_view);
-void function(std::string_view, std::string_view);
+void sv_function(std::string_view);
+void sv_function(std::string_view, std::string_view);
+
+// TODO: Handle cases where types, such as Class and Struct below, are
+// constructed with null arguments.
+class Class {
+public:
+ Class(std::string_view);
+ Class(std::string_view, std::string_view);
+};
+struct Struct {
+ std::string_view first = {};
+ std::string_view second = {};
+};
+void type_function(Class, Struct);
void temporary_construction() /* a */ {
// Functional Cast
@@ -197,23 +210,54 @@
// CHECK-FIXES: {{^}} (void)((const std::string_view){}) /* a24 */;
}
+ // Return Value
+ {
+ (void)([] -> std::string_view { return nullptr; }) /* a25 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} (void)([] -> std::string_view { return {}; }) /* a25 */;
+
+ (void)([] -> std::string_view { return (nullptr); }) /* a26 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} (void)([] -> std::string_view { return {}; }) /* a26 */;
+
+ (void)([] -> std::string_view { return {nullptr}; }) /* a27 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} (void)([] -> std::string_view { return {}; }) /* a27 */;
+
+ (void)([] -> std::string_view { return {(nullptr)}; }) /* a28 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} (void)([] -> std::string_view { return {}; }) /* a28 */;
+
+ (void)([] -> std::string_view { return {{nullptr}}; }) /* a29 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} (void)([] -> std::string_view { return {}; }) /* a29 */;
+
+ (void)([] -> std::string_view { return {{(nullptr)}}; }) /* a30 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} (void)([] -> std::string_view { return {}; }) /* a30 */;
+
+ (void)([] -> std::string_view { return {{}}; }) /* a31 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} (void)([] -> std::string_view { return {}; }) /* a31 */;
+ }
+
// Static Cast
{
- (void)(static_cast<std::string_view>(nullptr)) /* a25 */;
- // CHECK-MESSAGES: :[[@LINE-1]]:42: warning: constructing{{.*}}default
- // CHECK-FIXES: {{^}} (void)(static_cast<std::string_view>("")) /* a25 */;
+ (void)(static_cast<std::string_view>(nullptr)) /* a32 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:42: warning: constructing basic_string_view from null is undefined; replace with the empty string
+ // CHECK-FIXES: {{^}} (void)(static_cast<std::string_view>("")) /* a32 */;
- (void)(static_cast<std::string_view>((nullptr))) /* a26 */;
- // CHECK-MESSAGES: :[[@LINE-1]]:42: warning: constructing{{.*}}default
- // CHECK-FIXES: {{^}} (void)(static_cast<std::string_view>("")) /* a26 */;
+ (void)(static_cast<std::string_view>((nullptr))) /* a33 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:42: warning: constructing{{.*}}empty string
+ // CHECK-FIXES: {{^}} (void)(static_cast<std::string_view>("")) /* a33 */;
- (void)(static_cast<const std::string_view>(nullptr)) /* a27 */;
- // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: constructing{{.*}}default
- // CHECK-FIXES: {{^}} (void)(static_cast<const std::string_view>("")) /* a27 */;
+ (void)(static_cast<const std::string_view>(nullptr)) /* a34 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: constructing{{.*}}empty string
+ // CHECK-FIXES: {{^}} (void)(static_cast<const std::string_view>("")) /* a34 */;
- (void)(static_cast<const std::string_view>((nullptr))) /* a28 */;
- // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: constructing{{.*}}default
- // CHECK-FIXES: {{^}} (void)(static_cast<const std::string_view>("")) /* a28 */;
+ (void)(static_cast<const std::string_view>((nullptr))) /* a35 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: constructing{{.*}}empty string
+ // CHECK-FIXES: {{^}} (void)(static_cast<const std::string_view>("")) /* a35 */;
}
}
@@ -689,55 +733,73 @@
}
void function_invocation() /* f */ {
- // Single Argument
+ // Single Argument Function Invocation
{
- function(nullptr) /* f1 */;
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: constructing{{.*}}default
- // CHECK-FIXES: {{^}} function({}) /* f1 */;
+ sv_function(nullptr) /* f1 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} sv_function({}) /* f1 */;
+
+ sv_function((nullptr)) /* f2 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} sv_function({}) /* f2 */;
- function((nullptr)) /* f2 */;
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: constructing{{.*}}default
- // CHECK-FIXES: {{^}} function({}) /* f2 */;
+ sv_function({nullptr}) /* f3 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} sv_function({}) /* f3 */;
- function({nullptr}) /* f3 */;
- // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: constructing{{.*}}default
- // CHECK-FIXES: {{^}} function({}) /* f3 */;
+ sv_function({(nullptr)}) /* f4 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} sv_function({}) /* f4 */;
- function({(nullptr)}) /* f4 */;
- // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: constructing{{.*}}default
- // CHECK-FIXES: {{^}} function({}) /* f4 */;
+ sv_function({{nullptr}}) /* f5 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} sv_function({}) /* f5 */;
- function({{}}) /* f5 */; // Default `const CharT*`
- // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: constructing{{.*}}default
- // CHECK-FIXES: {{^}} function({}) /* f5 */;
+ sv_function({{(nullptr)}}) /* f6 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} sv_function({}) /* f6 */;
+
+ sv_function({{}}) /* f7 */; // Default `const CharT*`
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} sv_function({}) /* f7 */;
}
- // Multiple Argument
+ // Multiple Argument Function Invocation
{
- function(nullptr, nullptr) /* f6 */;
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: constructing{{.*}}default
- // CHECK-MESSAGES: :[[@LINE-2]]:23: warning: constructing{{.*}}default
- // CHECK-FIXES: {{^}} function({}, {}) /* f6 */;
-
- function((nullptr), (nullptr)) /* f7 */;
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: constructing{{.*}}default
- // CHECK-MESSAGES: :[[@LINE-2]]:25: warning: constructing{{.*}}default
- // CHECK-FIXES: {{^}} function({}, {}) /* f7 */;
-
- function({nullptr}, {nullptr}) /* f8 */;
- // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: constructing{{.*}}default
+ sv_function(nullptr, nullptr) /* f8 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: constructing{{.*}}default
// CHECK-MESSAGES: :[[@LINE-2]]:26: warning: constructing{{.*}}default
- // CHECK-FIXES: {{^}} function({}, {}) /* f8 */;
+ // CHECK-FIXES: {{^}} sv_function({}, {}) /* f8 */;
- function({(nullptr)}, {(nullptr)}) /* f9 */;
- // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: constructing{{.*}}default
+ sv_function((nullptr), (nullptr)) /* f9 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: constructing{{.*}}default
// CHECK-MESSAGES: :[[@LINE-2]]:28: warning: constructing{{.*}}default
- // CHECK-FIXES: {{^}} function({}, {}) /* f9 */;
-
- function({{}}, {{}}) /* f10 */; // Default `const CharT*`s
- // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: constructing{{.*}}default
- // CHECK-MESSAGES: :[[@LINE-2]]:21: warning: constructing{{.*}}default
- // CHECK-FIXES: {{^}} function({}, {}) /* f10 */;
+ // CHECK-FIXES: {{^}} sv_function({}, {}) /* f9 */;
+
+ sv_function({nullptr}, {nullptr}) /* f10 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: constructing{{.*}}default
+ // CHECK-MESSAGES: :[[@LINE-2]]:29: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} sv_function({}, {}) /* f10 */;
+
+ sv_function({(nullptr)}, {(nullptr)}) /* f11 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: constructing{{.*}}default
+ // CHECK-MESSAGES: :[[@LINE-2]]:31: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} sv_function({}, {}) /* f11 */;
+
+ sv_function({{nullptr}}, {{nullptr}}) /* f12 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: constructing{{.*}}default
+ // CHECK-MESSAGES: :[[@LINE-2]]:31: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} sv_function({}, {}) /* f12 */;
+
+ sv_function({{(nullptr)}}, {{(nullptr)}}) /* f13 */;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: constructing{{.*}}default
+ // CHECK-MESSAGES: :[[@LINE-2]]:33: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} sv_function({}, {}) /* f13 */;
+
+ sv_function({{}}, {{}}) /* f14 */; // Default `const CharT*`s
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: constructing{{.*}}default
+ // CHECK-MESSAGES: :[[@LINE-2]]:24: warning: constructing{{.*}}default
+ // CHECK-FIXES: {{^}} sv_function({}, {}) /* f14 */;
}
}
Index: clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
@@ -33,6 +33,9 @@
auto construction_warning =
cat("constructing basic_string_view from null is undefined; replace with "
"the default constructor");
+ auto fallback_construction_warning =
+ cat("constructing 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");
@@ -69,10 +72,16 @@
cStyleCastExpr(hasSourceExpression(StringViewConstructingFromNullExpr)),
changeTo(node("null_argument_expr"), cat("{}")), construction_warning);
- auto HandleTemporaryCXXStaticCastExpr = makeRule(
- cxxStaticCastExpr(
- hasSourceExpression(StringViewConstructingFromNullExpr)),
- changeTo(node("null_argument_expr"), cat("\"\"")), construction_warning);
+ auto HandleTemporaryReturnValue = makeRule(
+ returnStmt(
+ hasReturnValue(ignoringImpCasts(StringViewConstructingFromNullExpr))),
+ changeTo(node("construct_expr"), cat("{}")), construction_warning);
+
+ auto HandleTemporaryCXXStaticCastExpr =
+ makeRule(cxxStaticCastExpr(
+ hasSourceExpression(StringViewConstructingFromNullExpr)),
+ changeTo(node("null_argument_expr"), cat("\"\"")),
+ fallback_construction_warning);
auto HandleStackCopyInitialization = makeRule(
varDecl(hasInitializer(implicitCastExpr(
@@ -169,15 +178,23 @@
return applyFirst(
{HandleTemporaryCXXFunctionalCastExpr,
HandleTemporaryCXXTemporaryObjectExprAndCompoundLiteralExpr,
- HandleTemporaryCStyleCastExpr, HandleTemporaryCXXStaticCastExpr,
- HandleStackCopyInitialization, HandleStackDirectInitialization,
+ HandleTemporaryCStyleCastExpr,
+ HandleTemporaryReturnValue,
+ HandleTemporaryCXXStaticCastExpr,
+ HandleStackCopyInitialization,
+ HandleStackDirectInitialization,
HandleStackDirectListAndCopyListInitialization,
- HandleFieldCopyInitialization, HandleFieldOtherInitialization,
- HandleConstructorInitialization, HandleDefaultArgumentInitialization,
- HandleDefaultArgumentListInitialization, HandleHeapInitialization,
+ HandleFieldCopyInitialization,
+ HandleFieldOtherInitialization,
+ HandleConstructorInitialization,
+ HandleDefaultArgumentInitialization,
+ HandleDefaultArgumentListInitialization,
+ HandleHeapInitialization,
HandleFunctionArgumentInitialization,
- HandleFunctionArgumentListInitialization, HandleAssignment,
- HandleRelativeComparison, HandleEmptyEqualityComparison,
+ HandleFunctionArgumentListInitialization,
+ HandleAssignment,
+ HandleRelativeComparison,
+ HandleEmptyEqualityComparison,
HandleNonEmptyEqualityComparison});
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
