[clang-tools-extra] r329292 - [clang-tidy] Remove google-runtime-member-string-references

Benjamin Kramer via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 5 07:51:01 PDT 2018


Author: d0k
Date: Thu Apr  5 07:51:01 2018
New Revision: 329292

URL: http://llvm.org/viewvc/llvm-project?rev=329292&view=rev
Log:
[clang-tidy] Remove google-runtime-member-string-references

This is triggering on a pattern that's both too broad (const
std::string& members can be used safely) and too narrow (std::string is
not the only class with this problem). It has a very low true positive
rate, just remove it until we find a better solution for dangling string
references.

Removed:
    clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.cpp
    clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.h
    clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-member-string-references.rst
    clang-tools-extra/trunk/test/clang-tidy/google-runtime-member-string-references.cpp
Modified:
    clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/Resources/ClangTidyChecks.yaml
    clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt
    clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp
    clang-tools-extra/trunk/docs/ReleaseNotes.rst
    clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/Resources/ClangTidyChecks.yaml
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/Resources/ClangTidyChecks.yaml?rev=329292&r1=329291&r2=329292&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/Resources/ClangTidyChecks.yaml (original)
+++ clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/Resources/ClangTidyChecks.yaml Thu Apr  5 07:51:01 2018
@@ -115,10 +115,6 @@ Checks:
     Description: 
     Name:        google-runtime-int
   - Category:    Google Style Guide
-    Label:       Find const string references
-    Description: 
-    Name:        google-runtime-member-string-references
-  - Category:    Google Style Guide
     Label:       Find zero-length memsets
     Description: 
     Name:        google-runtime-memset

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=329292&r1=329291&r2=329292&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt Thu Apr  5 07:51:01 2018
@@ -12,7 +12,6 @@ add_clang_library(clangTidyGoogleModule
   IntegerTypesCheck.cpp
   NonConstReferences.cpp
   OverloadedUnaryAndCheck.cpp
-  StringReferenceMemberCheck.cpp
   TodoCommentCheck.cpp
   UnnamedNamespaceInHeaderCheck.cpp
   UsingNamespaceDirectiveCheck.cpp

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=329292&r1=329291&r2=329292&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp Thu Apr  5 07:51:01 2018
@@ -24,7 +24,6 @@
 #include "IntegerTypesCheck.h"
 #include "NonConstReferences.h"
 #include "OverloadedUnaryAndCheck.h"
-#include "StringReferenceMemberCheck.h"
 #include "TodoCommentCheck.h"
 #include "UnnamedNamespaceInHeaderCheck.h"
 #include "UsingNamespaceDirectiveCheck.h"
@@ -60,8 +59,6 @@ class GoogleModule : public ClangTidyMod
         "google-runtime-operator");
     CheckFactories.registerCheck<runtime::NonConstReferences>(
         "google-runtime-references");
-    CheckFactories.registerCheck<runtime::StringReferenceMemberCheck>(
-        "google-runtime-member-string-references");
     CheckFactories.registerCheck<readability::AvoidCStyleCastsCheck>(
         "google-readability-casting");
     CheckFactories.registerCheck<readability::TodoCommentCheck>(

Removed: 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=329291&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.cpp (removed)
@@ -1,51 +0,0 @@
-//===--- 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/AST/ASTContext.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/ASTMatchers/ASTMatchers.h"
-
-using namespace clang::ast_matchers;
-
-namespace clang {
-namespace tidy {
-namespace google {
-namespace runtime {
-
-void StringReferenceMemberCheck::registerMatchers(
-    ast_matchers::MatchFinder *Finder) {
-  // Only register the matchers for C++; the functionality currently does not
-  // provide any benefit to other languages, despite being benign.
-  if (!getLangOpts().CPlusPlus)
-    return;
-
-  // Look for const references to std::string or ::string.
-  auto String = anyOf(namedDecl(hasName("::std::string")),
-                      namedDecl(hasName("::string")));
-  auto ConstString = qualType(isConstQualified(), hasDeclaration(String));
-
-  // Ignore members in template instantiations.
-  Finder->addMatcher(
-      fieldDecl(hasType(references(ConstString)), unless(isInstantiated()))
-          .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 google
-} // namespace tidy
-} // namespace clang

Removed: 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=329291&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.h (removed)
@@ -1,54 +0,0 @@
-//===--- 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_STRINGREFERENCEMEMBERCHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_STRINGREFERENCEMEMBERCHECK_H
-
-#include "../ClangTidy.h"
-
-namespace clang {
-namespace tidy {
-namespace google {
-namespace runtime {
-
-/// 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:
-  StringReferenceMemberCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
-  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
-  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-};
-
-} // namespace runtime
-} // namespace google
-} // namespace tidy
-} // namespace clang
-
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_STRINGREFERENCEMEMBERCHECK_H

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=329292&r1=329291&r2=329292&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Thu Apr  5 07:51:01 2018
@@ -188,6 +188,8 @@ Improvements to clang-tidy
 - The 'misc-unused-raii' check was renamed to :doc:`bugprone-unused-raii
   <clang-tidy/checks/bugprone-unused-raii>`
 
+- The 'google-runtime-member-string-references' check was removed.
+
 Improvements to include-fixer
 -----------------------------
 

Removed: clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-member-string-references.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-member-string-references.rst?rev=329291&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-member-string-references.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-member-string-references.rst (removed)
@@ -1,25 +0,0 @@
-.. title:: clang-tidy - google-runtime-member-string-references
-
-google-runtime-member-string-references
-=======================================
-
-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-block:: c++
-
-  struct S {
-    S(const string &Str) : Str(Str) {}
-    const string &Str;
-  };
-  S instance("string");
-
-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`.

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=329292&r1=329291&r2=329292&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Thu Apr  5 07:51:01 2018
@@ -110,7 +110,6 @@ Clang-Tidy Checks
    google-readability-redundant-smartptr-get (redirects to readability-redundant-smartptr-get) <google-readability-redundant-smartptr-get>
    google-readability-todo
    google-runtime-int
-   google-runtime-member-string-references
    google-runtime-operator
    google-runtime-references
    hicpp-avoid-goto

Removed: clang-tools-extra/trunk/test/clang-tidy/google-runtime-member-string-references.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-runtime-member-string-references.cpp?rev=329291&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/google-runtime-member-string-references.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/google-runtime-member-string-references.cpp (removed)
@@ -1,49 +0,0 @@
-// RUN: %check_clang_tidy %s google-runtime-member-string-references %t
-
-namespace std {
-template<typename T>
-  class basic_string {};
-
-typedef basic_string<char> string;
-}
-
-class string {};
-
-
-struct A {
-  const std::string &s;
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: const string& members are dangerous; it is much better to use alternatives, such as pointers or simple constants [google-runtime-member-string-references]
-};
-
-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-MESSAGES: :[[@LINE-1]]:3: warning: const string& members are dangerous
-};
-
-D<std::string> d;
-
-struct AA {
-  const string &s;
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: const string& members are dangerous
-};
-
-struct BB {
-  string &s;
-};
-
-struct CC {
-  const string s;
-};
-
-D<string> dd;




More information about the cfe-commits mailing list