On Wed, Jul 16, 2014 at 3:00 AM, Benjamin Kramer <[email protected]> wrote: > Author: d0k > Date: Wed Jul 16 05:00:14 2014 > New Revision: 213133 > > URL: http://llvm.org/viewvc/llvm-project?rev=213133&view=rev > Log: > [clang-tidy] Add a checker that warns on const string & members.
Why strings specifically? I guess this is what was checked for in cpplint.py? At least the Google style guide mostly pushes on the actual ctor parameter type - that that type should be a pointer (in the same way Google style uses pointers to indicate mutability, it also espouses them to indicate "is stored/used beyond the lifetime of this function/ctor call"). This seems like a case where someone fixed an issue too narrowly, and a step or two from the root cause, and we maybe shouldn't duplicate that and instead diagnose the 'root' cause. > > Summary: > Those are considered unsafe and should be replaced with simple pointers or > full copies. It recognizes both std::string and ::string. > > Reviewers: alexfh, djasper > > Subscribers: cfe-commits > > Differential Revision: http://reviews.llvm.org/D4522 > > Added: > clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.cpp > clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.h > > clang-tools-extra/trunk/test/clang-tidy/google-member-string-references.cpp > Modified: > clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt > clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp > > Modified: clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt?rev=213133&r1=213132&r2=213133&view=diff > ============================================================================== > --- clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt (original) > +++ clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt Wed Jul 16 > 05:00:14 2014 > @@ -7,6 +7,7 @@ add_clang_library(clangTidyGoogleModule > GoogleTidyModule.cpp > NamedParameterCheck.cpp > OverloadedUnaryAndCheck.cpp > + StringReferenceMemberCheck.cpp > > LINK_LIBS > clangAST > > Modified: clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp?rev=213133&r1=213132&r2=213133&view=diff > ============================================================================== > --- clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp (original) > +++ clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp Wed Jul 16 > 05:00:14 2014 > @@ -15,6 +15,7 @@ > #include "ExplicitMakePairCheck.h" > #include "NamedParameterCheck.h" > #include "OverloadedUnaryAndCheck.h" > +#include "StringReferenceMemberCheck.h" > > using namespace clang::ast_matchers; > > @@ -34,6 +35,9 @@ public: > "google-runtime-operator", > new ClangTidyCheckFactory<runtime::OverloadedUnaryAndCheck>()); > CheckFactories.addCheckFactory( > + "google-runtime-member-string-references", > + new ClangTidyCheckFactory<runtime::StringReferenceMemberCheck>()); > + CheckFactories.addCheckFactory( > "google-readability-casting", > new ClangTidyCheckFactory<readability::AvoidCStyleCastsCheck>()); > CheckFactories.addCheckFactory( > > Added: > clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.cpp > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.cpp?rev=213133&view=auto > ============================================================================== > --- clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.cpp > (added) > +++ clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.cpp > Wed Jul 16 05:00:14 2014 > @@ -0,0 +1,48 @@ > +//===--- StringReferenceMemberCheck.cpp - clang-tidy ------------*- C++ > -*-===// > +// > +// The LLVM Compiler Infrastructure > +// > +// This file is distributed under the University of Illinois Open Source > +// License. See LICENSE.TXT for details. > +// > +//===----------------------------------------------------------------------===// > + > +#include "StringReferenceMemberCheck.h" > +#include "clang/ASTMatchers/ASTMatchFinder.h" > +#include "clang/ASTMatchers/ASTMatchers.h" > +#include "clang/AST/ASTContext.h" > + > +using namespace clang::ast_matchers; > + > +namespace clang { > +namespace tidy { > +namespace runtime { > + > +void StringReferenceMemberCheck::registerMatchers( > + ast_matchers::MatchFinder *Finder) { > + // Look for const references to std::string or ::string. > + auto String = anyOf(recordDecl(hasName("::std::basic_string")), > + recordDecl(hasName("::string"))); > + auto ConstString = qualType(isConstQualified(), hasDeclaration(String)); > + > + // Ignore members in template instantiations. > + auto InTemplateInstantiation = hasAncestor( > + decl(anyOf(recordDecl(ast_matchers::isTemplateInstantiation()), > + functionDecl(ast_matchers::isTemplateInstantiation())))); > + > + Finder->addMatcher(fieldDecl(hasType(references(ConstString)), > + > unless(InTemplateInstantiation)).bind("member"), > + this); > +} > + > +void > +StringReferenceMemberCheck::check(const MatchFinder::MatchResult &Result) { > + const auto *Member = Result.Nodes.getNodeAs<FieldDecl>("member"); > + diag(Member->getLocStart(), "const string& members are dangerous. It is > much " > + "better to use alternatives, such as pointers > or " > + "simple constants."); > +} > + > +} // namespace runtime > +} // namespace tidy > +} // namespace clang > > Added: clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.h > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.h?rev=213133&view=auto > ============================================================================== > --- clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.h > (added) > +++ clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.h > Wed Jul 16 05:00:14 2014 > @@ -0,0 +1,50 @@ > +//===--- StringReferenceMemberCheck.h - clang-tidy ----------------*- C++ > -*-===// > +// > +// The LLVM Compiler Infrastructure > +// > +// This file is distributed under the University of Illinois Open Source > +// License. See LICENSE.TXT for details. > +// > +//===----------------------------------------------------------------------===// > + > +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_STRING_REF_MEMBER_CHECK_H > +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_STRING_REF_MEMBER_CHECK_H > + > +#include "../ClangTidy.h" > + > +namespace clang { > +namespace tidy { > +namespace runtime { > + > +/// \brief Finds members of type 'const string&'. > +/// > +/// const string reference members are generally considered unsafe as they > can > +/// be created from a temporary quite easily. > +/// > +/// \code > +/// struct S { > +/// S(const string &Str) : Str(Str) {} > +/// const string &Str; > +/// }; > +/// S instance("string"); > +/// \endcode > +/// > +/// In the constructor call a string temporary is created from const char * > and > +/// destroyed immediately after the call. This leaves around a dangling > +/// reference. > +/// > +/// This check emit warnings for both std::string and ::string const > reference > +/// members. > +/// > +/// Corresponding cpplint.py check name: 'runtime/member_string_reference'. > +class StringReferenceMemberCheck : public ClangTidyCheck { > +public: > + void registerMatchers(ast_matchers::MatchFinder *Finder) override; > + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; > +}; > + > +} // namespace runtime > +} // namespace tidy > +} // namespace clang > + > +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_STRING_REF_MEMBER_CHECK_H > > Added: > clang-tools-extra/trunk/test/clang-tidy/google-member-string-references.cpp > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-member-string-references.cpp?rev=213133&view=auto > ============================================================================== > --- > clang-tools-extra/trunk/test/clang-tidy/google-member-string-references.cpp > (added) > +++ > clang-tools-extra/trunk/test/clang-tidy/google-member-string-references.cpp > Wed Jul 16 05:00:14 2014 > @@ -0,0 +1,49 @@ > +// RUN: clang-tidy %s -checks='-*,google-runtime-member-string-references' > -- | FileCheck %s -implicit-check-not="{{warning|error}}:" > + > +namespace std { > +template<typename T> > + class basic_string {}; > + > +typedef basic_string<char> string; > +} > + > +class string {}; > + > + > +struct A { > + const std::string &s; > +// CHECK: :[[@LINE-1]]:3: warning: const string& members are dangerous. It > is much better to use alternatives, such as pointers or simple constants. > +}; > + > +struct B { > + std::string &s; > +}; > + > +struct C { > + const std::string s; > +}; > + > +template <typename T> > +struct D { > + D(); > + const T &s; > + const std::string &s2; > +// CHECK: :[[@LINE-1]]:3: warning: const string& members are dangerous. It > is much better to use alternatives, such as pointers or simple constants. > +}; > + > +D<std::string> d; > + > +struct AA { > + const string &s; > +// CHECK: :[[@LINE-1]]:3: warning: const string& members are dangerous. It > is much better to use alternatives, such as pointers or simple constants. > +}; > + > +struct BB { > + string &s; > +}; > + > +struct CC { > + const string s; > +}; > + > +D<string> dd; > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
