[clang-tidy] bugprone-string-constructor: Catch string from nullptr.

[courbet]

Summary

Context: https://twitter.com/willkirkby/status/1084219580799741953

[courbet]

Remove commented code.

[aaron.ballman]

LGTM aside from a nit and a test case request.

clang-tidy/bugprone/StringConstructorCheck.cpp

141

Formatting is wrong here, and you can use const auto * as the type is spelled out in the initialization.

test/clang-tidy/bugprone-string-constructor.cpp

49–50

Can you also add: std::string q7 = 0; as a test case?

[courbet]

clang-format + add test

[courbet]

Thanks.

clang-tidy/bugprone/StringConstructorCheck.cpp

Finder->addMatcher(
               // Detect the expression: string("lit", 0x1234567);
               hasArgument(1, LargeLengthExpr.bind("large-length")),
               // Detect the expression: string("lit", 5)
               allOf(hasArgument(0, ConstStrLiteral.bind("literal-with-length")),
                     hasArgument(1, ignoringParenImpCasts(
                                        integerLiteral().bind("int"))))))
           .bind("constructor"),
       this);
+
+  // Check the literal string constructor with char pointer.
+  // [i.e. string (const char* s);]
+  Finder->addMatcher(
+      cxxConstructExpr(
+          hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
+          hasArgument(0, expr().bind("from-ptr")),
+          hasArgument(1, unless(hasType(isInteger()))))
+          .bind("constructor"),
+      this);
 }
 
 void StringConstructorCheck::check(const MatchFinder::MatchResult &Result) {
   const ASTContext &Ctx = *Result.Context;
   const auto *E = Result.Nodes.getNodeAs<CXXConstructExpr>("constructor");
   assert(E && "missing constructor expression");
   SourceLocation Loc = E->getBeginLoc();
 
if (Result.Nodes.getNodeAs<Expr>("swapped-parameter")) {
     if (WarnOnLargeLength)
       diag(Loc, "suspicious large length parameter");
   } else if (Result.Nodes.getNodeAs<Expr>("literal-with-length")) {
     const auto *Str = Result.Nodes.getNodeAs<StringLiteral>("str");
     const auto *Lit = Result.Nodes.getNodeAs<IntegerLiteral>("int");
     if (Lit->getValue().ugt(Str->getLength())) {
       diag(Loc, "length is bigger then string literal size");
     }
+  } else if (const Expr* Ptr = Result.Nodes.getNodeAs<Expr>("from-ptr")) {
+    Expr::EvalResult ConstPtr;
+    if (Ptr->EvaluateAsRValue(ConstPtr, Ctx) && (
+        (ConstPtr.Val.isInt() && ConstPtr.Val.getInt().isNullValue()) ||
+            (ConstPtr.Val.isLValue() && ConstPtr.Val.isNullPointer())
+      )) {
+      diag(Loc, "constructing string from nullptr is undefined behaviour");
+    }
   }
 }
 
 } // namespace bugprone
 } // namespace tidy
 } // namespace clang

test/clang-tidy/bugprone-string-constructor.cpp

 // RUN: %check_clang_tidy %s bugprone-string-constructor %t
 
 namespace std {
 template <typename T>
 class allocator {};
 template <typename T>
 class char_traits {};
 template <typename C, typename T = std::char_traits<C>, typename A = std::allocator<C> >
 struct basic_string {
   basic_string();
   basic_string(const C*, unsigned int size);
+  basic_string(const C*, const A& allocator = A());
   basic_string(unsigned int size, C c);
 };
 typedef basic_string<char> string;
 typedef basic_string<wchar_t> wstring;
 }
 
 const char* kText = "";
 const char kText2[] = "";
void Test() {
   std::string q2("test", 200);
   // CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger then string literal size
   std::string q3(kText, 200);
   // CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger then string literal size
   std::string q4(kText2, 200);
   // CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger then string literal size
   std::string q5(kText3,  0x1000000);
   // CHECK-MESSAGES: [[@LINE-1]]:15: warning: suspicious large length parameter
+  std::string q6(nullptr);
+  // CHECK-MESSAGES: [[@LINE-1]]:15: 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
 }
 
 void Valid() {
   std::string empty();
   std::string str(4, 'x');
   std::wstring wstr(4, L'x');
   std::string s1("test", 4);
   std::string s2("test", 3);
+  std::string s3("test");
 }