diff --git a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt @@ -9,6 +9,7 @@ LLVMTidyModule.cpp PreferIsaOrDynCastInConditionalsCheck.cpp PreferRegisterOverUnsignedCheck.cpp + StringReferencingCheck.cpp TwineLocalCheck.cpp LINK_LIBS diff --git a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp --- a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp @@ -16,6 +16,7 @@ #include "IncludeOrderCheck.h" #include "PreferIsaOrDynCastInConditionalsCheck.h" #include "PreferRegisterOverUnsignedCheck.h" +#include "StringReferencingCheck.h" #include "TwineLocalCheck.h" namespace clang { @@ -37,6 +38,8 @@ "llvm-prefer-register-over-unsigned"); CheckFactories.registerCheck( "llvm-qualified-auto"); + CheckFactories.registerCheck( + "llvm-string-referencing"); CheckFactories.registerCheck("llvm-twine-local"); } diff --git a/clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.h b/clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.h @@ -0,0 +1,46 @@ +//===--- StringReferencingCheck.h - clang-tidy ------------------*- C++ -*-===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_STRINGREFERENCINGCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_STRINGREFERENCINGCHECK_H + +#include "../ClangTidyCheck.h" +#include "../utils/IncludeInserter.h" +#include "clang/Frontend/CompilerInstance.h" + +namespace clang { +namespace tidy { +namespace llvm_check { + +/// LThe LLVM Programmer's Manual recommends that llvm::StringRef should be +/// used for function parameters instead of references to const std::string: +/// https://llvm.org/docs/ProgrammersManual.html#the-stringref-class +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/llvm-string-referencing.html +class StringReferencingCheck : public ClangTidyCheck { + const llvm::StringRef BoundNodeId = "string_reference"; + +public: + StringReferencingCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + Inserter(Options.getLocalOrGlobal("IncludeStyle", + utils::IncludeSorter::IS_LLVM)) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + utils::IncludeInserter Inserter; +}; + +} // namespace llvm_check +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_STRINGREFERENCINGCHECK_H diff --git a/clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.cpp b/clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.cpp @@ -0,0 +1,63 @@ +//===--- StringReferencingCheck.cpp - clang-tidy --------------------------===// +// +// 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 "StringReferencingCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace llvm_check { + +void StringReferencingCheck::registerMatchers(MatchFinder *Finder) { + // create a matcher looking for const std::string& parameters + auto Matcher = + parmVarDecl(hasType(references(namedDecl( + hasName("string"), isInStdNamespace()))), // std::string& + hasType(references(qualType(isConstQualified()))), // const + hasType(lValueReferenceType()) // don't match std::string&& + ); + // Add matcher to the finder and bound matched nodes + Finder->addMatcher(Matcher.bind(BoundNodeId), this); +} + +void StringReferencingCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + Inserter.registerPreprocessor(PP); +} + +void StringReferencingCheck::check(const MatchFinder::MatchResult &Result) { + // Retrieve parameters found by matcher + const auto *ParamDecl = Result.Nodes.getNodeAs(BoundNodeId); + // Extract parameter type location in source + clang::SourceRange TypeRange; + auto ParamBegin = ParamDecl->getBeginLoc(); + if (!ParamBegin.isValid()) + return; + TypeRange.setBegin(ParamBegin); + TypeRange.setEnd(ParamDecl->getTypeSpecEndLoc()); + // Generate diagnostics and fix + diag(ParamBegin, "Use of const std::string & is discouraged in the LLVM " + "Programmer's Manual"); + diag(ParamBegin, "replace parameter %0 type with llvm::StringRef", + DiagnosticIDs::Note) + << ParamDecl + << FixItHint::CreateReplacement(TypeRange, "llvm::StringRef ") + << Inserter.createMainFileIncludeInsertion( + "llvm/include/llvm/ADT/StringRef.h", + /*IsAngled=*/false); +} + +} // namespace llvm_check +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -192,6 +192,7 @@ `llvm-namespace-comment `_, `llvm-prefer-isa-or-dyn-cast-in-conditionals `_, "Yes" `llvm-prefer-register-over-unsigned `_, "Yes" + `llvm-string-referencing `_, "Yes" `llvm-twine-local `_, "Yes" `llvmlibc-callee-namespace `_, `llvmlibc-implementation-in-namespace `_, diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm-string-referencing.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm-string-referencing.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm-string-referencing.rst @@ -0,0 +1,6 @@ +.. title:: clang-tidy - llvm-string-referencing + +llvm-string-referencing +======================= + +FIXME: Describe what patterns does the check detect and why. Give examples. diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm-string-referencing.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm-string-referencing.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm-string-referencing.cpp @@ -0,0 +1,66 @@ +// RUN: %check_clang_tidy %s llvm-string-referencing %t + +namespace std { +class string {}; +class u18_string_t; + +} // namespace std + +namespace llvm { +class StringRef; +} // namespace llvm + +class String; + +namespace A { +using namespace std; +void f(const string &P); +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] +// CHECK-FIXES: void f(llvm::StringRef P);{{$}} +} // namespace A + +namespace B { +using std::string; +void f1(const string &P); +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] +// CHECK-FIXES: void f1(llvm::StringRef P);{{$}} +} // namespace B + +void f2(std::string, int, const std::string &); +// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] +// CHECK-FIXES: void f2(std::string, int, llvm::StringRef );{{$}} +void f2(std::string P, int x, const std::string &P2) { + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] + // CHECK-FIXES: void f2(std::string P, int x, llvm::StringRef P2) {{{$}} + return; +} + +void f3(const std::string &P1, const std::string &P2); +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] +// CHECK-MESSAGES: :[[@LINE-2]]:32: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] +// CHECK-FIXES: void f3(llvm::StringRef P1, llvm::StringRef P2);{{$}} + +struct St { + void operator=(const std::string &Val) const { + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] + // CHECK-FIXES: void operator=(llvm::StringRef Val) const {{{$}} + return; + } +}; + +void f7(const std::string &); +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] +// CHECK-FIXES: void f7(llvm::StringRef );{{$}} + +// Functions below this line should not trigger the check +void f1(std::string &P); + +void f4(std::string *P); + +void f5(String &P); + +void f6(llvm::StringRef P); + +void f9(std::u18_string_t &P); + +void f10(const std::string &&P);