[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 160112.
hugoeg marked 10 inline comments as done.
hugoeg added a comment.

Applied corrections from first round comments


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,62 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1){
+  return s1;
+}
+class SomeContainer{};
+
+namespace strings_internal{
+  
+  void InternalFunction(){}
+
+  template 
+  P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal{
+  struct InternalStruct{};
+
+} // namespace container_internal
+} // namespace absl
+
+void foo(){
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+
+  absl::strings_internal::InternalTemplateFunction ("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+}
+
+class foo2{
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+};
+
+namespace absl{
+  void foo3(){
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+  }
+} // namespace absl
+
+
+
+// should not trigger warnings
+void foo4(){
+  std::string str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+  SomeContainer b;
+  std::string str = absl::StringsFunction("a");
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-string-find-startswith
+   abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,16 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Warns users if they attempt to depend on internal details. If something is in a namespace or filename/path that includes the word “internal”, users are not allowed to depend upon it beaucse it’s an implementation detail. They cannot friend it, include it, you mention it or refer to it in any way. Doing so violtaes Abseil's compatibility guidelines and may result in breakage.
+
+The folowing cases will result in warnings:
+
+.. code-block:: c++
+
+absl::strings_internal::foo();
+class foo{
+  friend struct absl::container_internal::faa;
+};
+absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
 Improvements to clang-rename
 
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Warns Abseil users if they attempt to depend on internal details.
 
 Improvements to clang-tidy
 --
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.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_ABSEIL_NOINTERNALDEPSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_C

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added inline comments.



Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:24
+
+  Finder->addMatcher(
+  nestedNameSpecifierLoc(loc(specifiesNamespace(namespaceDecl(

JonasToth wrote:
> Actually that one is generally useful. Accessing the `foo::internal` from 
> outside of `foo` is always a problem. Maybe this matcher can become 
> configurable or just match on any `internal` access from outside the 
> enclosing namespace.
That's a good idea. While we agree, right now our efforts are focused on 
releasing abseil specific functions. Perhaps we can refactor this check at a 
later time. 


https://reviews.llvm.org/D50542



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 160124.

https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,62 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1){
+  return s1;
+}
+class SomeContainer{};
+
+namespace strings_internal{
+  
+  void InternalFunction(){}
+
+  template 
+  P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal{
+  struct InternalStruct{};
+
+} // namespace container_internal
+} // namespace absl
+
+void foo(){
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+
+  absl::strings_internal::InternalTemplateFunction ("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+}
+
+class foo2{
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+};
+
+namespace absl{
+  void foo3(){
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+  }
+} // namespace absl
+
+
+
+// should not trigger warnings
+void foo4(){
+  std::string str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+  SomeContainer b;
+  std::string str = absl::StringsFunction("a");
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-string-find-startswith
+   abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,16 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Warns users if they attempt to depend on internal details. If something is in a namespace or filename/path that includes the word “internal”, users are not allowed to depend upon it beaucse it’s an implementation detail. They cannot friend it, include it, you mention it or refer to it in any way. Doing so violtaes Abseil's compatibility guidelines and may result in breakage.
+
+The folowing cases will result in warnings:
+
+.. code-block:: c++
+
+absl::strings_internal::foo();
+class foo{
+  friend struct absl::container_internal::faa;
+};
+absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
 Improvements to clang-rename
 
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Warns Abseil users if they attempt to depend on internal details.
 
 Improvements to clang-tidy
 --
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.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_ABSEIL_NOINTERNALDEPSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namesp

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 160168.
hugoeg added a comment.

corrections from comments applied


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,62 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1){
+  return s1;
+}
+class SomeContainer{
+
+};
+
+namespace strings_internal{
+  
+  void InternalFunction(){}
+
+  template 
+  P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal{
+  struct InternalStruct{};
+
+} // namespace container_internal
+} // namespace absl
+
+void foo(){
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+
+  absl::strings_internal::InternalTemplateFunction ("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+}
+
+class foo2{
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+};
+
+namespace absl{
+  void foo3(){
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+  }
+} // namespace absl
+
+// should not trigger warnings
+void foo4(){
+  std::string str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+  SomeContainer b;
+  std::string str = absl::StringsFunction("a");
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-string-find-startswith
+   abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,16 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something is in a namespace or filename/path that includes the word “internal”, code is not allowed to depend upon it beaucse it’s an implementation detail. They cannot friend it, include it, you mention it or refer to it in any way. Doing so violtaes Abseil's compatibility guidelines and may result in breakage.
+
+The folowing cases will result in warnings:
+
+.. code-block:: c++
+
+absl::strings_internal::foo();
+class foo{
+  friend struct absl::container_internal::faa;
+};
+absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
 Improvements to clang-rename
 
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Gives a warning if code using Abseil depends on internal details.
 
 Improvements to clang-tidy
 --
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.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_ABSEIL_NOINTERNALDEPSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+
+#includ

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg marked 8 inline comments as done.
hugoeg added inline comments.



Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:2
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+

hokein wrote:
> nit: please make sure the code follow LLVM code style, even for test code :)
what is this in reference too?
Will the test still work if I wrap the CHECK MESSAGE lines?



Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:11
+
+namespace absl {
+std::string StringsFunction (std::string s1){

hokein wrote:
> Since we have multiple abseil checks that might use these fake abseil 
> declarations, I'd suggest pull out these to a common header, and include it 
> in this test file.
do I just put the header file  in test/clang-tidy ?


https://reviews.llvm.org/D50542



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 160363.
hugoeg added a comment.

most corrections from comments have been applied


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,62 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1){
+  return s1;
+}
+class SomeContainer{
+
+};
+
+namespace strings_internal{
+  
+  void InternalFunction(){}
+
+  template 
+  P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal{
+  struct InternalStruct{};
+
+} // namespace container_internal
+} // namespace absl
+
+void foo(){
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction ("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class foo2{
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl{
+  void foo3(){
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+  }
+} // namespace absl
+
+// should not trigger warnings
+void foo4(){
+  std::string str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+  SomeContainer b;
+  std::string str = absl::StringsFunction("a");
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-string-find-startswith
+   abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,21 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to 
+depend upon it beaucse it’s an implementation detail. They cannot friend it, 
+include it, you mention it or refer to it in any way. Doing so violates 
+Abseil's compatibility guidelines and may result in breakage. See 
+https://abseil.io/about/compatibility for more information.
+
+The following cases will result in warnings:
+
+.. code-block:: c++
+
+absl::strings_internal::foo();
+class foo{
+  friend struct absl::container_internal::faa;
+};
+absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
 Improvements to clang-rename
 
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Gives a warning if code using Abseil depends on internal details.
 
 Improvements to clang-tidy
 --
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.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_ABSEIL_NOINTERNALDEPSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Finds instances

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 160371.

https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,62 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1){
+  return s1;
+}
+class SomeContainer{
+
+};
+
+namespace strings_internal{
+  
+  void InternalFunction(){}
+
+  template 
+  P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal{
+  struct InternalStruct{};
+
+} // namespace container_internal
+} // namespace absl
+
+void foo(){
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction ("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class foo2{
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl{
+  void foo3(){
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+  }
+} // namespace absl
+
+// should not trigger warnings
+void foo4(){
+  std::string str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+  SomeContainer b;
+  std::string str = absl::StringsFunction("a");
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-string-find-startswith
+   abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,21 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to 
+depend upon it beaucse it’s an implementation detail. They cannot friend it, 
+include it, you mention it or refer to it in any way. Doing so violates 
+Abseil's compatibility guidelines and may result in breakage. See 
+https://abseil.io/about/compatibility for more information.
+
+The following cases will result in warnings:
+
+.. code-block:: c++
+
+absl::strings_internal::foo();
+class foo{
+  friend struct absl::container_internal::faa;
+};
+absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
 Improvements to clang-rename
 
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Gives a warning if code using Abseil depends on internal details.
 
 Improvements to clang-tidy
 --
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.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_ABSEIL_NOINTERNALDEPSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Finds instances where the user depends on internal details and warns them
+/// against doi

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added inline comments.



Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:2
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+

JonasToth wrote:
> hugoeg wrote:
> > hokein wrote:
> > > nit: please make sure the code follow LLVM code style, even for test code 
> > > :)
> > what is this in reference too?
> > Will the test still work if I wrap the CHECK MESSAGE lines?
> CHECK-MESSAGE can be on one line, even if its longer (that is common in the 
> clang-tidy tests).
> 
> But dont use many empty lines and respect naming conventions and run 
> clang-format over the code (except there is a valid reason that the 
> formatting would infer with the tested logic).
Do my function names have to be verbs, they're not doing anything.

I could rename InternalFunction to something like InternallyProcessString annd 
StringFunction to process String
Would this be preferred?


https://reviews.llvm.org/D50542



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 160380.
hugoeg marked 3 inline comments as done.

https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-fake-declarations.h
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,34 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+#include "abseil-fake-declarations.h"
+
+void foo(){
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction ("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class foo2{
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl{
+  void foo3(){
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+  }
+} // namespace absl
+
+// should not trigger warnings
+void foo4(){
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+  SomeContainer b;
+  std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/abseil-fake-declarations.h
===
--- test/clang-tidy/abseil-fake-declarations.h
+++ test/clang-tidy/abseil-fake-declarations.h
@@ -0,0 +1,24 @@
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1){
+  return s1;
+}
+class SomeContainer{
+
+};
+namespace strings_internal{
+  void InternalFunction(){}
+  template 
+  P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal{
+  struct InternalStruct{};
+} // namespace container_internal
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-string-find-startswith
+   abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,21 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to 
+depend upon it beaucse it’s an implementation detail. They cannot friend it, 
+include it, you mention it or refer to it in any way. Doing so violates 
+Abseil's compatibility guidelines and may result in breakage. See 
+https://abseil.io/about/compatibility for more information.
+
+The following cases will result in warnings:
+
+.. code-block:: c++
+
+  absl::strings_internal::foo();
+  class foo{
+friend struct absl::container_internal::faa;
+  };
+  absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
 Improvements to clang-rename
 
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Gives a warning if code using Abseil depends on internal details.
 
 Improvements to clang-tidy
 --
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.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.
+//
+//===---

[PATCH] D50580: [clang-tidy] Abseil: no namespace check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added inline comments.



Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23
+
+  Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);

deannagarcia wrote:
> aaron.ballman wrote:
> > hokein wrote:
> > > aaron.ballman wrote:
> > > > I think this needs a `not(isExpansionInSystemHeader())` in there, as 
> > > > this is going to trigger on code that includes a header file using an 
> > > > `absl` namespace. If I'm incorrect and users typically include abseil 
> > > > as something other than system includes, you'll have to find a 
> > > > different way to solve this.
> > > Yeah, we have discussed this issue internally.  abseil-user code usually 
> > > includes abseil header like `#include 
> > > "whatever/abseil/base/optimization.h"`, so `IsExpansionInSystemHeader` 
> > > doesn't work for most cases. 
> > > 
> > > We need a way to filter out all headers being a part of abseil library. I 
> > > think we can create a matcher `InExpansionInAbseilHeader` -- basically 
> > > using `IsExpansionInFileMatching` with a regex expression that matches 
> > > typical abseil include path. What do you think?
> > > 
> > > And we'll have more abseil checks (e.g.  `abseil-no-internal-deps`) that 
> > > use `InExpansionInAbseilHeader`, we should put it to a common header.
> > I think that is a sensible approach.
> We will begin working on this, I think it will first be implemented in 
> abseil-no-internal-deps but once it looks good I will add it to this patch.
I'm going to go ahead a implement this in ASTMatchers.h and include it on the 
patch for **abseil-no-internal-dep**s


https://reviews.llvm.org/D50580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 160432.
hugoeg marked an inline comment as done.
hugoeg added a comment.

applied corrections from comments


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-fake-declarations.h
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,34 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+#include "abseil-fake-declarations.h"
+
+void DirectAcess(){
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction ("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage{
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl{
+  void OpeningNamespace(){
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+  }
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage(){
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+  SomeContainer b;
+  std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/abseil-fake-declarations.h
===
--- test/clang-tidy/abseil-fake-declarations.h
+++ test/clang-tidy/abseil-fake-declarations.h
@@ -0,0 +1,24 @@
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1){
+  return s1;
+}
+class SomeContainer{
+
+};
+namespace strings_internal{
+  void InternalFunction(){}
+  template 
+  P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal{
+  struct InternalStruct{};
+} // namespace container_internal
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-string-find-startswith
+   abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,21 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to 
+depend upon it beaucse it’s an implementation detail. They cannot friend it, 
+include it, you mention it or refer to it in any way. Doing so violates 
+Abseil's compatibility guidelines and may result in breakage. See 
+https://abseil.io/about/compatibility for more information.
+
+The following cases will result in warnings:
+
+.. code-block:: c++
+
+  absl::strings_internal::foo();
+  class foo{
+friend struct absl::container_internal::faa;
+  };
+  absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
 Improvements to clang-rename
 
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Gives a warning if code using Abseil depends on internal details.
 
 Improvements to clang-tidy
 --
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License.

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 160562.
hugoeg marked an inline comment as done.

https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-fake-declarations.h
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,34 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+#include "abseil-fake-declarations.h"
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction ("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+  void OpeningNamespace() {
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+  }
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+  SomeContainer b;
+  std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/abseil-fake-declarations.h
===
--- test/clang-tidy/abseil-fake-declarations.h
+++ test/clang-tidy/abseil-fake-declarations.h
@@ -0,0 +1,24 @@
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1) {
+  return s1;
+}
+class SomeContainer {
+
+};
+namespace strings_internal {
+  void InternalFunction(){}
+  template 
+  P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+  struct InternalStruct{};
+} // namespace container_internal
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-string-find-startswith
+   abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,21 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to 
+depend upon it beaucse it’s an implementation detail. They cannot friend it, 
+include it, you mention it or refer to it in any way. Doing so violates 
+Abseil's compatibility guidelines and may result in breakage. See 
+https://abseil.io/about/compatibility for more information.
+
+The following cases will result in warnings:
+
+.. code-block:: c++
+
+  absl::strings_internal::foo();
+  class foo{
+friend struct absl::container_internal::faa;
+  };
+  absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
 Improvements to clang-rename
 
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Gives a warning if code using Abseil depends on internal details.
 
 Improvements to clang-tidy
 --
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.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.
+//
+//===---

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added inline comments.



Comment at: test/clang-tidy/abseil-fake-declarations.h:1
+namespace std {
+struct string {

hokein wrote:
> I'd expect this header file is used as as a real absl library file:
> 
> * create an `absl` directory in `test/clang-tidy/Inputs/`, and this directory 
> is the abseil source root directory
> * use the real absl function/class names instead of some fake names in the 
> header, e.g. this file could be 
> `test/clang-tidy/Inputs/absl/strings/str_cat.h`.
> 
> This would make the lit test align with the real world case, and it could be 
> helpful if you implement the `IsExpansionInAbseilHeader` matcher in this 
> check. 
My mentors on the absl team told me to refrain from using real absl 
function/class names as I did originally. They would prefer I use fake names. 
I'd be happy to update the filename something real like str_cat.h though

I have been implementing IsExpansionInHeader for the past couple days and I 
hope to have it included on this patch by the end of the week. 



Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:19
+namespace absl {
+  void OpeningNamespace() {
+strings_internal::InternalFunction();

hokein wrote:
> the style doesn't looks correct to me.
This is the style given from clang-format


https://reviews.llvm.org/D50542



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50852: abseil-auto-make-unique

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg created this revision.
hugoeg added reviewers: alexfh, hokein.
hugoeg added a project: clang-tools-extra.
Herald added a subscriber: mgorny.

warns to use 'auto' to avoid repeating the type name and fixes the issue

Replace:
 std::unique_ptr x = make_unique(...);
with:

  auto x = make_unique(...);


https://reviews.llvm.org/D50852

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/AutoMakeUniqueCheck.cpp
  clang-tidy/abseil/AutoMakeUniqueCheck.h
  clang-tidy/abseil/CMakeLists.txt
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-auto-make-unique.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-auto-make-unique.cpp

Index: test/clang-tidy/abseil-auto-make-unique.cpp
===
--- test/clang-tidy/abseil-auto-make-unique.cpp
+++ test/clang-tidy/abseil-auto-make-unique.cpp
@@ -0,0 +1,115 @@
+// RUN: %check_clang_tidy %s abseil-auto-make-unique %t
+
+namespace std {
+template 
+struct default_delete {};
+
+template >
+class unique_ptr {
+ public:
+  unique_ptr();
+  ~unique_ptr();
+  explicit unique_ptr(T*);
+  template 
+  unique_ptr(unique_ptr&&);
+};
+template 
+unique_ptr make_unique(Args&&...);
+}  // namespace std
+
+namespace absl {
+template 
+std::unique_ptr MakeUnique(Args&&...);
+template 
+std::unique_ptr make_unique(Args&&...);
+}  // namespace absl
+using absl::make_unique;
+
+typedef int integer;
+
+struct Base {};
+struct Derived : public Base {};
+
+void Primitive() {
+  std::unique_ptr x = absl::make_unique();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name
+  // CHECK-FIXES: auto x = absl::make_unique();
+
+  std::unique_ptr< int >y = absl::make_unique();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name
+  // CHECK-FIXES: auto y = absl::make_unique();
+
+  const std::unique_ptr z = absl::make_unique();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use 'auto' to avoid repeating the type name
+  // CHECK-FIXES: const auto z = absl::make_unique();
+
+  std::unique_ptr t = absl::make_unique();
+}
+
+void Typedefs() {
+  std::unique_ptr x = absl::make_unique();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name
+  // CHECK-FIXES: auto x = absl::make_unique();
+
+  std::unique_ptr y = absl::make_unique();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name
+  // CHECK-FIXES: auto y = absl::make_unique();
+
+  std::unique_ptr z = absl::make_unique();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name
+  // CHECK-FIXES: auto z = absl::make_unique();
+}
+
+void Class() {
+  std::unique_ptr base = make_unique();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name
+  // CHECK-FIXES: auto base = make_unique();
+
+  std::unique_ptr base2(make_unique());
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name
+  // CHECK-FIXES: auto base2(make_unique());
+
+  // Different type. No change.
+  std::unique_ptr z = make_unique();
+  std::unique_ptr z2(make_unique());
+}
+
+template 
+void f() {
+  std::unique_ptr x = make_unique();
+}
+
+void Negatives() {
+  // Different deleter. No change.
+  struct MyDeleter {};
+  std::unique_ptr z3 = make_unique();
+  std::unique_ptr z4(make_unique());
+
+  f();
+}
+
+std::unique_ptr global_var = make_unique();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'auto' to avoid repeating the type name
+// CHECK-FIXES: auto global_var = make_unique();
+
+struct Struct {
+  static std::unique_ptr static_field;
+};
+// This code with "auto" replaced doesn't compile in GCC.
+std::unique_ptr Struct::static_field = make_unique();
+
+void FunctionWithStatic() {
+  static std::unique_ptr static_var = make_unique();
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'auto' to avoid repeating the type name
+  // CHECK-FIXES: auto static_var = make_unique();
+}
+
+void OtherNames() {
+  std::unique_ptr x = absl::MakeUnique();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name
+  // CHECK-FIXES: auto x = absl::MakeUnique();
+
+  std::unique_ptr y = std::make_unique();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'auto' to avoid repeating the type name
+  // CHECK-FIXES: auto y = std::make_unique();
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
 =
 
 .. toctree::
+   abseil-auto-make-unique
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-auto-make-unique.rst
===
--- docs/clang-tidy/checks/abseil-auto-make-unique.rst
+++ docs/clang-tidy/checks/abseil-auto-make-un

[PATCH] D50852: abseil-auto-make-unique

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

what do you mean by "with full context"?


https://reviews.llvm.org/D50852



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50852: abseil-auto-make-unique

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

In https://reviews.llvm.org/D50852#1202774, @lebedev.ri wrote:

> 1. Please always upload all patches with full context.
> 2. There already is `modernize-use-auto`. Does it handle this case? Then this 
> should be just an alias to that check. Else, i think it would be best to 
> extend that one, and still alias.


since this check checks for absl::make_unique primarily 
if we move it to the general check, it'd be weird to check for absl::make_unique
right now our main goal is to release checks tailored specifically to abseil 
users, so if we can we would like to release this check separately in the 
abseil directory


https://reviews.llvm.org/D50852



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 161130.
hugoeg added a comment.

added IsInAbseilFile Matcher


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/strings/str_cat.h
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+#include "Inputs/absl/strings/str_cat.h"
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction ("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+i hate this
+namespace absl {
+  void OpeningNamespace() {
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+  }
+} // namespace absl
+
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+  SomeContainer b;
+  std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/str_cat.h
===
--- test/clang-tidy/Inputs/absl/strings/str_cat.h
+++ test/clang-tidy/Inputs/absl/strings/str_cat.h
@@ -0,0 +1,33 @@
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
 =
 
 .. toctree::
+   abseil-no-internal-deps
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,21 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to 
+depend upon it beaucse it’s an implementation detail. They cannot friend it, 
+include it, you mention it or refer to it in any way. Doing so violates 
+Abseil's compatibility guidelines and may result in breakage. See 
+https://abseil.io/about/compatibility for more information.
+
+The following cases will result in warnings:
+
+.. code-block:: c++
+
+  absl::strings_internal::foo();
+  class foo{
+friend struct absl::container_internal::faa;
+  };
+  absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,7 +57,10 @@
 Improvements to clang-tidy
 --
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Gives a warning if code using Abseil depends on internal details.
 
 Improvements to include-fixer
 -
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 161263.
hugoeg marked 5 inline comments as done.
hugoeg added a comment.

applied corrections form comments


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/strings/str_cat.h
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+#include "Inputs/absl/strings/str_cat.h"
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction ("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+i hate this
+namespace absl {
+  void OpeningNamespace() {
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+  }
+} // namespace absl
+
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+  SomeContainer b;
+  std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/str_cat.h
===
--- test/clang-tidy/Inputs/absl/strings/str_cat.h
+++ test/clang-tidy/Inputs/absl/strings/str_cat.h
@@ -0,0 +1,33 @@
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
 =
 
 .. toctree::
+   abseil-no-internal-deps
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,21 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to 
+depend upon it beaucse it’s an implementation detail. They cannot friend it, 
+include it, you mention it or refer to it in any way. Doing so violates 
+Abseil's compatibility guidelines and may result in breakage. See 
+https://abseil.io/about/compatibility for more information.
+
+The following cases will result in warnings:
+
+.. code-block:: c++
+
+  absl::strings_internal::foo();
+  class foo{
+friend struct absl::container_internal::faa;
+  };
+  absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,7 +57,10 @@
 Improvements to clang-tidy
 --
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Gives a warning if code using Abseil depends on internal details.
 
 Improvements to include-fixer
 -
Index: clang-tidy/abseil/NoInternalDepsCheck.h
=

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added inline comments.



Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:20
+
+bool IsInAbseilFile(const SourceManager& manager, SourceLocation loc){
+  if (loc.isInvalid()) {

hokein wrote:
> I think we can make it as an ASTMatcher instead of a function like:
> 
> ```
> AST_POLYMORPHIC_MATCHER_P(isInAbseilFile,
>   AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, 
> TypeLoc)) {
>// your code here.
> }
> ```
Unfortunately, I do not think we can.  That was the way I originally tried to 
implement it. It worked on no-namespace-check, but not in this one. This is 
because as an AST_POLYMORPHIC_MATCHER_P, we are trying to match an usage of a 
Decl, not the Decl itself and since we are matching a TypeLoc in 
no-internal-deps-check, not really the usage, it doesn't work.

As a result, I modified my original implementation, which you will see in 
no-namespace-check and turned it into IsInAbseilFile(SourceManager&,  
SourceLocation), which is just takes a source location and returns whether the 
location we matched is in an Abseil file


https://reviews.llvm.org/D50542



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

Anymore changes I should make or issues I should be aware of?


https://reviews.llvm.org/D50542



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 161485.
hugoeg added a comment.

fixed some minor things in the test


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/strings/str_cat.h
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t,  -- -- -I %S/Inputs/absl/strings
+
+#include "str_cat.h"
+
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+void OpeningNamespace() {
+  strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+SomeContainer b;
+std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/str_cat.h
===
--- test/clang-tidy/Inputs/absl/strings/str_cat.h
+++ test/clang-tidy/Inputs/absl/strings/str_cat.h
@@ -0,0 +1,33 @@
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
 =
 
 .. toctree::
+   abseil-no-internal-deps
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,21 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to 
+depend upon it beaucse it’s an implementation detail. They cannot friend it, 
+include it, you mention it or refer to it in any way. Doing so violates 
+Abseil's compatibility guidelines and may result in breakage. See 
+https://abseil.io/about/compatibility for more information.
+
+The following cases will result in warnings:
+
+.. code-block:: c++
+
+  absl::strings_internal::foo();
+  class foo{
+friend struct absl::container_internal::faa;
+  };
+  absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,7 +57,10 @@
 Improvements to clang-tidy
 --
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Gives a warning if code using Abseil depends on internal details.
 
 Improvements to include-fixer
 -
Index: clang-tidy/abseil/NoInternalDepsCheck.h

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 161494.
hugoeg marked 5 inline comments as done.
hugoeg added a comment.

corrections applied


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/strings/str_cat.h
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,34 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t,  -- -- -I %S/Inputs
+
+#include "absl/strings/str_cat.h"
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+void OpeningNamespace() {
+  strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+SomeContainer b;
+std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/str_cat.h
===
--- test/clang-tidy/Inputs/absl/strings/str_cat.h
+++ test/clang-tidy/Inputs/absl/strings/str_cat.h
@@ -0,0 +1,33 @@
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
 =
 
 .. toctree::
+   abseil-no-internal-deps
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,23 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to 
+depend upon it beaucse it’s an implementation detail. They cannot friend it, 
+include it, you mention it or refer to it in any way. Doing so violates 
+Abseil's compatibility guidelines and may result in breakage. See 
+https://abseil.io/about/compatibility for more information.
+
+The following cases will result in warnings:
+
+.. code-block:: c++
+
+  absl::strings_internal::foo();
+  class foo{
+friend struct absl::container_internal::faa;
+// warning triggered on this line
+  };
+  absl::memory_internal::MakeUniqueResult();
+  // warning triggered on the line 
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,7 +57,10 @@
 Improvements to clang-tidy
 --
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Gives a warning if code using Abseil depends on internal details.
 
 Improvements to include-fixer
 --

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg marked an inline comment as done.
hugoeg added inline comments.



Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:20
+
+bool IsInAbseilFile(const SourceManager& manager, SourceLocation loc){
+  if (loc.isInvalid()) {

hokein wrote:
> hugoeg wrote:
> > hokein wrote:
> > > I think we can make it as an ASTMatcher instead of a function like:
> > > 
> > > ```
> > > AST_POLYMORPHIC_MATCHER_P(isInAbseilFile,
> > >   AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, 
> > > TypeLoc)) {
> > >// your code here.
> > > }
> > > ```
> > Unfortunately, I do not think we can.  That was the way I originally tried 
> > to implement it. It worked on no-namespace-check, but not in this one. This 
> > is because as an AST_POLYMORPHIC_MATCHER_P, we are trying to match an usage 
> > of a Decl, not the Decl itself and since we are matching a TypeLoc in 
> > no-internal-deps-check, not really the usage, it doesn't work.
> > 
> > As a result, I modified my original implementation, which you will see in 
> > no-namespace-check and turned it into IsInAbseilFile(SourceManager&,  
> > SourceLocation), which is just takes a source location and returns whether 
> > the location we matched is in an Abseil file
> Could you explain a bit more why it won't work? I don't understand why it 
> doesn't work. 
> 
> Basically you define the matcher like:
> 
> ```
> AST_POLYMORPHIC_MATCHER(
> isInAbseilFile, AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc,
> NestedNameSpecifierLoc)) {
>   auto &SourceManager = Finder->getASTContext().getSourceManager();
>   SourceLocation loc = Node.getBeginLoc();
>   if (loc.isInvalid()) return false;
>   const FileEntry *FileEntry =
>   SourceManager.getFileEntryForID(SourceManager.getFileID(loc));
>   if (!FileEntry) return false;
>   StringRef Filename = FileEntry->getName();
>   llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
>  "synchronization|types|utiliy)");
>   return RE.match(Filename);
> }
> ```
> 
> And use it for example in your check
> 
> ```
> Finder->addMatcher(nestedNameSpecifierLoc(
>  loc(specifiesNamespace(namespaceDecl(
>  matchesName("internal"),
>  hasParent(namespaceDecl(hasName("absl")),
>  unless(isInAbseilFile()))
>  .bind("InternalDep"),
>  this);
> ```
You're right, this implementation seems to work, I seemed to have a simple 
logic error in my original implementation, the new diff will include this 
version 


https://reviews.llvm.org/D50542



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 161550.
hugoeg added a comment.

refined test to include some more cases as a result of the new matcher


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/external-file.h
  test/clang-tidy/Inputs/absl/strings/internal-file.h
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t,  -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-internal-deps' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:1:23: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil [abseil-no-internal-deps]
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+void OpeningNamespace() {
+  strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+SomeContainer b;
+std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/internal-file.h
===
--- test/clang-tidy/Inputs/absl/strings/internal-file.h
+++ test/clang-tidy/Inputs/absl/strings/internal-file.h
@@ -0,0 +1,33 @@
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/external-file.h
===
--- test/clang-tidy/Inputs/absl/external-file.h
+++ test/clang-tidy/Inputs/absl/external-file.h
@@ -0,0 +1 @@
+void DirectAcess2() { absl::strings_internal::InternalFunction(); }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
 =
 
 .. toctree::
+   abseil-no-internal-deps
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,23 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to 
+depend upon it beaucse it’s an implementation detail. They cannot friend it, 
+include it, you mention it or refer to it in any way. Doing so violates 
+Abseil's compatibility guidelines and may result in breakage. See 
+https://abseil.io/about/compatibility for m

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 161714.
hugoeg marked an inline comment as done.
hugoeg added a comment.

All comments have been address and documentation has been added to matcher


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/external-file.h
  test/clang-tidy/Inputs/absl/strings/internal-file.h
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t,  -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-internal-deps' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:1:23: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil [abseil-no-internal-deps]
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+void OpeningNamespace() {
+  strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+SomeContainer b;
+std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/internal-file.h
===
--- test/clang-tidy/Inputs/absl/strings/internal-file.h
+++ test/clang-tidy/Inputs/absl/strings/internal-file.h
@@ -0,0 +1,33 @@
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/external-file.h
===
--- test/clang-tidy/Inputs/absl/external-file.h
+++ test/clang-tidy/Inputs/absl/external-file.h
@@ -0,0 +1 @@
+void DirectAcess2() { absl::strings_internal::InternalFunction(); }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
 =
 
 .. toctree::
+   abseil-no-internal-deps
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,23 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Warns if code using Abseil depends on internal details. If something is in a
+namespace that includes the word “internal”, code is not allowed to depend upon
+it beaucse it’s an implementation detail. They cannot friend it, include it,
+you mention it or refer to it in any way. Doing so violates Abseil's
+compatibility guidelines and may result in breakage. See
+https://abse

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

Ok , so I've clean some stuff up, I'm currently working with @deannagarcia to 
get https://reviews.llvm.org/D50580 submitted with the match I implemented for 
us first, then will adjust this as necessary


https://reviews.llvm.org/D50542



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50580: [clang-tidy] Abseil: no namespace check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added inline comments.



Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+  auto Filename = FileEntry->getName();
+  llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+ "synchronization|types|utiliy)");

deannagarcia wrote:
> deannagarcia wrote:
> > lebedev.ri wrote:
> > > hokein wrote:
> > > > lebedev.ri wrote:
> > > > > hokein wrote:
> > > > > > The regex seems incomplete, we are missing `algorithm`, `time` 
> > > > > > subdirectory. 
> > > > > So what happens if open the namespace in a file that is located in my 
> > > > > personal `absl/base` directory?
> > > > > It will be false-negatively not reported?
> > > >  Yes, I'd say this is a known limitation.
> > > Similarly, the check will break (start producing false-positives) as soon 
> > > as a new directory is added to abseil proper.
> > > I don't have any reliable idea on how to do this better, but the current 
> > > solution seems rather suboptimal..
> > We are aware that there will be a false-negative if code opens namespace in 
> > the absl directories, however we think that is pretty rare and that users 
> > shouldn't be doing that anyway. No matter how we do this there will be a 
> > way for users to circumvent the check, and we will allow those users to do 
> > it because it will be their code that breaks in the end.
> The false-positive with new directories is something we thought about, but 
> new directories aren't added to absl very often so we thought it wouldn't be 
> too hard to add them to this regex when they are.
Members of the Abseil team can also update this check as new directories are 
release


https://reviews.llvm.org/D50580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 161970.
hugoeg marked 11 inline comments as done.
hugoeg added a comment.

applied corrections from comments


https://reviews.llvm.org/D51061

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/StrCatAppendCheck.cpp
  clang-tidy/abseil/StrCatAppendCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-str-cat-append.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-str-cat-append.cpp

Index: test/clang-tidy/abseil-str-cat-append.cpp
===
--- test/clang-tidy/abseil-str-cat-append.cpp
+++ test/clang-tidy/abseil-str-cat-append.cpp
@@ -0,0 +1,129 @@
+// RUN: %check_clang_tidy %s abseil-str-cat-append %t -- -- -I%S -std=c++11
+
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+typedef __SIZE_TYPE__ size;
+
+namespace std {
+template 
+class allocator {};
+template 
+class char_traits {};
+template 
+struct basic_string {
+  typedef basic_string _Type;
+  basic_string();
+  basic_string(const C* p, const A& a = A());
+
+  const C* c_str() const;
+  const C* data() const;
+
+  _Type& append(const C* s);
+  _Type& append(const C* s, size n);
+  _Type& assign(const C* s);
+  _Type& assign(const C* s, size n);
+
+  int compare(const _Type&) const;
+  int compare(const C* s) const;
+  int compare(size pos, size len, const _Type&) const;
+  int compare(size pos, size len, const C* s) const;
+
+  size find(const _Type& str, size pos = 0) const;
+  size find(const C* s, size pos = 0) const;
+  size find(const C* s, size pos, size n) const;
+
+  _Type& insert(size pos, const _Type& str);
+  _Type& insert(size pos, const C* s);
+  _Type& insert(size pos, const C* s, size n);
+
+  _Type& operator+=(const _Type& str);
+  _Type& operator+=(const C* s);
+  _Type& operator=(const _Type& str);
+  _Type& operator=(const C* s);
+};
+
+typedef basic_string, std::allocator> string;
+typedef basic_string,
+ std::allocator>
+wstring;
+typedef basic_string, std::allocator>
+u16string;
+typedef basic_string, std::allocator>
+u32string;
+}  // namespace std
+
+std::string operator+(const std::string&, const std::string&);
+std::string operator+(const std::string&, const char*);
+std::string operator+(const char*, const std::string&);
+
+bool operator==(const std::string&, const std::string&);
+bool operator==(const std::string&, const char*);
+bool operator==(const char*, const std::string&);
+
+namespace llvm {
+struct StringRef {
+  StringRef(const char* p);
+  StringRef(const std::string&);
+};
+}  // namespace llvm
+
+namespace absl {
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char* c_str);
+  AlphaNum(const std::string& str);
+
+ private:
+  AlphaNum(const AlphaNum&);
+  AlphaNum& operator=(const AlphaNum&);
+};
+
+std::string StrCat(const AlphaNum& a);
+std::string StrCat(const AlphaNum& a, const AlphaNum& b);
+
+template 
+void foo(A& a) {
+  a = StrCat(a);
+}
+
+void bar() {
+  std::string a, b;
+  foo(a);
+
+  std::string c = StrCat(a);
+  a = StrCat(a);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call to absl::StrCat does nothing
+  a = StrCat(a, b);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&a, b);
+  b = StrCat(a, b);
+
+#define M(x) x = StrCat(x, a)
+  M(b);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: #define M(x) x = StrCat(x, a)
+}
+
+void Regression_SelfAppend() {
+  std::string a;
+  a = StrCat(a, a);
+}
+
+}  // namespace absl
+
+void OutsideAbsl() {
+  std::string a, b;
+  a = absl::StrCat(a, b);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&a, b);
+}
+
+void OutisdeUsingAbsl() {
+  std::string a, b;
+  using absl::StrCat;
+  a = StrCat(a, b);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&a, b);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-duration-division
+   abseil-str-cat-append
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-str-cat-append.rst
===
--- docs/clang-tidy/checks/abseil-str-cat-append.rst
+++ docs/clang-tidy/checks/abseil-str-cat-append.rst
@@ -0,0 +1,11 @@
+.. title:: clang-tidy - abseil-str-cat-append
+
+abseil-str-cat-append
+

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added inline comments.



Comment at: test/clang-tidy/abseil-str-cat-append.cpp:91
+
+void bar() {
+  std::string a, b;

JonasToth wrote:
> What happens if `StrCat` is used e.g. in an `std::accumulate` call as the 
> binary operator? (https://en.cppreference.com/w/cpp/algorithm/accumulate  the 
> Example includes such a case)
> Is this diagnosed, too?
> 
> The general case would be if `StrCat` is used as a template argument for a 
> functor.
it doesn't diagnose those cases. and diagnosing it doesn't seem to be worthwhile


https://reviews.llvm.org/D51061



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 161974.
hugoeg added a comment.

minor fixes


https://reviews.llvm.org/D51061

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/StrCatAppendCheck.cpp
  clang-tidy/abseil/StrCatAppendCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-str-cat-append.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-str-cat-append.cpp

Index: test/clang-tidy/abseil-str-cat-append.cpp
===
--- test/clang-tidy/abseil-str-cat-append.cpp
+++ test/clang-tidy/abseil-str-cat-append.cpp
@@ -0,0 +1,129 @@
+// RUN: %check_clang_tidy %s abseil-str-cat-append %t -- -- -I%S -std=c++11
+
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+typedef __SIZE_TYPE__ size;
+
+namespace std {
+template 
+class allocator {};
+template 
+class char_traits {};
+template 
+struct basic_string {
+  typedef basic_string _Type;
+  basic_string();
+  basic_string(const C* p, const A& a = A());
+
+  const C* c_str() const;
+  const C* data() const;
+
+  _Type& append(const C* s);
+  _Type& append(const C* s, size n);
+  _Type& assign(const C* s);
+  _Type& assign(const C* s, size n);
+
+  int compare(const _Type&) const;
+  int compare(const C* s) const;
+  int compare(size pos, size len, const _Type&) const;
+  int compare(size pos, size len, const C* s) const;
+
+  size find(const _Type& str, size pos = 0) const;
+  size find(const C* s, size pos = 0) const;
+  size find(const C* s, size pos, size n) const;
+
+  _Type& insert(size pos, const _Type& str);
+  _Type& insert(size pos, const C* s);
+  _Type& insert(size pos, const C* s, size n);
+
+  _Type& operator+=(const _Type& str);
+  _Type& operator+=(const C* s);
+  _Type& operator=(const _Type& str);
+  _Type& operator=(const C* s);
+};
+
+typedef basic_string, std::allocator> string;
+typedef basic_string,
+ std::allocator>
+wstring;
+typedef basic_string, std::allocator>
+u16string;
+typedef basic_string, std::allocator>
+u32string;
+}  // namespace std
+
+std::string operator+(const std::string&, const std::string&);
+std::string operator+(const std::string&, const char*);
+std::string operator+(const char*, const std::string&);
+
+bool operator==(const std::string&, const std::string&);
+bool operator==(const std::string&, const char*);
+bool operator==(const char*, const std::string&);
+
+namespace llvm {
+struct StringRef {
+  StringRef(const char* p);
+  StringRef(const std::string&);
+};
+}  // namespace llvm
+
+namespace absl {
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char* c_str);
+  AlphaNum(const std::string& str);
+
+ private:
+  AlphaNum(const AlphaNum&);
+  AlphaNum& operator=(const AlphaNum&);
+};
+
+std::string StrCat(const AlphaNum& A);
+std::string StrCat(const AlphaNum& A, const AlphaNum& B);
+
+template 
+void foo(A& a) {
+  a = StrCat(a);
+}
+
+void bar() {
+  std::string A, B;
+  foo(A);
+
+  std::string C = StrCat(A);
+  A = StrCat(A);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call to absl::StrCat does nothing
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+  B = StrCat(A, B);
+
+#define M(X) X = StrCat(X, A)
+  M(B);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: #define M(X) X = StrCat(X, A)
+}
+
+void Regression_SelfAppend() {
+  std::string A;
+  A = StrCat(A, A);
+}
+
+}  // namespace absl
+
+void OutsideAbsl() {
+  std::string A, B;
+  A = absl::StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
+
+void OutisdeUsingAbsl() {
+  std::string A, B;
+  using absl::StrCat;
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-duration-division
+   abseil-str-cat-append
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-str-cat-append.rst
===
--- docs/clang-tidy/checks/abseil-str-cat-append.rst
+++ docs/clang-tidy/checks/abseil-str-cat-append.rst
@@ -0,0 +1,11 @@
+.. title:: clang-tidy - abseil-str-cat-append
+
+abseil-str-cat-append
+=
+
+Flags uses of ``absl::StrCat()`` to append to a std::string

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 161982.

https://reviews.llvm.org/D51061

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/StrCatAppendCheck.cpp
  clang-tidy/abseil/StrCatAppendCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-str-cat-append.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-str-cat-append.cpp

Index: test/clang-tidy/abseil-str-cat-append.cpp
===
--- test/clang-tidy/abseil-str-cat-append.cpp
+++ test/clang-tidy/abseil-str-cat-append.cpp
@@ -0,0 +1,129 @@
+// RUN: %check_clang_tidy %s abseil-str-cat-append %t -- -- -I%S -std=c++11
+
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+typedef __SIZE_TYPE__ size;
+
+namespace std {
+template 
+class allocator {};
+template 
+class char_traits {};
+template 
+struct basic_string {
+  typedef basic_string _Type;
+  basic_string();
+  basic_string(const C* p, const A& a = A());
+
+  const C* c_str() const;
+  const C* data() const;
+
+  _Type& append(const C* s);
+  _Type& append(const C* s, size n);
+  _Type& assign(const C* s);
+  _Type& assign(const C* s, size n);
+
+  int compare(const _Type&) const;
+  int compare(const C* s) const;
+  int compare(size pos, size len, const _Type&) const;
+  int compare(size pos, size len, const C* s) const;
+
+  size find(const _Type& str, size pos = 0) const;
+  size find(const C* s, size pos = 0) const;
+  size find(const C* s, size pos, size n) const;
+
+  _Type& insert(size pos, const _Type& str);
+  _Type& insert(size pos, const C* s);
+  _Type& insert(size pos, const C* s, size n);
+
+  _Type& operator+=(const _Type& str);
+  _Type& operator+=(const C* s);
+  _Type& operator=(const _Type& str);
+  _Type& operator=(const C* s);
+};
+
+typedef basic_string, std::allocator> string;
+typedef basic_string,
+ std::allocator>
+wstring;
+typedef basic_string, std::allocator>
+u16string;
+typedef basic_string, std::allocator>
+u32string;
+}  // namespace std
+
+std::string operator+(const std::string&, const std::string&);
+std::string operator+(const std::string&, const char*);
+std::string operator+(const char*, const std::string&);
+
+bool operator==(const std::string&, const std::string&);
+bool operator==(const std::string&, const char*);
+bool operator==(const char*, const std::string&);
+
+namespace llvm {
+struct StringRef {
+  StringRef(const char* p);
+  StringRef(const std::string&);
+};
+}  // namespace llvm
+
+namespace absl {
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char* c_str);
+  AlphaNum(const std::string& str);
+
+ private:
+  AlphaNum(const AlphaNum&);
+  AlphaNum& operator=(const AlphaNum&);
+};
+
+std::string StrCat(const AlphaNum& A);
+std::string StrCat(const AlphaNum& A, const AlphaNum& B);
+
+template 
+void foo(A& a) {
+  a = StrCat(a);
+}
+
+void bar() {
+  std::string A, B;
+  foo(A);
+
+  std::string C = StrCat(A);
+  A = StrCat(A);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call to absl::StrCat does nothing
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+  B = StrCat(A, B);
+
+#define M(X) X = StrCat(X, A)
+  M(B);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: #define M(X) X = StrCat(X, A)
+}
+
+void Regression_SelfAppend() {
+  std::string A;
+  A = StrCat(A, A);
+}
+
+}  // namespace absl
+
+void OutsideAbsl() {
+  std::string A, B;
+  A = absl::StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
+
+void OutisdeUsingAbsl() {
+  std::string A, B;
+  using absl::StrCat;
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-duration-division
+   abseil-str-cat-append
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-str-cat-append.rst
===
--- docs/clang-tidy/checks/abseil-str-cat-append.rst
+++ docs/clang-tidy/checks/abseil-str-cat-append.rst
@@ -0,0 +1,14 @@
+.. title:: clang-tidy - abseil-str-cat-append
+
+abseil-str-cat-append
+=
+
+Flags uses of ``absl::StrCat()`` to append to a std::string. Suggests 
+``absl::StrAppend()`` sho

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 161995.
hugoeg marked 2 inline comments as done.

https://reviews.llvm.org/D51061

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/StrCatAppendCheck.cpp
  clang-tidy/abseil/StrCatAppendCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-str-cat-append.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-str-cat-append.cpp

Index: test/clang-tidy/abseil-str-cat-append.cpp
===
--- test/clang-tidy/abseil-str-cat-append.cpp
+++ test/clang-tidy/abseil-str-cat-append.cpp
@@ -0,0 +1,129 @@
+// RUN: %check_clang_tidy %s abseil-str-cat-append %t -- -- -I%S -std=c++11
+
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+typedef __SIZE_TYPE__ size;
+
+namespace std {
+template 
+class allocator {};
+template 
+class char_traits {};
+template 
+struct basic_string {
+  typedef basic_string _Type;
+  basic_string();
+  basic_string(const C* p, const A& a = A());
+
+  const C* c_str() const;
+  const C* data() const;
+
+  _Type& append(const C* s);
+  _Type& append(const C* s, size n);
+  _Type& assign(const C* s);
+  _Type& assign(const C* s, size n);
+
+  int compare(const _Type&) const;
+  int compare(const C* s) const;
+  int compare(size pos, size len, const _Type&) const;
+  int compare(size pos, size len, const C* s) const;
+
+  size find(const _Type& str, size pos = 0) const;
+  size find(const C* s, size pos = 0) const;
+  size find(const C* s, size pos, size n) const;
+
+  _Type& insert(size pos, const _Type& str);
+  _Type& insert(size pos, const C* s);
+  _Type& insert(size pos, const C* s, size n);
+
+  _Type& operator+=(const _Type& str);
+  _Type& operator+=(const C* s);
+  _Type& operator=(const _Type& str);
+  _Type& operator=(const C* s);
+};
+
+typedef basic_string, std::allocator> string;
+typedef basic_string,
+ std::allocator>
+wstring;
+typedef basic_string, std::allocator>
+u16string;
+typedef basic_string, std::allocator>
+u32string;
+}  // namespace std
+
+std::string operator+(const std::string&, const std::string&);
+std::string operator+(const std::string&, const char*);
+std::string operator+(const char*, const std::string&);
+
+bool operator==(const std::string&, const std::string&);
+bool operator==(const std::string&, const char*);
+bool operator==(const char*, const std::string&);
+
+namespace llvm {
+struct StringRef {
+  StringRef(const char* p);
+  StringRef(const std::string&);
+};
+}  // namespace llvm
+
+namespace absl {
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char* c_str);
+  AlphaNum(const std::string& str);
+
+ private:
+  AlphaNum(const AlphaNum&);
+  AlphaNum& operator=(const AlphaNum&);
+};
+
+std::string StrCat(const AlphaNum& A);
+std::string StrCat(const AlphaNum& A, const AlphaNum& B);
+
+template 
+void foo(A& a) {
+  a = StrCat(a);
+}
+
+void bar() {
+  std::string A, B;
+  foo(A);
+
+  std::string C = StrCat(A);
+  A = StrCat(A);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call to absl::StrCat does nothing
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+  B = StrCat(A, B);
+
+#define M(X) X = StrCat(X, A)
+  M(B);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: #define M(X) X = StrCat(X, A)
+}
+
+void Regression_SelfAppend() {
+  std::string A;
+  A = StrCat(A, A);
+}
+
+}  // namespace absl
+
+void OutsideAbsl() {
+  std::string A, B;
+  A = absl::StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
+
+void OutisdeUsingAbsl() {
+  std::string A, B;
+  using absl::StrCat;
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-duration-division
+   abseil-str-cat-append
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-str-cat-append.rst
===
--- docs/clang-tidy/checks/abseil-str-cat-append.rst
+++ docs/clang-tidy/checks/abseil-str-cat-append.rst
@@ -0,0 +1,14 @@
+.. title:: clang-tidy - abseil-str-cat-append
+
+abseil-str-cat-append
+=
+
+Flags uses of ``absl::StrCat()`` to append to a ``std::s

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg marked 7 inline comments as done.
hugoeg added a comment.

If the fix-it can be implemented, I certainly do not know how. As mentioned 
above, I am not the original author for this check.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51132



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162174.
hugoeg added a comment.

corrections applied


https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view a, string_view b);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &a);
+string StrCat(const AlphaNum &a, const AlphaNum &b);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d, const AlphaNum &e, const AV &... args);
+
+void StrAppend(string *dest, const AlphaNum &a);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d, const AlphaNum &e,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)));
+  // CHECK-MESSAGES: [[@LINE-1

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162219.

https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view a, string_view b);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &a);
+string StrCat(const AlphaNum &a, const AlphaNum &b);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d, const AlphaNum &e, const AV &... args);
+
+void StrAppend(string *dest, const AlphaNum &a);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d, const AlphaNum &e,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)));
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: redundant StrCat calls
+
+  S 

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162223.

https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view a, string_view b);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &a);
+string StrCat(const AlphaNum &a, const AlphaNum &b);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c);
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &a, const AlphaNum &b, const AlphaNum &c,
+  const AlphaNum &d, const AlphaNum &e, const AV &... args);
+
+void StrAppend(string *dest, const AlphaNum &a);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c);
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *dest, const AlphaNum &a, const AlphaNum &b,
+   const AlphaNum &c, const AlphaNum &d, const AlphaNum &e,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)));
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: redundant StrCat calls
+
+  S 

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

Let me know if there's anything else I can fix to move the process along.


https://reviews.llvm.org/D51061



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162231.

https://reviews.llvm.org/D51061

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/StrCatAppendCheck.cpp
  clang-tidy/abseil/StrCatAppendCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-str-cat-append.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-str-cat-append.cpp

Index: test/clang-tidy/abseil-str-cat-append.cpp
===
--- test/clang-tidy/abseil-str-cat-append.cpp
+++ test/clang-tidy/abseil-str-cat-append.cpp
@@ -0,0 +1,129 @@
+// RUN: %check_clang_tidy %s abseil-str-cat-append %t -- -- -I%S -std=c++11
+
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+typedef __SIZE_TYPE__ size;
+
+namespace std {
+template 
+class allocator {};
+template 
+class char_traits {};
+template 
+struct basic_string {
+  typedef basic_string _Type;
+  basic_string();
+  basic_string(const C* p, const A& a = A());
+
+  const C* c_str() const;
+  const C* data() const;
+
+  _Type& append(const C* s);
+  _Type& append(const C* s, size n);
+  _Type& assign(const C* s);
+  _Type& assign(const C* s, size n);
+
+  int compare(const _Type&) const;
+  int compare(const C* s) const;
+  int compare(size pos, size len, const _Type&) const;
+  int compare(size pos, size len, const C* s) const;
+
+  size find(const _Type& str, size pos = 0) const;
+  size find(const C* s, size pos = 0) const;
+  size find(const C* s, size pos, size n) const;
+
+  _Type& insert(size pos, const _Type& str);
+  _Type& insert(size pos, const C* s);
+  _Type& insert(size pos, const C* s, size n);
+
+  _Type& operator+=(const _Type& str);
+  _Type& operator+=(const C* s);
+  _Type& operator=(const _Type& str);
+  _Type& operator=(const C* s);
+};
+
+typedef basic_string, std::allocator> string;
+typedef basic_string,
+ std::allocator>
+wstring;
+typedef basic_string, std::allocator>
+u16string;
+typedef basic_string, std::allocator>
+u32string;
+}  // namespace std
+
+std::string operator+(const std::string&, const std::string&);
+std::string operator+(const std::string&, const char*);
+std::string operator+(const char*, const std::string&);
+
+bool operator==(const std::string&, const std::string&);
+bool operator==(const std::string&, const char*);
+bool operator==(const char*, const std::string&);
+
+namespace llvm {
+struct StringRef {
+  StringRef(const char* p);
+  StringRef(const std::string&);
+};
+}  // namespace llvm
+
+namespace absl {
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char* c_str);
+  AlphaNum(const std::string& str);
+
+ private:
+  AlphaNum(const AlphaNum&);
+  AlphaNum& operator=(const AlphaNum&);
+};
+
+std::string StrCat(const AlphaNum& A);
+std::string StrCat(const AlphaNum& A, const AlphaNum& B);
+
+template 
+void foo(A& a) {
+  a = StrCat(a);
+}
+
+void bar() {
+  std::string A, B;
+  foo(A);
+
+  std::string C = StrCat(A);
+  A = StrCat(A);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call to absl::StrCat does nothing
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+  B = StrCat(A, B);
+
+#define M(X) X = StrCat(X, A)
+  M(B);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: #define M(X) X = StrCat(X, A)
+}
+
+void Regression_SelfAppend() {
+  std::string A;
+  A = StrCat(A, A);
+}
+
+}  // namespace absl
+
+void OutsideAbsl() {
+  std::string A, B;
+  A = absl::StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
+
+void OutisdeUsingAbsl() {
+  std::string A, B;
+  using absl::StrCat;
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-duration-division
+   abseil-str-cat-append
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-str-cat-append.rst
===
--- docs/clang-tidy/checks/abseil-str-cat-append.rst
+++ docs/clang-tidy/checks/abseil-str-cat-append.rst
@@ -0,0 +1,14 @@
+.. title:: clang-tidy - abseil-str-cat-append
+
+abseil-str-cat-append
+=
+
+Flags uses of ``absl::StrCat()`` to append to a ``std::string``. Suggests 
+``absl::StrAppend()``

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162253.
hugoeg added a comment.

minor fixes, style improvements


https://reviews.llvm.org/D51061

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/StrCatAppendCheck.cpp
  clang-tidy/abseil/StrCatAppendCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-str-cat-append.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-str-cat-append.cpp

Index: test/clang-tidy/abseil-str-cat-append.cpp
===
--- test/clang-tidy/abseil-str-cat-append.cpp
+++ test/clang-tidy/abseil-str-cat-append.cpp
@@ -0,0 +1,129 @@
+// RUN: %check_clang_tidy %s abseil-str-cat-append %t -- -- -I%S -std=c++11
+
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+typedef __SIZE_TYPE__ size;
+
+namespace std {
+template 
+class allocator {};
+template 
+class char_traits {};
+template 
+struct basic_string {
+  typedef basic_string _Type;
+  basic_string();
+  basic_string(const C* p, const A& a = A());
+
+  const C* c_str() const;
+  const C* data() const;
+
+  _Type& append(const C* s);
+  _Type& append(const C* s, size n);
+  _Type& assign(const C* s);
+  _Type& assign(const C* s, size n);
+
+  int compare(const _Type&) const;
+  int compare(const C* s) const;
+  int compare(size pos, size len, const _Type&) const;
+  int compare(size pos, size len, const C* s) const;
+
+  size find(const _Type& str, size pos = 0) const;
+  size find(const C* s, size pos = 0) const;
+  size find(const C* s, size pos, size n) const;
+
+  _Type& insert(size pos, const _Type& str);
+  _Type& insert(size pos, const C* s);
+  _Type& insert(size pos, const C* s, size n);
+
+  _Type& operator+=(const _Type& str);
+  _Type& operator+=(const C* s);
+  _Type& operator=(const _Type& str);
+  _Type& operator=(const C* s);
+};
+
+typedef basic_string, std::allocator> string;
+typedef basic_string,
+ std::allocator>
+wstring;
+typedef basic_string, std::allocator>
+u16string;
+typedef basic_string, std::allocator>
+u32string;
+}  // namespace std
+
+std::string operator+(const std::string&, const std::string&);
+std::string operator+(const std::string&, const char*);
+std::string operator+(const char*, const std::string&);
+
+bool operator==(const std::string&, const std::string&);
+bool operator==(const std::string&, const char*);
+bool operator==(const char*, const std::string&);
+
+namespace llvm {
+struct StringRef {
+  StringRef(const char* p);
+  StringRef(const std::string&);
+};
+}  // namespace llvm
+
+namespace absl {
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char* c_str);
+  AlphaNum(const std::string& str);
+
+ private:
+  AlphaNum(const AlphaNum&);
+  AlphaNum& operator=(const AlphaNum&);
+};
+
+std::string StrCat(const AlphaNum& A);
+std::string StrCat(const AlphaNum& A, const AlphaNum& B);
+
+template 
+void Foo(A& a) {
+  a = StrCat(a);
+}
+
+void Bar() {
+  std::string A, B;
+  Foo(A);
+
+  std::string C = StrCat(A);
+  A = StrCat(A);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call to absl::StrCat does nothing
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+  B = StrCat(A, B);
+
+#define M(X) X = StrCat(X, A)
+  M(B);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: #define M(X) X = StrCat(X, A)
+}
+
+void Regression_SelfAppend() {
+  std::string A;
+  A = StrCat(A, A);
+}
+
+}  // namespace absl
+
+void OutsideAbsl() {
+  std::string A, B;
+  A = absl::StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
+
+void OutisdeUsingAbsl() {
+  std::string A, B;
+  using absl::StrCat;
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-duration-division
+   abseil-str-cat-append
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-str-cat-append.rst
===
--- docs/clang-tidy/checks/abseil-str-cat-append.rst
+++ docs/clang-tidy/checks/abseil-str-cat-append.rst
@@ -0,0 +1,14 @@
+.. title:: clang-tidy - abseil-str-cat-append
+
+abseil-str-cat-append
+=
+
+Flags uses of ``absl::StrCat()`` to app

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162256.
hugoeg added a comment.

minor fixes and style improvement


https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view A, string_view B);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &A);
+string StrCat(const AlphaNum &A, const AlphaNum &B);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D, const AlphaNum &E, const AV &... args);
+
+void StrAppend(string *Dest, const AlphaNum &A);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D, const AlphaNum &E,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)));
+  // CHECK-MESSA

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162379.
hugoeg marked 2 inline comments as done.
hugoeg added a comment.

fixed comments


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/external-file.h
  test/clang-tidy/Inputs/absl/strings/internal-file.h
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t,  -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-internal-deps' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:1:23: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil [abseil-no-internal-deps]
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+void OpeningNamespace() {
+  strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+SomeContainer b;
+std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/internal-file.h
===
--- test/clang-tidy/Inputs/absl/strings/internal-file.h
+++ test/clang-tidy/Inputs/absl/strings/internal-file.h
@@ -0,0 +1,33 @@
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/external-file.h
===
--- test/clang-tidy/Inputs/absl/external-file.h
+++ test/clang-tidy/Inputs/absl/external-file.h
@@ -0,0 +1 @@
+void DirectAcess2() { absl::strings_internal::InternalFunction(); }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -6,6 +6,7 @@
 .. toctree::
abseil-duration-division
abseil-faster-strsplit-delimiter
+   abseil-no-internal-deps
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,24 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Warns if code using Abseil depends on internal details. If something is in a
+namespace that includes the word “internal”, code is not allowed to depend upon
+it beaucse it’s an implementation detail. They cannot friend it, include it,
+you mention it or refer to it in any way. Doing so violates Abseil's
+compatibility guidelines and may result in breakage. See
+https://abseil.io/about/com

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added inline comments.



Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:8
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:1:23: warning: do not reference any 'internal' 
namespaces; those implementation details are reserved to Abseil 
[abseil-no-internal-deps]
+

hokein wrote:
> Does the test get passed on the first run `RUN: %check_clang_tidy %s 
> abseil-no-internal-deps %t,  -- -- -I %S/Inputs` of the test? It will 
> suppress clang-tidy warnings from the header, and the warning here should not 
> appear.
Yes, the test passes cleanly on both runs, I just re ran it a couple times to 
make sure. 


https://reviews.llvm.org/D50542



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162382.
hugoeg marked 7 inline comments as done.
hugoeg added a comment.

applied corrections form comments


https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view A, string_view B);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &A);
+string StrCat(const AlphaNum &A, const AlphaNum &B);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D, const AlphaNum &E, const AV &... args);
+
+void StrAppend(string *Dest, const AlphaNum &A);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D, const AlphaNum &E,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, Str

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added inline comments.



Comment at: clang-tidy/abseil/RedundantStrcatCallsCheck.cpp:37-38
+  // Those are handled on the ancestor call.
+  const auto CallToEither = callExpr(
+  callee(functionDecl(hasAnyName("::absl::StrCat", "::absl::StrAppend";
+  Finder->addMatcher(

aaron.ballman wrote:
> Can this be replaced by `anyOf(CallToStrcat, CallToStrappend)`?
I just tried it, it doesn't work, so we should probably keep it as is. 


https://reviews.llvm.org/D51132



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162390.
hugoeg marked 9 inline comments as done.
hugoeg added a comment.

applied corrections from comments


https://reviews.llvm.org/D51061

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/StrCatAppendCheck.cpp
  clang-tidy/abseil/StrCatAppendCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-str-cat-append.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-str-cat-append.cpp

Index: test/clang-tidy/abseil-str-cat-append.cpp
===
--- test/clang-tidy/abseil-str-cat-append.cpp
+++ test/clang-tidy/abseil-str-cat-append.cpp
@@ -0,0 +1,129 @@
+// RUN: %check_clang_tidy %s abseil-str-cat-append %t -- -- -I%S -std=c++11
+
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+typedef __SIZE_TYPE__ size;
+
+namespace std {
+template 
+class allocator {};
+template 
+class char_traits {};
+template 
+struct basic_string {
+  typedef basic_string _Type;
+  basic_string();
+  basic_string(const C* p, const A& a = A());
+
+  const C* c_str() const;
+  const C* data() const;
+
+  _Type& append(const C* s);
+  _Type& append(const C* s, size n);
+  _Type& assign(const C* s);
+  _Type& assign(const C* s, size n);
+
+  int compare(const _Type&) const;
+  int compare(const C* s) const;
+  int compare(size pos, size len, const _Type&) const;
+  int compare(size pos, size len, const C* s) const;
+
+  size find(const _Type& str, size pos = 0) const;
+  size find(const C* s, size pos = 0) const;
+  size find(const C* s, size pos, size n) const;
+
+  _Type& insert(size pos, const _Type& str);
+  _Type& insert(size pos, const C* s);
+  _Type& insert(size pos, const C* s, size n);
+
+  _Type& operator+=(const _Type& str);
+  _Type& operator+=(const C* s);
+  _Type& operator=(const _Type& str);
+  _Type& operator=(const C* s);
+};
+
+typedef basic_string, std::allocator> string;
+typedef basic_string,
+ std::allocator>
+wstring;
+typedef basic_string, std::allocator>
+u16string;
+typedef basic_string, std::allocator>
+u32string;
+}  // namespace std
+
+std::string operator+(const std::string&, const std::string&);
+std::string operator+(const std::string&, const char*);
+std::string operator+(const char*, const std::string&);
+
+bool operator==(const std::string&, const std::string&);
+bool operator==(const std::string&, const char*);
+bool operator==(const char*, const std::string&);
+
+namespace llvm {
+struct StringRef {
+  StringRef(const char* p);
+  StringRef(const std::string&);
+};
+}  // namespace llvm
+
+namespace absl {
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char* c_str);
+  AlphaNum(const std::string& str);
+
+ private:
+  AlphaNum(const AlphaNum&);
+  AlphaNum& operator=(const AlphaNum&);
+};
+
+std::string StrCat(const AlphaNum& A);
+std::string StrCat(const AlphaNum& A, const AlphaNum& B);
+
+template 
+void Foo(A& a) {
+  a = StrCat(a);
+}
+
+void Bar() {
+  std::string A, B;
+  Foo(A);
+
+  std::string C = StrCat(A);
+  A = StrCat(A);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call to 'absl::StrCat' has no effect
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: call 'absl::StrAppend' instead of 'absl::StrCat' when appending to a string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+  B = StrCat(A, B);
+
+#define M(X) X = StrCat(X, A)
+  M(B);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: call 'absl::StrAppend' instead of 'absl::StrCat' when appending to a string
+// CHECK-FIXES: #define M(X) X = StrCat(X, A)
+}
+
+void Regression_SelfAppend() {
+  std::string A;
+  A = StrCat(A, A);
+}
+
+}  // namespace absl
+
+void OutsideAbsl() {
+  std::string A, B;
+  A = absl::StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: call 'absl::StrAppend' instead of 'absl::StrCat' when appending to a string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
+
+void OutisdeUsingAbsl() {
+  std::string A, B;
+  using absl::StrCat;
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: call 'absl::StrAppend' instead of 'absl::StrCat' when appending to a string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-duration-division
+   abseil-str-cat-append
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-str-cat-append.rst
===
--- docs/clang-tidy/checks/abseil-str-cat-append.rst
+++ docs/clang-tidy/checks/abseil-str-cat-append.rst
@@ -0,0 +1,17 @@
+.. title:: clang-tidy - abseil-str-cat-append
+
+abseil-str-cat-append
+=
+
+Flags uses of ``absl::StrCat()`` to appen

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162395.
hugoeg added a comment.

ran svn update


https://reviews.llvm.org/D51061

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/StrCatAppendCheck.cpp
  clang-tidy/abseil/StrCatAppendCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-str-cat-append.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-str-cat-append.cpp

Index: test/clang-tidy/abseil-str-cat-append.cpp
===
--- test/clang-tidy/abseil-str-cat-append.cpp
+++ test/clang-tidy/abseil-str-cat-append.cpp
@@ -0,0 +1,129 @@
+// RUN: %check_clang_tidy %s abseil-str-cat-append %t -- -- -I%S -std=c++11
+
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+typedef __SIZE_TYPE__ size;
+
+namespace std {
+template 
+class allocator {};
+template 
+class char_traits {};
+template 
+struct basic_string {
+  typedef basic_string _Type;
+  basic_string();
+  basic_string(const C* p, const A& a = A());
+
+  const C* c_str() const;
+  const C* data() const;
+
+  _Type& append(const C* s);
+  _Type& append(const C* s, size n);
+  _Type& assign(const C* s);
+  _Type& assign(const C* s, size n);
+
+  int compare(const _Type&) const;
+  int compare(const C* s) const;
+  int compare(size pos, size len, const _Type&) const;
+  int compare(size pos, size len, const C* s) const;
+
+  size find(const _Type& str, size pos = 0) const;
+  size find(const C* s, size pos = 0) const;
+  size find(const C* s, size pos, size n) const;
+
+  _Type& insert(size pos, const _Type& str);
+  _Type& insert(size pos, const C* s);
+  _Type& insert(size pos, const C* s, size n);
+
+  _Type& operator+=(const _Type& str);
+  _Type& operator+=(const C* s);
+  _Type& operator=(const _Type& str);
+  _Type& operator=(const C* s);
+};
+
+typedef basic_string, std::allocator> string;
+typedef basic_string,
+ std::allocator>
+wstring;
+typedef basic_string, std::allocator>
+u16string;
+typedef basic_string, std::allocator>
+u32string;
+}  // namespace std
+
+std::string operator+(const std::string&, const std::string&);
+std::string operator+(const std::string&, const char*);
+std::string operator+(const char*, const std::string&);
+
+bool operator==(const std::string&, const std::string&);
+bool operator==(const std::string&, const char*);
+bool operator==(const char*, const std::string&);
+
+namespace llvm {
+struct StringRef {
+  StringRef(const char* p);
+  StringRef(const std::string&);
+};
+}  // namespace llvm
+
+namespace absl {
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char* c_str);
+  AlphaNum(const std::string& str);
+
+ private:
+  AlphaNum(const AlphaNum&);
+  AlphaNum& operator=(const AlphaNum&);
+};
+
+std::string StrCat(const AlphaNum& A);
+std::string StrCat(const AlphaNum& A, const AlphaNum& B);
+
+template 
+void Foo(A& a) {
+  a = StrCat(a);
+}
+
+void Bar() {
+  std::string A, B;
+  Foo(A);
+
+  std::string C = StrCat(A);
+  A = StrCat(A);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call to 'absl::StrCat' has no effect
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: call 'absl::StrAppend' instead of 'absl::StrCat' when appending to a string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+  B = StrCat(A, B);
+
+#define M(X) X = StrCat(X, A)
+  M(B);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: call 'absl::StrAppend' instead of 'absl::StrCat' when appending to a string
+// CHECK-FIXES: #define M(X) X = StrCat(X, A)
+}
+
+void Regression_SelfAppend() {
+  std::string A;
+  A = StrCat(A, A);
+}
+
+}  // namespace absl
+
+void OutsideAbsl() {
+  std::string A, B;
+  A = absl::StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: call 'absl::StrAppend' instead of 'absl::StrCat' when appending to a string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
+
+void OutisdeUsingAbsl() {
+  std::string A, B;
+  using absl::StrCat;
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: call 'absl::StrAppend' instead of 'absl::StrCat' when appending to a string
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -7,6 +7,7 @@
abseil-duration-division
abseil-faster-strsplit-delimiter
abseil-string-find-startswith
+   abseil-str-cat-append
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-str-cat-append.rst
===
--- docs/clang-tidy/checks/abseil-str-cat-append.rst
+++ docs/clang-tidy/checks/abseil-str-cat-append.rst
@@ -0,0 +1,17 @@
+.. title:: clang-tidy - abseil-str-cat-append
+
+abseil-str-cat-append
+=
+
+Flags uses of ``absl::StrCat()`` to append to a ``std:

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162403.
hugoeg marked an inline comment as done.

https://reviews.llvm.org/D51061

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/StrCatAppendCheck.cpp
  clang-tidy/abseil/StrCatAppendCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-str-cat-append.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-str-cat-append.cpp

Index: test/clang-tidy/abseil-str-cat-append.cpp
===
--- test/clang-tidy/abseil-str-cat-append.cpp
+++ test/clang-tidy/abseil-str-cat-append.cpp
@@ -0,0 +1,129 @@
+// RUN: %check_clang_tidy %s abseil-str-cat-append %t -- -- -I%S -std=c++11
+
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+typedef __SIZE_TYPE__ size;
+
+namespace std {
+template 
+class allocator {};
+template 
+class char_traits {};
+template 
+struct basic_string {
+  typedef basic_string _Type;
+  basic_string();
+  basic_string(const C* p, const A& a = A());
+
+  const C* c_str() const;
+  const C* data() const;
+
+  _Type& append(const C* s);
+  _Type& append(const C* s, size n);
+  _Type& assign(const C* s);
+  _Type& assign(const C* s, size n);
+
+  int compare(const _Type&) const;
+  int compare(const C* s) const;
+  int compare(size pos, size len, const _Type&) const;
+  int compare(size pos, size len, const C* s) const;
+
+  size find(const _Type& str, size pos = 0) const;
+  size find(const C* s, size pos = 0) const;
+  size find(const C* s, size pos, size n) const;
+
+  _Type& insert(size pos, const _Type& str);
+  _Type& insert(size pos, const C* s);
+  _Type& insert(size pos, const C* s, size n);
+
+  _Type& operator+=(const _Type& str);
+  _Type& operator+=(const C* s);
+  _Type& operator=(const _Type& str);
+  _Type& operator=(const C* s);
+};
+
+typedef basic_string, std::allocator> string;
+typedef basic_string,
+ std::allocator>
+wstring;
+typedef basic_string, std::allocator>
+u16string;
+typedef basic_string, std::allocator>
+u32string;
+}  // namespace std
+
+std::string operator+(const std::string&, const std::string&);
+std::string operator+(const std::string&, const char*);
+std::string operator+(const char*, const std::string&);
+
+bool operator==(const std::string&, const std::string&);
+bool operator==(const std::string&, const char*);
+bool operator==(const char*, const std::string&);
+
+namespace llvm {
+struct StringRef {
+  StringRef(const char* p);
+  StringRef(const std::string&);
+};
+}  // namespace llvm
+
+namespace absl {
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char* c_str);
+  AlphaNum(const std::string& str);
+
+ private:
+  AlphaNum(const AlphaNum&);
+  AlphaNum& operator=(const AlphaNum&);
+};
+
+std::string StrCat(const AlphaNum& A);
+std::string StrCat(const AlphaNum& A, const AlphaNum& B);
+
+template 
+void Foo(A& a) {
+  a = StrCat(a);
+}
+
+void Bar() {
+  std::string A, B;
+  Foo(A);
+
+  std::string C = StrCat(A);
+  A = StrCat(A);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call to 'absl::StrCat' has no effect
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: call 'absl::StrAppend' instead of 'absl::StrCat' when appending to a string to avoid a performance penalty
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+  B = StrCat(A, B);
+
+#define M(X) X = StrCat(X, A)
+  M(B);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: call 'absl::StrAppend' instead of 'absl::StrCat' when appending to a string to avoid a performance penalty
+// CHECK-FIXES: #define M(X) X = StrCat(X, A)
+}
+
+void Regression_SelfAppend() {
+  std::string A;
+  A = StrCat(A, A);
+}
+
+}  // namespace absl
+
+void OutsideAbsl() {
+  std::string A, B;
+  A = absl::StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: call 'absl::StrAppend' instead of 'absl::StrCat' when appending to a string to avoid a performance penalty
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
+
+void OutisdeUsingAbsl() {
+  std::string A, B;
+  using absl::StrCat;
+  A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: call 'absl::StrAppend' instead of 'absl::StrCat' when appending to a string to avoid a performance penalty
+// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -7,6 +7,7 @@
abseil-duration-division
abseil-faster-strsplit-delimiter
abseil-string-find-startswith
+   abseil-str-cat-append
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-str-cat-append.rst
===
--- docs/clang-tidy/checks/abseil-str-cat-append.rst
+++ docs/clang-tidy/checks/abseil-str-cat-append.rst
@@ -0,0 +1,17 @@
+.. title:: clang-tidy - abs

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162405.
hugoeg marked an inline comment as done.
hugoeg added a comment.

minor updates


https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view A, string_view B);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &A);
+string StrCat(const AlphaNum &A, const AlphaNum &B);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D, const AlphaNum &E, const AV &... args);
+
+void StrAppend(string *Dest, const AlphaNum &A);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D, const AlphaNum &E,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162438.
hugoeg marked 2 inline comments as done.
hugoeg added a comment.

added corrections


https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view A, string_view B);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &A);
+string StrCat(const AlphaNum &A, const AlphaNum &B);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D, const AlphaNum &E, const AV &... args);
+
+void StrAppend(string *Dest, const AlphaNum &A);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D, const AlphaNum &E,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162454.
hugoeg marked an inline comment as done.

https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view A, string_view B);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &A);
+string StrCat(const AlphaNum &A, const AlphaNum &B);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D, const AlphaNum &E, const AV &... args);
+
+void StrAppend(string *Dest, const AlphaNum &A);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D, const AlphaNum &E,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)));
+  // CHECK-MESSAGES: [[@LINE-1]]:14

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162455.

https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view A, string_view B);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &A);
+string StrCat(const AlphaNum &A, const AlphaNum &B);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D, const AlphaNum &E, const AV &... args);
+
+void StrAppend(string *Dest, const AlphaNum &A);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D, const AlphaNum &E,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)));
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: multiple calls to 'absl::StrCa

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162462.

https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view A, string_view B);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &A);
+string StrCat(const AlphaNum &A, const AlphaNum &B);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D, const AlphaNum &E, const AV &... args);
+
+void StrAppend(string *Dest, const AlphaNum &A);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D, const AlphaNum &E,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)));
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: multiple calls to 'absl::StrCa

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

In https://reviews.llvm.org/D51132#1212903, @Eugene.Zelenko wrote:

> Indentation of example in documentation is still fixed.


yea sorry about that, I caught that after i updated the diff and corrected and 
re updated


https://reviews.llvm.org/D51132



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162864.
hugoeg marked 2 inline comments as done.
hugoeg added a comment.

made some major updates after no-namespace landed


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/external-file.h
  test/clang-tidy/Inputs/absl/strings/internal-file.h
  test/clang-tidy/abseil-no-internal-deps.cpp
  test/clang-tidy/abseil-no-namespace.cpp

Index: test/clang-tidy/abseil-no-namespace.cpp
===
--- test/clang-tidy/abseil-no-namespace.cpp
+++ test/clang-tidy/abseil-no-namespace.cpp
@@ -7,7 +7,7 @@
 
 /// Warning will be triggered on code that is not internal that is included.
 #include "absl/external-file.h"
-// CHECK: absl/external-file.h:1:11: warning: namespace 'absl' is reserved
+// CHECK: absl/external-file.h:3:11: warning: namespace 'absl' is reserved
 
 namespace absl {}
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: namespace 'absl' is reserved for implementation of the Abseil library and should not be opened in user code [abseil-no-namespace]
Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t,  -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-internal-deps' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:1:23: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil [abseil-no-internal-deps]
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+void OpeningNamespace() {
+  strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+SomeContainer b;
+std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/internal-file.h
===
--- test/clang-tidy/Inputs/absl/strings/internal-file.h
+++ test/clang-tidy/Inputs/absl/strings/internal-file.h
@@ -1 +1,34 @@
-namespace absl {}
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
+
Index: test/clang-tidy/Inputs/absl/external-file.h
===
--- test/clang-tidy/Inputs/absl/external-file.h
+++ test/clang-tidy/Inputs/absl/external-file.h
@@ -1 +1,3 @@
+void DirectAcess2() { absl::strings_internal::InternalFunction(); }
+
 namespace absl {}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -6,6 +6,7 @@
 .. toctree::
abseil-duration-division
abseil-faster-strsplit-delimiter
+   abseil-no-internal-deps
abseil-no-namespace
abseil-string-fi

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

@aaron.ballman when you get the chance could you take another look at this and 
commit if it is ready? My internship ends rather soon and this is a tad time 
sensitive. Thank you for your time!


https://reviews.llvm.org/D51132



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

@aaron.ballman when you get the chance could you take another look at this and 
commit if it is ready? My internship ends rather soon and this is a tad time 
sensitive and I don't have commit access. Thank you for your time!


https://reviews.llvm.org/D51061



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg marked an inline comment as done.
hugoeg added inline comments.



Comment at: test/clang-tidy/Inputs/absl/external-file.h:1
+void DirectAcess2() { absl::strings_internal::InternalFunction(); }
+

hokein wrote:
> The file can not self-compile, and we should make it compilable.
> 
> And put the newly-added code at the end of the file, so that other test file 
> can keep unchanged.
how do I make it compilable?


https://reviews.llvm.org/D50542



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162895.
hugoeg marked 3 inline comments as done.
hugoeg added a comment.

fixed issues outlines in comments


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/external-file.h
  test/clang-tidy/Inputs/absl/strings/internal-file.h
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t,  -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-internal-deps' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:6:24: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil [abseil-no-internal-deps]
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+void OpeningNamespace() {
+  strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+SomeContainer b;
+std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/internal-file.h
===
--- test/clang-tidy/Inputs/absl/strings/internal-file.h
+++ test/clang-tidy/Inputs/absl/strings/internal-file.h
@@ -1 +1,33 @@
-namespace absl {}
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/external-file.h
===
--- test/clang-tidy/Inputs/absl/external-file.h
+++ test/clang-tidy/Inputs/absl/external-file.h
@@ -1 +1,6 @@
-namespace absl {}
+namespace absl {
+namespace base_internal {
+void InternalFunction() {}
+} // namespace base_internal 
+} //namespace absl
+void DirectAccess2() { absl::base_internal::InternalFunction(); }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -6,6 +6,7 @@
 .. toctree::
abseil-duration-division
abseil-faster-strsplit-delimiter
+   abseil-no-internal-deps
abseil-no-namespace
abseil-string-find-startswith
android-cloexec-accept
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,24 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Warns if code using Abseil depends on internal details. If something is in a
+namespace that includes the word “internal”, code is not allowed to depend upon
+it beaucse it’s an implementation detail. They cannot friend

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162921.
hugoeg added a comment.

renamed check as suggested


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDependenciesCheck.cpp
  clang-tidy/abseil/NoInternalDependenciesCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/external-file.h
  test/clang-tidy/Inputs/absl/strings/internal-file.h
  test/clang-tidy/abseil-no-internal-dependencies.cpp

Index: test/clang-tidy/abseil-no-internal-dependencies.cpp
===
--- test/clang-tidy/abseil-no-internal-dependencies.cpp
+++ test/clang-tidy/abseil-no-internal-dependencies.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-dependencies %t,  -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-internal-dependencies' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:6:24: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil [abseil-no-internal-dependencies]
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+void OpeningNamespace() {
+  strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+SomeContainer b;
+std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/internal-file.h
===
--- test/clang-tidy/Inputs/absl/strings/internal-file.h
+++ test/clang-tidy/Inputs/absl/strings/internal-file.h
@@ -1 +1,33 @@
-namespace absl {}
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/external-file.h
===
--- test/clang-tidy/Inputs/absl/external-file.h
+++ test/clang-tidy/Inputs/absl/external-file.h
@@ -1 +1,6 @@
-namespace absl {}
+namespace absl {
+namespace base_internal {
+void InternalFunction() {}
+} // namespace base_internal 
+} //namespace absl
+void DirectAccess2() { absl::base_internal::InternalFunction(); }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -6,6 +6,7 @@
 .. toctree::
abseil-duration-division
abseil-faster-strsplit-delimiter
+   abseil-no-internal-dependencies
abseil-no-namespace
abseil-string-find-startswith
android-cloexec-accept
Index: docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
+++ docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
@@ -0,0 +1,24 @@
+subl.. title:: clang-tidy - abseil-no-internal-dependencies
+
+abseil-no-internal-dependencies
+===
+
+Warns if code using Abseil depends on internal details. If something is in a
+namespace that includes the word “internal”, code is 

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 162932.
hugoeg added a comment.

merge conflicts resolved @aaron.ballman


https://reviews.llvm.org/D51132

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tidy/abseil/RedundantStrcatCallsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-redundant-strcat-calls.cpp

Index: test/clang-tidy/abseil-redundant-strcat-calls.cpp
===
--- test/clang-tidy/abseil-redundant-strcat-calls.cpp
+++ test/clang-tidy/abseil-redundant-strcat-calls.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s abseil-redundant-strcat-calls %t
+
+int strlen(const char *);
+
+// Here we mimic the hierarchy of ::string.
+// We need to do so because we are matching on the fully qualified name of the
+// methods.
+struct __sso_string_base {};
+namespace __gnu_cxx {
+template 
+class __versa_string {
+ public:
+  const char *c_str() const;
+  const char *data() const;
+  int size() const;
+  int capacity() const;
+  int length() const;
+  bool empty() const;
+  char &operator[](int);
+  void clear();
+  void resize(int);
+  int compare(const __versa_string &) const;
+};
+}  // namespace __gnu_cxx
+
+namespace std {
+template 
+class char_traits {};
+template 
+class allocator {};
+}  // namespace std
+
+template ,
+  typename C = std::allocator>
+class basic_string : public __gnu_cxx::__versa_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, C = C());
+  basic_string(const char *, int, C = C());
+  basic_string(const basic_string &, int, int, C = C());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+};
+
+template 
+basic_string operator+(const basic_string &,
+const basic_string &);
+template 
+basic_string operator+(const basic_string &, const char *);
+
+typedef basic_string string;
+
+bool operator==(const string &, const string &);
+bool operator==(const string &, const char *);
+bool operator==(const char *, const string &);
+
+bool operator!=(const string &, const string &);
+bool operator<(const string &, const string &);
+bool operator>(const string &, const string &);
+bool operator<=(const string &, const string &);
+bool operator>=(const string &, const string &);
+
+namespace std {
+template ,
+  typename _Alloc = allocator<_CharT>>
+class basic_string;
+
+template 
+class basic_string {
+ public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const char *, const _Alloc & = _Alloc());
+  basic_string(const char *, int, const _Alloc & = _Alloc());
+  basic_string(const basic_string &, int, int, const _Alloc & = _Alloc());
+  ~basic_string();
+
+  basic_string &operator+=(const basic_string &);
+
+  unsigned size() const;
+  unsigned length() const;
+  bool empty() const;
+};
+
+typedef basic_string string;
+}  // namespace std
+
+namespace absl {
+
+class string_view {
+ public:
+  typedef std::char_traits traits_type;
+
+  string_view();
+  string_view(const char *);
+  string_view(const string &);
+  string_view(const char *, int);
+  string_view(string_view, int);
+
+  template 
+  explicit operator ::basic_string() const;
+
+  const char *data() const;
+  int size() const;
+  int length() const;
+};
+
+bool operator==(string_view A, string_view B);
+
+struct AlphaNum {
+  AlphaNum(int i);
+  AlphaNum(double f);
+  AlphaNum(const char *c_str);
+  AlphaNum(const string &str);
+  AlphaNum(const string_view &pc);
+
+ private:
+  AlphaNum(const AlphaNum &);
+  AlphaNum &operator=(const AlphaNum &);
+};
+
+string StrCat(const AlphaNum &A);
+string StrCat(const AlphaNum &A, const AlphaNum &B);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C);
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+string StrCat(const AlphaNum &A, const AlphaNum &B, const AlphaNum &C,
+  const AlphaNum &D, const AlphaNum &E, const AV &... args);
+
+void StrAppend(string *Dest, const AlphaNum &A);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C);
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D);
+
+// Support 5 or more arguments
+template 
+void StrAppend(string *Dest, const AlphaNum &A, const AlphaNum &B,
+   const AlphaNum &C, const AlphaNum &D, const AlphaNum &E,
+   const AV &... args);
+
+}  // namespace absl
+
+using absl::AlphaNum;
+using absl::StrAppend;
+using absl::StrCat;
+
+void Positives() {
+  string S = StrCat(1, StrCat("A", StrCat(1.1)));
+  // CHECK

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 163083.
hugoeg marked an inline comment as done.
hugoeg added a comment.

nit comment fixed


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDependenciesCheck.cpp
  clang-tidy/abseil/NoInternalDependenciesCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/external-file.h
  test/clang-tidy/Inputs/absl/strings/internal-file.h
  test/clang-tidy/abseil-no-internal-dependencies.cpp

Index: test/clang-tidy/abseil-no-internal-dependencies.cpp
===
--- test/clang-tidy/abseil-no-internal-dependencies.cpp
+++ test/clang-tidy/abseil-no-internal-dependencies.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-dependencies %t,  -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-internal-dependencies' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:6:24: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil [abseil-no-internal-dependencies]
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+void OpeningNamespace() {
+  strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+SomeContainer b;
+std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/internal-file.h
===
--- test/clang-tidy/Inputs/absl/strings/internal-file.h
+++ test/clang-tidy/Inputs/absl/strings/internal-file.h
@@ -1 +1,33 @@
-namespace absl {}
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/external-file.h
===
--- test/clang-tidy/Inputs/absl/external-file.h
+++ test/clang-tidy/Inputs/absl/external-file.h
@@ -1 +1,6 @@
-namespace absl {}
+namespace absl {
+namespace base_internal {
+void InternalFunction() {}
+} // namespace base_internal 
+} //namespace absl
+void DirectAccess2() { absl::base_internal::InternalFunction(); }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -6,6 +6,7 @@
 .. toctree::
abseil-duration-division
abseil-faster-strsplit-delimiter
+   abseil-no-internal-dependencies
abseil-no-namespace
abseil-string-find-startswith
android-cloexec-accept
Index: docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
+++ docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
@@ -0,0 +1,24 @@
+subl.. title:: clang-tidy - abseil-no-internal-dependencies
+
+abseil-no-internal-dependencies
+===
+
+Warns if code using Abseil depends on internal details. If something is in a
+namespace that includ

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

In https://reviews.llvm.org/D50542#1217515, @JonasToth wrote:

> In https://reviews.llvm.org/D50542#1217511, @hugoeg wrote:
>
> > nit comment fixed
>
>
> DO you have commit rights or shall i commit for you?


I do not have commit rights so I would appreciate it if you would commit me. 
Thanks!


https://reviews.llvm.org/D50542



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

In https://reviews.llvm.org/D50542#1217517, @JonasToth wrote:

> Committed r340928. Thanks very much!
>
> Will you (and your team) upstream even more abseil checks? I think it might 
> be reasonable to ask for commit rights.


We will likely not be up streaming more in the near future. Possibly one or 
two. I am also an intern really close to being done, so it will probably not be 
worth applying for commit rights. Thank you though for all the help.


Repository:
  rL LLVM

https://reviews.llvm.org/D50542



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits