================
@@ -0,0 +1,149 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "StringViewConversionsCheck.h"
+#include "clang/AST/Expr.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::performance {
+
+static auto getStringTypeMatcher(StringRef CharType) {
+  return hasCanonicalType(hasDeclaration(cxxRecordDecl(hasName(CharType))));
+}
+
+void StringViewConversionsCheck::registerMatchers(MatchFinder *Finder) {
+  // Matchers for std::basic_[w|u8|u16|u32]string and
+  // std::basic_[w|u8|u16|u32]string_view families.
+  const auto IsStdString = getStringTypeMatcher("::std::basic_string");
+  const auto IsStdStringView = 
getStringTypeMatcher("::std::basic_string_view");
+
+  // Matches pointer to any character type (char*, const char*, wchar_t*, etc.)
+  // or array of any character type (char[], char[N], const char[N], etc.).
+  const auto IsCharPointerOrArray =
+      anyOf(hasType(pointerType(pointee(isAnyCharacter()))),
+            hasType(arrayType(hasElementType(isAnyCharacter()))));
+
+  // Matches expressions that can be implicitly converted to string_view
+  // without going through std::string:
+  //   - string_view itself (no conversion needed)
+  //   - string literals ("hello", L"wide", u8"utf8", etc.)
+  //   - character pointers (const char*, char*)
+  //   - character arrays (char arr[], char arr[N])
+  //   - std::string (has implicit conversion to string_view)
+  // These are the expressions we want to preserve after removing
+  // the redundant std::string conversion.
+  const auto ImplicitlyConvertibleToStringView =
+      expr(anyOf(hasType(IsStdStringView), stringLiteral(),
+                 IsCharPointerOrArray, hasType(IsStdString)))
+          .bind("originalStringView");
+
+  // Matches std::string construction from a string_view-convertible 
expression.
+  // This handles cases like:
+  //   - Direct construction: std::string{sv}, std::string{s}
+  //   - Copy from existing string: std::string(s) where s is std::string
+  const auto RedundantStringConstruction = cxxConstructExpr(
+      hasType(IsStdString),
+      hasArgument(0, ignoringImplicit(ImplicitlyConvertibleToStringView)));
+
+  // Matches functional cast syntax: std::string(expr)
+  // In the AST, this appears as CXXFunctionalCastExpr containing
+  // a CXXConstructExpr. Example: std::string(sv), std::string("literal")
+  const auto RedundantFunctionalCast = cxxFunctionalCastExpr(
+      hasType(IsStdString), hasDescendant(RedundantStringConstruction));
+
+  // Match method calls on std::string that modify or use the string,
+  // such as operator+, append(), substr(), c_str(), etc.
+  // When these are present, the std::string construction is not redundant.
+  const auto HasStringOperatorCall = hasDescendant(cxxOperatorCallExpr(
+      hasOverloadedOperatorName("+"), hasType(IsStdString)));
+  const auto HasStringMethodCall =
+      hasDescendant(cxxMemberCallExpr(on(hasType(IsStdString))));
+
+  // Main matcher: finds function calls where:
+  // 1. A parameter has type string_view
+  // 2. The corresponding argument contains a redundant std::string 
construction
+  //    (either functional cast syntax or direct construction/brace init)
+  // 3. The argument does NOT involve:
+  //    - String concatenation with operator+ (string_view doesn't support it)
+  //    - Method calls on the std::string (like append(), substr(), etc.)
+  //
+  // Detected patterns (will be flagged):
+  //   void foo(std::string_view sv);
+  //   foo(std::string(sv));        // sv -> string -> string_view
+  //   foo(std::string{"literal"}); // literal -> string -> string_view
+  //   foo(std::string(ptr));       // const char* -> string -> string_view
+  //   foo(std::string(s));         // string -> string copy -> string_view
+  //
+  // Excluded patterns (will NOT be flagged):
+  //   foo(std::string(sv) + "suffix");     // operator+ requires std::string
+  //   foo("prefix" + std::string(sv));     // operator+ requires std::string
+  //   foo(std::string("x").append("y"));   // append() requires std::string
+  //   foo(std::string(sv).substr(0, 5));   // substr() requires std::string
+  Finder->addMatcher(
+      callExpr(
+          forEachArgumentWithParam(
+              expr(hasType(IsStdStringView),
+                   // Match either syntax for std::string construction
+                   hasDescendant(expr(anyOf(RedundantFunctionalCast,
+                                            RedundantStringConstruction))
+                                     .bind("redundantExpr")),
+                   // Exclude cases of std::string methods or operator+ calls
+                   unless(anyOf(HasStringOperatorCall, HasStringMethodCall)))
----------------
localspook wrote:

It feels a bit inelegant to match using `hasDescendant` and then add specific 
exclusions because `hasDescendant` matches too much. Could we avoid 
`hasDescendant` altogether, with something like:
```suggestion
                   ignoringParenImpCasts(expr(anyOf(RedundantFunctionalCast,
                                                    
RedundantStringConstruction))
                                         .bind("redundantExpr"))
```
? If that works, I imagine it has better performance too.

https://github.com/llvm/llvm-project/pull/174288
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to