[PATCH] D50389: [clang-tidy] new check for Abseil
deannagarcia updated this revision to Diff 159537.
https://reviews.llvm.org/D50389
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/DurationDivisionCheck.cpp
clang-tidy/abseil/DurationDivisionCheck.h
docs/clang-tidy/checks/abseil-duration-division.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-duration-division.cpp
Index: test/clang-tidy/abseil-duration-division.cpp
===
--- test/clang-tidy/abseil-duration-division.cpp
+++ test/clang-tidy/abseil-duration-division.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s abseil-duration-division %t
+
+namespace absl {
+
+class Duration {};
+
+int operator/(Duration lhs, Duration rhs);
+
+double FDivDuration(Duration num, Duration den);
+
+} // namespace absl
+
+void TakesInt(int);
+void TakesDouble(double);
+
+absl::Duration d;
+
+#define MACRO_EQ(x, y) (x == y)
+#define MACRO_DIVEQ(x,y,z) (x/y == z)
+#define CHECK(x) (x)
+
+void Positives() {
+ const double num = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division]
+ // CHECK-FIXES: const double num = absl::FDivDuration(d, d);
+ if (MACRO_EQ(d/d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on Duration objects
+ // CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {}
+ if (CHECK(MACRO_EQ(d/d, 0.0))) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on Duration objects
+ // CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {}
+
+ // This one generates a message, but no fix.
+ if (MACRO_DIVEQ(d, d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on Duration objects
+ // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {}
+}
+
+void Negatives() {
+ const int num = d/d;
+
+ // Explicit cast should disable the warning.
+ const double num_d = static_cast(d/d);
+}
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-duration-division
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-duration-division.rst
===
--- docs/clang-tidy/checks/abseil-duration-division.rst
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+
+abseil-duration-division
+
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d); // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1); // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds(1)); // GOOD: No truncation.
+ double dsec2 = absl::ToDoubleSeconds(d); // GOOD: No truncation.
+ assert(dsec1 == 3.5 && dsec2 == 3.5);
+
+
+This check looks for uses of ``absl::Duration`` division that is done in a
+floating-point context, and recommends the use of a function that returns a
+floating-point value.
Index: clang-tidy/abseil/DurationDivisionCheck.h
===
--- clang-tidy/abseil/DurationDivisionCheck.h
+++ clang-tidy/abseil/DurationDivisionCheck.h
@@ -0,0 +1,31 @@
+//===--- DurationDivisionCheck.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_DURATIONDIVISIONCHECK_H_
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_DURATIONDIVISIONCHECK_H_
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+// Find potential incorrect uses of integer division of absl::Duration objects.
+class DurationDivisionCheck : public ClangTidyCheck {
+public:
+ using ClangTidyCheck::ClangTidyCheck;
+ void registerMatchers(ast_matchers::MatchFinder *finder) override;
+ void check(const ast_matchers::MatchFi
[PATCH] D50389: [clang-tidy] new check for Abseil
deannagarcia updated this revision to Diff 159550.
https://reviews.llvm.org/D50389
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/DurationDivisionCheck.cpp
clang-tidy/abseil/DurationDivisionCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-division.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-duration-division.cpp
Index: test/clang-tidy/abseil-duration-division.cpp
===
--- test/clang-tidy/abseil-duration-division.cpp
+++ test/clang-tidy/abseil-duration-division.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s abseil-duration-division %t
+
+namespace absl {
+
+class Duration {};
+
+int operator/(Duration lhs, Duration rhs);
+
+double FDivDuration(Duration num, Duration den);
+
+} // namespace absl
+
+void TakesInt(int);
+void TakesDouble(double);
+
+absl::Duration d;
+
+#define MACRO_EQ(x, y) (x == y)
+#define MACRO_DIVEQ(x,y,z) (x/y == z)
+#define CHECK(x) (x)
+
+void Positives() {
+ const double num = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division]
+ // CHECK-FIXES: const double num = absl::FDivDuration(d, d);
+ if (MACRO_EQ(d/d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on Duration objects
+ // CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {}
+ if (CHECK(MACRO_EQ(d/d, 0.0))) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on Duration objects
+ // CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {}
+
+ // This one generates a message, but no fix.
+ if (MACRO_DIVEQ(d, d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on Duration objects
+ // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {}
+}
+
+void Negatives() {
+ const int num = d/d;
+
+ // Explicit cast should disable the warning.
+ const double num_d = static_cast(d/d);
+}
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-duration-division
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-duration-division.rst
===
--- docs/clang-tidy/checks/abseil-duration-division.rst
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+
+abseil-duration-division
+
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d); // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1); // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds(1)); // GOOD: No truncation.
+ double dsec2 = absl::ToDoubleSeconds(d); // GOOD: No truncation.
+ assert(dsec1 == 3.5 && dsec2 == 3.5);
+
+
+This check looks for uses of ``absl::Duration`` division that is done in a
+floating-point context, and recommends the use of a function that returns a
+floating-point value.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -56,6 +56,12 @@
Improvements to clang-tidy
--
+- New :doc:``abseil-duration-division
+ `` check.
+
+ Checks for uses of ``absl::Duration`` division that is done in a
+ floating-point context, and recommends the use of a function that
+ returns a floating-point value.
The improvements are...
Index: clang-tidy/abseil/DurationDivisionCheck.h
===
--- clang-tidy/abseil/DurationDivisionCheck.h
+++ clang-tidy/abseil/DurationDivisionCheck.h
@@ -0,0 +1,31 @@
+//===--- DurationDivisionCheck.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_
[PATCH] D50389: [clang-tidy] new check for Abseil
deannagarcia updated this revision to Diff 159590.
https://reviews.llvm.org/D50389
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/DurationDivisionCheck.cpp
clang-tidy/abseil/DurationDivisionCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-division.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-duration-division.cpp
Index: test/clang-tidy/abseil-duration-division.cpp
===
--- test/clang-tidy/abseil-duration-division.cpp
+++ test/clang-tidy/abseil-duration-division.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s abseil-duration-division %t
+
+namespace absl {
+
+class Duration {};
+
+int operator/(Duration lhs, Duration rhs);
+
+double FDivDuration(Duration num, Duration den);
+
+} // namespace absl
+
+void TakesInt(int);
+void TakesDouble(double);
+
+absl::Duration d;
+
+#define MACRO_EQ(x, y) (x == y)
+#define MACRO_DIVEQ(x,y,z) (x/y == z)
+#define CHECK(x) (x)
+
+void Positives() {
+ const double num = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division]
+ // CHECK-FIXES: const double num = absl::FDivDuration(d, d);
+ if (MACRO_EQ(d/d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on Duration objects
+ // CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {}
+ if (CHECK(MACRO_EQ(d/d, 0.0))) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on Duration objects
+ // CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {}
+
+ // This one generates a message, but no fix.
+ if (MACRO_DIVEQ(d, d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on Duration objects
+ // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {}
+}
+
+void Negatives() {
+ const int num = d/d;
+
+ // Explicit cast should disable the warning.
+ const double num_d = static_cast(d/d);
+}
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-duration-division
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-duration-division.rst
===
--- docs/clang-tidy/checks/abseil-duration-division.rst
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+
+abseil-duration-division
+
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d); // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1); // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds(1)); // GOOD: No truncation.
+ double dsec2 = absl::ToDoubleSeconds(d); // GOOD: No truncation.
+ assert(dsec1 == 3.5 && dsec2 == 3.5);
+
+
+This check looks for uses of ``absl::Duration`` division that is done in a
+floating-point context, and recommends the use of a function that returns a
+floating-point value.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -56,6 +56,12 @@
Improvements to clang-tidy
--
+- New :doc:`abseil-duration-division
+ ` check.
+
+ Checks for uses of ``absl::Duration`` division that is done in a
+ floating-point context, and recommends the use of a function that
+ returns a floating-point value.
The improvements are...
Index: clang-tidy/abseil/DurationDivisionCheck.h
===
--- clang-tidy/abseil/DurationDivisionCheck.h
+++ clang-tidy/abseil/DurationDivisionCheck.h
@@ -0,0 +1,31 @@
+//===--- DurationDivisionCheck.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_CL
[PATCH] D50389: [clang-tidy] new check for Abseil
deannagarcia updated this revision to Diff 160013.
Herald added a subscriber: mgorny.
https://reviews.llvm.org/D50389
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationDivisionCheck.cpp
clang-tidy/abseil/DurationDivisionCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-division.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-duration-division.cpp
Index: test/clang-tidy/abseil-duration-division.cpp
===
--- test/clang-tidy/abseil-duration-division.cpp
+++ test/clang-tidy/abseil-duration-division.cpp
@@ -0,0 +1,75 @@
+// RUN: %check_clang_tidy %s abseil-duration-division %t
+
+namespace absl {
+
+class Duration {};
+
+int operator/(Duration lhs, Duration rhs);
+
+double FDivDuration(Duration num, Duration den);
+
+} // namespace absl
+
+void TakesInt(int);
+void TakesDouble(double);
+template
+void TakesGeneric(T);
+
+absl::Duration d;
+
+#define MACRO_EQ(x, y) (x == y)
+#define MACRO_DIVEQ(x,y,z) (x/y == z)
+#define CHECK(x) (x)
+
+
+void Positives() {
+ const double num_double = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:30: warning: operator/ on absl::Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division]
+ // CHECK-FIXES: const double num_double = absl::FDivDuration(d, d);
+ const float num_float = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:28: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const float num_float = absl::FDivDuration(d, d);
+ const auto SomeVal = 1.0 + d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:31: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const auto SomeVal = 1.0 + absl::FDivDuration(d, d);
+ if (MACRO_EQ(d/d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {}
+ if (CHECK(MACRO_EQ(d/d, 0.0))) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {}
+
+ // This one generates a message, but no fix.
+ if (MACRO_DIVEQ(d, d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {}
+
+ TakesDouble(d/d);
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: TakesDouble(absl::FDivDuration(d, d));
+}
+
+template
+double DoubleDivision(T t1, T t2) {return t1/t2;}
+
+void AnotherPositive() {
+ DoubleDivision(d, d);
+ // CHECK-MESSAGES: [[@LINE-4]]:45: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: double DoubleDivision(T t1, T t2) {return
+ // absl::FDivDuration(t1, t2);}
+}
+
+void Negatives() {
+ const int num_int = d/d;
+ const long num_long = d/d;
+ const short num_short = d/d;
+ const char num_char = d/d;
+ const auto num_auto = d/d;
+ const auto SomeVal = 1 + d/d;
+
+ TakesInt(d/d);
+ TakesGeneric(d/d);
+ // Explicit cast should disable the warning.
+ const double num_cast1 = static_cast(d/d);
+ const double num_cast2 = (double)(d/d);
+}
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-duration-division
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-duration-division.rst
===
--- docs/clang-tidy/checks/abseil-duration-division.rst
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+
+abseil-duration-division
+
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0. See `this link `_ for more information on how ``absl::Duration`` arithmetic functions.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d); // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1); // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds(1)
[PATCH] D50389: [clang-tidy] new check for Abseil
deannagarcia marked 9 inline comments as done. deannagarcia added inline comments. Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:32 + hasImplicitDestinationType(qualType(unless(isInteger(, + unless(hasParent(cxxStaticCastExpr(, + this); JonasToth wrote: > What about different kinds of casts, like C-Style casts? > Doesn't the `hasImplicitDestinationType` already remove the possibility for > an cast as destination? I know the test fails without this line and flags the casts, so I'm pretty sure it's necessary but I'm not exactly sure why hasImplicitDestinationType doesn't catch it. Comment at: docs/clang-tidy/checks/abseil-duration-division.rst:8 +division of two `absl::Duration` objects returns an `int64` with any fractional +component truncated toward 0. + JonasToth wrote: > Please add one more sentence, why this is something you don't want, so it > gets clear that floating point contextes are the interesting here. Does this link work or do you still want more? Comment at: docs/clang-tidy/checks/abseil-duration-division.rst:3 + +abseil-duration-division + hokein wrote: > This is a nice document. Does abseil have similar thing in its official > guideline/practice? If yes, we can add the link in the document as well. I linked our general documentation on absl::Duration, specifically the stuff on duration arithmetic. Is that what you wanted? https://reviews.llvm.org/D50389 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50389: [clang-tidy] new check for Abseil
deannagarcia updated this revision to Diff 160096.
deannagarcia marked 7 inline comments as done.
https://reviews.llvm.org/D50389
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationDivisionCheck.cpp
clang-tidy/abseil/DurationDivisionCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-division.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-duration-division.cpp
Index: test/clang-tidy/abseil-duration-division.cpp
===
--- test/clang-tidy/abseil-duration-division.cpp
+++ test/clang-tidy/abseil-duration-division.cpp
@@ -0,0 +1,75 @@
+// RUN: %check_clang_tidy %s abseil-duration-division %t
+
+namespace absl {
+
+class Duration {};
+
+int operator/(Duration lhs, Duration rhs);
+
+double FDivDuration(Duration num, Duration den);
+
+} // namespace absl
+
+void TakesInt(int);
+void TakesDouble(double);
+template
+void TakesGeneric(T);
+
+absl::Duration d;
+
+#define MACRO_EQ(x, y) (x == y)
+#define MACRO_DIVEQ(x,y,z) (x/y == z)
+#define CHECK(x) (x)
+
+
+void Positives() {
+ const double num_double = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:30: warning: operator/ on absl::Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division]
+ // CHECK-FIXES: const double num_double = absl::FDivDuration(d, d);
+ const float num_float = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:28: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const float num_float = absl::FDivDuration(d, d);
+ const auto SomeVal = 1.0 + d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:31: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const auto SomeVal = 1.0 + absl::FDivDuration(d, d);
+ if (MACRO_EQ(d/d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {}
+ if (CHECK(MACRO_EQ(d/d, 0.0))) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {}
+
+ // This one generates a message, but no fix.
+ if (MACRO_DIVEQ(d, d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {}
+
+ TakesDouble(d/d);
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: TakesDouble(absl::FDivDuration(d, d));
+}
+
+template
+double DoubleDivision(T t1, T t2) {return t1/t2;}
+
+void AnotherPositive() {
+ DoubleDivision(d, d);
+ // CHECK-MESSAGES: [[@LINE-4]]:45: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: double DoubleDivision(T t1, T t2) {return
+ // absl::FDivDuration(t1, t2);}
+}
+
+void Negatives() {
+ const int num_int = d/d;
+ const long num_long = d/d;
+ const short num_short = d/d;
+ const char num_char = d/d;
+ const auto num_auto = d/d;
+ const auto SomeVal = 1 + d/d;
+
+ TakesInt(d/d);
+ TakesGeneric(d/d);
+ // Explicit cast should disable the warning.
+ const double num_cast1 = static_cast(d/d);
+ const double num_cast2 = (double)(d/d);
+}
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-duration-division
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-duration-division.rst
===
--- docs/clang-tidy/checks/abseil-duration-division.rst
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+
+abseil-duration-division
+
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0. See `this link `_ for more information on arithmetic with ``absl::Duration``.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d); // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1); // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds
[PATCH] D50389: [clang-tidy] new check for Abseil
deannagarcia updated this revision to Diff 160109.
deannagarcia marked 3 inline comments as done.
https://reviews.llvm.org/D50389
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationDivisionCheck.cpp
clang-tidy/abseil/DurationDivisionCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-division.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-duration-division.cpp
Index: test/clang-tidy/abseil-duration-division.cpp
===
--- test/clang-tidy/abseil-duration-division.cpp
+++ test/clang-tidy/abseil-duration-division.cpp
@@ -0,0 +1,75 @@
+// RUN: %check_clang_tidy %s abseil-duration-division %t
+
+namespace absl {
+
+class Duration {};
+
+int operator/(Duration lhs, Duration rhs);
+
+double FDivDuration(Duration num, Duration den);
+
+} // namespace absl
+
+void TakesInt(int);
+void TakesDouble(double);
+template
+void TakesGeneric(T);
+
+absl::Duration d;
+
+#define MACRO_EQ(x, y) (x == y)
+#define MACRO_DIVEQ(x,y,z) (x/y == z)
+#define CHECK(x) (x)
+
+
+void Positives() {
+ const double num_double = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:30: warning: operator/ on absl::Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division]
+ // CHECK-FIXES: const double num_double = absl::FDivDuration(d, d);
+ const float num_float = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:28: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const float num_float = absl::FDivDuration(d, d);
+ const auto SomeVal = 1.0 + d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:31: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const auto SomeVal = 1.0 + absl::FDivDuration(d, d);
+ if (MACRO_EQ(d/d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {}
+ if (CHECK(MACRO_EQ(d/d, 0.0))) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {}
+
+ // This one generates a message, but no fix.
+ if (MACRO_DIVEQ(d, d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {}
+
+ TakesDouble(d/d);
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: TakesDouble(absl::FDivDuration(d, d));
+}
+
+template
+double DoubleDivision(T t1, T t2) {return t1/t2;}
+
+void AnotherPositive() {
+ DoubleDivision(d, d);
+ // CHECK-MESSAGES: [[@LINE-4]]:45: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: double DoubleDivision(T t1, T t2) {return
+ // absl::FDivDuration(t1, t2);}
+}
+
+void Negatives() {
+ const int num_int = d/d;
+ const long num_long = d/d;
+ const short num_short = d/d;
+ const char num_char = d/d;
+ const auto num_auto = d/d;
+ const auto SomeVal = 1 + d/d;
+
+ TakesInt(d/d);
+ TakesGeneric(d/d);
+ // Explicit cast should disable the warning.
+ const double num_cast1 = static_cast(d/d);
+ const double num_cast2 = (double)(d/d);
+}
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-duration-division
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-duration-division.rst
===
--- docs/clang-tidy/checks/abseil-duration-division.rst
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+
+abseil-duration-division
+
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0. See `this link `_ for more information on arithmetic with ``absl::Duration``.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d); // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1); // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds
[PATCH] D50389: [clang-tidy] new check for Abseil
deannagarcia added inline comments. Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:50 + *result.SourceManager, result.Context->getLangOpts()), + ")"); +} JonasToth wrote: > This line looks odd, does it come from clang-format? I have run clang-format on this file and it's still formatted like this. https://reviews.llvm.org/D50389 ___ 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
deannagarcia created this revision.
deannagarcia added reviewers: alexfh, hokein.
deannagarcia added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, mgorny.
This check ensures that users of Abseil do not open namespace absl in their
code, as that violates our compatibility guidelines.
We are aware that this test will cause warnings on users code through their
dependencies on abseil. However, from what we know it seems like these warnings
are normally suppressed. If anyone has a good idea on how to avoid this/has
insight on whether this will be a problem for the average user, please let me
know!
https://reviews.llvm.org/D50580
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/NoNamespaceCheck.cpp
clang-tidy/abseil/NoNamespaceCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
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
@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s abseil-no-namespace %t
+
+namespace absl {}
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: namespace 'absl' is reserved for
+// implementation of the Abseil library and should not be opened in the user
+// code [abseil-no-namespace]
+
+namespace absl {
+int i = 5;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: namespace 'absl' is reserved for
+// implementation of the Abseil library and should not be opened in the user
+// code
+
+// Things that shouldn't trigger the check
+int i = 5;
+namespace std {}
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-namespace
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -59,6 +59,12 @@
The improvements are...
+- New :doc:`abseil-no-namespace
+ ` check.
+
+ Checks to ensure user did not open namespace absl as that
+ violates abseil's compatibility guidelines.
+
Improvements to include-fixer
-
Index: clang-tidy/abseil/NoNamespaceCheck.h
===
--- clang-tidy/abseil/NoNamespaceCheck.h
+++ clang-tidy/abseil/NoNamespaceCheck.h
@@ -0,0 +1,37 @@
+//===--- NoNamespaceCheck.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_NONAMESPACECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// This test ensures users don't open namespace absl, as that violates
+/// our compatibility guidelines.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-no-namespace.html
+class NoNamespaceCheck : public ClangTidyCheck {
+ public:
+ NoNamespaceCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H
+
Index: clang-tidy/abseil/NoNamespaceCheck.cpp
===
--- clang-tidy/abseil/NoNamespaceCheck.cpp
+++ clang-tidy/abseil/NoNamespaceCheck.cpp
@@ -0,0 +1,38 @@
+//===--- NoNamespaceCheck.cpp - clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "NoNamespaceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+void NoNamespaceCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus) return;
+
+ Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);
+}
+
+void NoNamespaceCheck::check(const MatchFinder::MatchResult &R
[PATCH] D50389: [clang-tidy] Abseil: integral division of Duration check
deannagarcia added inline comments.
Comment at: test/clang-tidy/abseil-duration-division.cpp:58
+ // CHECK-MESSAGES: [[@LINE-4]]:45: warning: operator/ on absl::Duration
objects
+ // CHECK-FIXES: double DoubleDivision(T t1, T t2) {return
+ // absl::FDivDuration(t1, t2);}
hokein wrote:
> I think we should ignore templates. The template function could be used by
> other types, fixing the template is not correct.
I removed this template function, but left the template function TakesGeneric
below to show that the automatic type does not require/give a warning. Is that
alright?
https://reviews.llvm.org/D50389
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50389: [clang-tidy] Abseil: integral division of Duration check
deannagarcia updated this revision to Diff 160336.
deannagarcia marked 6 inline comments as done.
https://reviews.llvm.org/D50389
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationDivisionCheck.cpp
clang-tidy/abseil/DurationDivisionCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-division.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-duration-division.cpp
Index: test/clang-tidy/abseil-duration-division.cpp
===
--- test/clang-tidy/abseil-duration-division.cpp
+++ test/clang-tidy/abseil-duration-division.cpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s abseil-duration-division %t
+
+namespace absl {
+
+class Duration {};
+
+int operator/(Duration lhs, Duration rhs);
+
+double FDivDuration(Duration num, Duration den);
+
+} // namespace absl
+
+void TakesInt(int);
+
+#define MACRO_EQ(x, y) (x == y)
+#define MACRO_DIVEQ(x,y,z) (x/y == z)
+#define CHECK(x) (x)
+
+void Positives() {
+ absl::Duration d;
+
+ const double num_double = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:30: warning: operator/ on absl::Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division]
+ // CHECK-FIXES: const double num_double = absl::FDivDuration(d, d);
+ const float num_float = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:28: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const float num_float = absl::FDivDuration(d, d);
+ const auto SomeVal = 1.0 + d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:31: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const auto SomeVal = 1.0 + absl::FDivDuration(d, d);
+ if (MACRO_EQ(d/d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {}
+ if (CHECK(MACRO_EQ(d/d, 0.0))) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {}
+
+ // This one generates a message, but no fix.
+ if (MACRO_DIVEQ(d, d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {}
+
+ TakesDouble(d/d);
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: TakesDouble(absl::FDivDuration(d, d));
+}
+
+void TakesDouble(double);
+template
+void TakesGeneric(T);
+
+void Negatives() {
+ absl::Duration d;
+ const int num_int = d/d;
+ const long num_long = d/d;
+ const short num_short = d/d;
+ const char num_char = d/d;
+ const auto num_auto = d/d;
+ const auto SomeVal = 1 + d/d;
+
+ TakesInt(d/d);
+ TakesGeneric(d/d);
+ // Explicit cast should disable the warning.
+ const double num_cast1 = static_cast(d/d);
+ const double num_cast2 = (double)(d/d);
+}
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-duration-division
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-duration-division.rst
===
--- docs/clang-tidy/checks/abseil-duration-division.rst
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+
+abseil-duration-division
+
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0. See `this link `_ for more information on arithmetic with ``absl::Duration``.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d); // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1); // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds(1)); // GOOD: No truncation.
+ double dsec2 = absl::ToDoubleSeconds(d); // GOOD: No truncation.
+ assert(dsec1 == 3.5 && dsec2 == 3.5);
+
+
+This check looks for uses of ``absl::Duration`` division that is done in a
+floating-point context, and recommends th
[PATCH] D50389: [clang-tidy] Abseil: integral division of Duration check
deannagarcia updated this revision to Diff 160338.
https://reviews.llvm.org/D50389
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationDivisionCheck.cpp
clang-tidy/abseil/DurationDivisionCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-division.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-duration-division.cpp
Index: test/clang-tidy/abseil-duration-division.cpp
===
--- test/clang-tidy/abseil-duration-division.cpp
+++ test/clang-tidy/abseil-duration-division.cpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s abseil-duration-division %t
+
+namespace absl {
+
+class Duration {};
+
+int operator/(Duration lhs, Duration rhs);
+
+double FDivDuration(Duration num, Duration den);
+
+} // namespace absl
+
+void TakesDouble(double);
+
+#define MACRO_EQ(x, y) (x == y)
+#define MACRO_DIVEQ(x,y,z) (x/y == z)
+#define CHECK(x) (x)
+
+void Positives() {
+ absl::Duration d;
+
+ const double num_double = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:30: warning: operator/ on absl::Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division]
+ // CHECK-FIXES: const double num_double = absl::FDivDuration(d, d);
+ const float num_float = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:28: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const float num_float = absl::FDivDuration(d, d);
+ const auto SomeVal = 1.0 + d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:31: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const auto SomeVal = 1.0 + absl::FDivDuration(d, d);
+ if (MACRO_EQ(d/d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {}
+ if (CHECK(MACRO_EQ(d/d, 0.0))) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {}
+
+ // This one generates a message, but no fix.
+ if (MACRO_DIVEQ(d, d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {}
+
+ TakesDouble(d/d);
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: TakesDouble(absl::FDivDuration(d, d));
+}
+
+void TakesInt(int);
+template
+void TakesGeneric(T);
+
+void Negatives() {
+ absl::Duration d;
+ const int num_int = d/d;
+ const long num_long = d/d;
+ const short num_short = d/d;
+ const char num_char = d/d;
+ const auto num_auto = d/d;
+ const auto SomeVal = 1 + d/d;
+
+ TakesInt(d/d);
+ TakesGeneric(d/d);
+ // Explicit cast should disable the warning.
+ const double num_cast1 = static_cast(d/d);
+ const double num_cast2 = (double)(d/d);
+}
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-duration-division
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-duration-division.rst
===
--- docs/clang-tidy/checks/abseil-duration-division.rst
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+
+abseil-duration-division
+
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0. See `this link `_ for more information on arithmetic with ``absl::Duration``.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d); // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1); // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds(1)); // GOOD: No truncation.
+ double dsec2 = absl::ToDoubleSeconds(d); // GOOD: No truncation.
+ assert(dsec1 == 3.5 && dsec2 == 3.5);
+
+
+This check looks for uses of ``absl::Duration`` division that is done in a
+floating-point context, and recommends the use of a function that returns a
+floating-po
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
deannagarcia updated this revision to Diff 160399.
deannagarcia marked 7 inline comments as done.
https://reviews.llvm.org/D50580
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/NoNamespaceCheck.cpp
clang-tidy/abseil/NoNamespaceCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-no-namespace.rst
docs/clang-tidy/checks/list.rst
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
@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s abseil-no-namespace %t
+
+namespace absl {}
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: namespace 'absl' is reserved for
+// implementation of the Abseil library and should not be opened in the user
+// code [abseil-no-namespace]
+
+namespace absl {
+int i = 5;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: namespace 'absl' is reserved for
+// implementation of the Abseil library and should not be opened in the user
+// code
+
+// Things that shouldn't trigger the check
+int i = 5;
+namespace std {}
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-namespace
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-namespace.rst
===
--- docs/clang-tidy/checks/abseil-no-namespace.rst
+++ docs/clang-tidy/checks/abseil-no-namespace.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - abseil-no-namespace
+
+abseil-no-namespace
+===
+
+This check gives users a warning if they attempt to open
+``namespace absl``. Users should not open ``namespace absl``
+as that conflicts with abseil's compatibility guidelines and
+may result in breakage.
+
+Any user that uses:
+
+.. code-block:: c++
+
+ namespace absl {
+ ...
+ }
+
+will be prompted with a warning.
+
+See `the full Abseil compatibility guidelines ` for more information.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,7 +57,11 @@
Improvements to clang-tidy
--
-The improvements are...
+- New :doc:`abseil-no-namespace
+ ` check.
+
+ Checks to ensure code does not open `namespace absl` as that
+ violates Abseil's compatibility guidelines.
Improvements to include-fixer
-
Index: clang-tidy/abseil/NoNamespaceCheck.h
===
--- clang-tidy/abseil/NoNamespaceCheck.h
+++ clang-tidy/abseil/NoNamespaceCheck.h
@@ -0,0 +1,36 @@
+//===--- NoNamespaceCheck.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_NONAMESPACECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// This test ensures users don't open namespace absl, as that violates
+/// our compatibility guidelines.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-no-namespace.html
+class NoNamespaceCheck : public ClangTidyCheck {
+public:
+ NoNamespaceCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H
Index: clang-tidy/abseil/NoNamespaceCheck.cpp
===
--- clang-tidy/abseil/NoNamespaceCheck.cpp
+++ clang-tidy/abseil/NoNamespaceCheck.cpp
@@ -0,0 +1,39 @@
+//===--- NoNamespaceCheck.cpp - clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "NoNamespaceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
deannagarcia added inline comments.
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23
+
+ Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);
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.
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] D50389: [clang-tidy] Abseil: integral division of Duration check
deannagarcia updated this revision to Diff 160582.
https://reviews.llvm.org/D50389
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationDivisionCheck.cpp
clang-tidy/abseil/DurationDivisionCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-division.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-duration-division.cpp
Index: test/clang-tidy/abseil-duration-division.cpp
===
--- test/clang-tidy/abseil-duration-division.cpp
+++ test/clang-tidy/abseil-duration-division.cpp
@@ -0,0 +1,75 @@
+// RUN: %check_clang_tidy %s abseil-duration-division %t
+
+namespace absl {
+
+class Duration {};
+
+int operator/(Duration lhs, Duration rhs);
+
+double FDivDuration(Duration num, Duration den);
+
+} // namespace absl
+
+void TakesDouble(double);
+
+#define MACRO_EQ(x, y) (x == y)
+#define MACRO_DIVEQ(x,y,z) (x/y == z)
+#define CHECK(x) (x)
+
+void Positives() {
+ absl::Duration d;
+
+ const double num_double = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:30: warning: operator/ on absl::Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division]
+ // CHECK-FIXES: const double num_double = absl::FDivDuration(d, d);
+ const float num_float = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:28: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const float num_float = absl::FDivDuration(d, d);
+ const auto SomeVal = 1.0 + d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:31: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const auto SomeVal = 1.0 + absl::FDivDuration(d, d);
+ if (MACRO_EQ(d/d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {}
+ if (CHECK(MACRO_EQ(d/d, 0.0))) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {}
+
+ // This one generates a message, but no fix.
+ if (MACRO_DIVEQ(d, d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {}
+
+ TakesDouble(d/d);
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: TakesDouble(absl::FDivDuration(d, d));
+}
+
+void TakesInt(int);
+template
+void TakesGeneric(T);
+
+void Negatives() {
+ absl::Duration d;
+ const int num_int = d/d;
+ const long num_long = d/d;
+ const short num_short = d/d;
+ const char num_char = d/d;
+ const auto num_auto = d/d;
+ const auto SomeVal = 1 + d/d;
+
+ TakesInt(d/d);
+ TakesGeneric(d/d);
+ // Explicit cast should disable the warning.
+ const double num_cast1 = static_cast(d/d);
+ const double num_cast2 = (double)(d/d);
+}
+
+template
+double DoubleDivision(T t1, T t2) {return t1/t2;}
+
+//This also won't trigger a warning
+void TemplateDivision() {
+ absl::Duration d;
+ DoubleDivision(d, d);
+}
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-duration-division
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-duration-division.rst
===
--- docs/clang-tidy/checks/abseil-duration-division.rst
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+
+abseil-duration-division
+
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0. See `this link `_ for more information on arithmetic with ``absl::Duration``.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d); // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1); // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds(1)); // GOOD: No truncation.
+ double dsec2 = absl::ToDoubleSeconds(d); // GOOD: No truncation.
+ assert(dsec1 == 3.5 && dsec2
[PATCH] D50389: [clang-tidy] Abseil: integral division of Duration check
deannagarcia added inline comments.
Comment at: test/clang-tidy/abseil-duration-division.cpp:58
+ // CHECK-MESSAGES: [[@LINE-4]]:45: warning: operator/ on absl::Duration
objects
+ // CHECK-FIXES: double DoubleDivision(T t1, T t2) {return
+ // absl::FDivDuration(t1, t2);}
hokein wrote:
> deannagarcia wrote:
> > hokein wrote:
> > > I think we should ignore templates. The template function could be used
> > > by other types, fixing the template is not correct.
> > I removed this template function, but left the template function
> > TakesGeneric below to show that the automatic type does not require/give a
> > warning. Is that alright?
> The check will still give warnings in the template instantiation, I think we
> need `unless(isInTemplateInstantiation()` matcher to filter them out.
I added that and then put the test back to show that it now won't trigger a
warning. Is that what you wanted?
https://reviews.llvm.org/D50389
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check
deannagarcia created this revision.
deannagarcia added reviewers: hokein, alexfh.
deannagarcia added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, mgorny.
This check is an abseil specific check that checks for code using single
character string literals as delimiters and transforms the code into characters.
https://reviews.llvm.org/D50862
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
clang-tidy/abseil/FasterStrsplitDelimiterCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
Index: test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
===
--- test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
+++ test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
@@ -0,0 +1,95 @@
+// RUN: %check_clang_tidy %s abseil-faster-strsplit-delimiter %t
+
+namespace absl {
+
+class string_view {
+ public:
+string_view();
+string_view(const char *);
+};
+
+namespace strings_internal {
+struct Splitter {};
+struct MaxSplitsImpl {
+ MaxSplitsImpl();
+ ~MaxSplitsImpl();
+};
+} //namespace strings_internal
+
+template
+strings_internal::Splitter StrSplit(absl::string_view, Delim) {
+ return {};
+}
+template
+strings_internal::Splitter StrSplit(absl::string_view, Delim, Pred) {
+ return {};
+}
+
+class ByAnyChar {
+ public:
+explicit ByAnyChar(absl::string_view);
+~ByAnyChar();
+};
+
+template
+strings_internal::MaxSplitsImpl MaxSplits(Delim, int) {
+ return {};
+}
+
+} //namespace absl
+
+void SplitDelimiters() {
+ absl::StrSplit("ABC", "A");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()/ByAnyChar()/MaxSplits() called with a string literal consisting of a single character; consider using the more efficient overload accepting a character [abseil-faster-strsplit-delimiter]
+ // CHECK-FIXES: absl::StrSplit("ABC", 'A');
+
+ absl::StrSplit("ABC", absl::ByAnyChar("\n"));
+ // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::StrSplit()/ByAnyChar()
+ // CHECK-FIXES: absl::StrSplit("ABC", '\n');
+
+ // Works with predicate
+ absl::StrSplit("ABC", "A", [](absl::string_view) { return true; });
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()/ByAnyChar()
+ // CHECK-FIXES: absl::StrSplit("ABC", 'A', [](absl::string_view) { return true; });
+
+ // Doesn't do anything with other strings lenghts.
+ absl::StrSplit("ABC", "AB");
+ absl::StrSplit("ABC", absl::ByAnyChar(""));
+ absl::StrSplit("ABC", absl::ByAnyChar(" \t"));
+
+ // Escapes a single quote in the resulting character literal.
+ absl::StrSplit("ABC", "'");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()/ByAnyChar()
+ // CHECK-FIXES: absl::StrSplit("ABC", '\'');
+
+ absl::StrSplit("ABC", absl::MaxSplits("\t", 1));
+ // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::StrSplit()/ByAnyChar()
+ // CHECK-FIXES: absl::StrSplit("ABC", absl::MaxSplits('\t', 1));
+
+ auto delim = absl::MaxSplits(absl::ByAnyChar(" "), 1);
+ // CHECK-MESSAGES: [[@LINE-1]]:48: warning: absl::StrSplit()/ByAnyChar()
+ // CHECK-FIXES: auto delim = absl::MaxSplits(' ', 1);
+}
+
+#define MACRO(str) absl::StrSplit("ABC", str)
+
+void Macro() {
+ MACRO("A");
+}
+
+template
+void FunctionTemplate() {
+ // This one should not warn because ByAnyChar is a dependent type.
+ absl::StrSplit("TTT", T("A"));
+
+ // This one will warn, but we are checking that we get a correct warning only
+ // once.
+ absl::StrSplit("TTT", "A");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()/ByAnyChar()
+ // CHECK-FIXES: absl::StrSplit("TTT", 'A');
+}
+
+void FunctionTemplateCaller() {
+ FunctionTemplate();
+ FunctionTemplate();
+}
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-faster-strsplit-delimiter
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
===
--- docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
+++ docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-faster-strsplit-delimiter
+
+abseil-faster-strsplit-delimiter
+
+
+This check triggers on calls to ``absl::StrSplit()`` or ``absl::MaxSplits()``
+where the delimiter is a single character string literal. The check will offer
+a suggestion to change the string literal into a character. It will also catch
+when code uses ``absl::ByAnyChar()`` for just a single character and will
+transform that into a single charact
[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check
deannagarcia updated this revision to Diff 161309.
deannagarcia marked 10 inline comments as done.
https://reviews.llvm.org/D50862
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
clang-tidy/abseil/FasterStrsplitDelimiterCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
Index: test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
===
--- test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
+++ test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
@@ -0,0 +1,95 @@
+// RUN: %check_clang_tidy %s abseil-faster-strsplit-delimiter %t
+
+namespace absl {
+
+class string_view {
+ public:
+string_view();
+string_view(const char *);
+};
+
+namespace strings_internal {
+struct Splitter {};
+struct MaxSplitsImpl {
+ MaxSplitsImpl();
+ ~MaxSplitsImpl();
+};
+} //namespace strings_internal
+
+template
+strings_internal::Splitter StrSplit(absl::string_view, Delim) {
+ return {};
+}
+template
+strings_internal::Splitter StrSplit(absl::string_view, Delim, Pred) {
+ return {};
+}
+
+class ByAnyChar {
+ public:
+explicit ByAnyChar(absl::string_view);
+~ByAnyChar();
+};
+
+template
+strings_internal::MaxSplitsImpl MaxSplits(Delim, int) {
+ return {};
+}
+
+} //namespace absl
+
+void SplitDelimiters() {
+ absl::StrSplit("ABC", "A");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit() called with a string literal consisting of a single character; consider using the character overload [abseil-faster-strsplit-delimiter]
+ // CHECK-FIXES: absl::StrSplit("ABC", 'A');
+
+ absl::StrSplit("ABC", absl::ByAnyChar("\n"));
+ // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("ABC", '\n');
+
+ // Works with predicate
+ absl::StrSplit("ABC", "A", [](absl::string_view) { return true; });
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("ABC", 'A', [](absl::string_view) { return true; });
+
+ // Doesn't do anything with other strings lenghts.
+ absl::StrSplit("ABC", "AB");
+ absl::StrSplit("ABC", absl::ByAnyChar(""));
+ absl::StrSplit("ABC", absl::ByAnyChar(" \t"));
+
+ // Escapes a single quote in the resulting character literal.
+ absl::StrSplit("ABC", "'");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("ABC", '\'');
+
+ absl::StrSplit("ABC", absl::MaxSplits("\t", 1));
+ // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::MaxSplits()
+ // CHECK-FIXES: absl::StrSplit("ABC", absl::MaxSplits('\t', 1));
+
+ auto delim = absl::MaxSplits(absl::ByAnyChar(" "), 1);
+ // CHECK-MESSAGES: [[@LINE-1]]:48: warning: absl::MaxSplits()
+ // CHECK-FIXES: auto delim = absl::MaxSplits(' ', 1);
+}
+
+#define MACRO(str) absl::StrSplit("ABC", str)
+
+void Macro() {
+ MACRO("A");
+}
+
+template
+void FunctionTemplate() {
+ // This one should not warn because ByAnyChar is a dependent type.
+ absl::StrSplit("TTT", T("A"));
+
+ // This one will warn, but we are checking that we get a correct warning only
+ // once.
+ absl::StrSplit("TTT", "A");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("TTT", 'A');
+}
+
+void FunctionTemplateCaller() {
+ FunctionTemplate();
+ FunctionTemplate();
+}
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-faster-strsplit-delimiter
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
===
--- docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
+++ docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
@@ -0,0 +1,41 @@
+.. title:: clang-tidy - abseil-faster-strsplit-delimiter
+
+abseil-faster-strsplit-delimiter
+
+
+Finds instances of ``absl::StrSplit()`` or ``absl::MaxSplits()`` where the
+delimiter is a single character string literal and replaces with a character.
+The check will offer a suggestion to change the string literal into a character.
+It will also catch code using ``absl::ByAnyChar()`` for just a single character
+and will transform that into a single character as well.
+
+These changes will give the same result, but using characters rather than
+single character string literals is more efficient and readable.
+
+Examples:
+
+.. code-block:: c++
+
+ // Original - the argument is a string literal.
+ for (auto piece : absl::StrSplit(str, "B")) {
+
+ // Suggested - the argument is a character,
[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check
deannagarcia added inline comments.
Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:49
+ // in the character literal.
+ if (Result == R"("'")") {
+return std::string(R"('\'')");
JonasToth wrote:
> The comment suggest, that all single quotes need to be escaped and then
> further processing happens, but you check on identity to `'` and return the
> escaped version of it.
> That confuses me.
Does the new comment clear it up?
Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:54
+ // Now replace the " with '.
+ auto Pos = Result.find_first_of('"');
+ if (Pos == Result.npos)
JonasToth wrote:
> This expects at max 2 `"` characters. couldnt there be more?
The only way this will be run is if the string is a single character, so the
only possibility if there are more than 2 " characters is that the character
that is the delimiter is actually " which is taken care of in the check. Does
that make sense/do you think I need to add a comment about this?
Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:75
+
+ // string_view passed by value and contructed from string literal.
+ auto StringViewArg =
JonasToth wrote:
> What about the `std::string_view`?
This matcher is only here for the next matcher to check to see if
absl::ByAnyChar has been passed in a single character string view and is only
necessary because ByAnyChar accepts an absl::string_view so it isn't necessary
to make one for std::string_view
Comment at: test/clang-tidy/abseil-faster-strsplit-delimiter.cpp:42
+void SplitDelimiters() {
+ absl::StrSplit("ABC", "A");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning:
absl::StrSplit()/ByAnyChar()/MaxSplits() called with a string literal
consisting of a single character; consider using the more efficient overload
accepting a character [abseil-faster-strsplit-delimiter]
JonasToth wrote:
> Please add a test, where `"A"` is used as an arbitray function argument
> (similar to the template case, but without templates involved)
Can you explain this more/provide an example?
https://reviews.llvm.org/D50862
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check
deannagarcia updated this revision to Diff 161472.
deannagarcia marked 11 inline comments as done.
https://reviews.llvm.org/D50862
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
clang-tidy/abseil/FasterStrsplitDelimiterCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
Index: test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
===
--- test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
+++ test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
@@ -0,0 +1,95 @@
+// RUN: %check_clang_tidy %s abseil-faster-strsplit-delimiter %t
+
+namespace absl {
+
+class string_view {
+ public:
+string_view();
+string_view(const char *);
+};
+
+namespace strings_internal {
+struct Splitter {};
+struct MaxSplitsImpl {
+ MaxSplitsImpl();
+ ~MaxSplitsImpl();
+};
+} //namespace strings_internal
+
+template
+strings_internal::Splitter StrSplit(absl::string_view, Delim) {
+ return {};
+}
+template
+strings_internal::Splitter StrSplit(absl::string_view, Delim, Pred) {
+ return {};
+}
+
+class ByAnyChar {
+ public:
+explicit ByAnyChar(absl::string_view);
+~ByAnyChar();
+};
+
+template
+strings_internal::MaxSplitsImpl MaxSplits(Delim, int) {
+ return {};
+}
+
+} //namespace absl
+
+void SplitDelimiters() {
+ absl::StrSplit("ABC", "A");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit() called with a string literal consisting of a single character; consider using the character overload [abseil-faster-strsplit-delimiter]
+ // CHECK-FIXES: absl::StrSplit("ABC", 'A');
+
+ absl::StrSplit("ABC", absl::ByAnyChar("\n"));
+ // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("ABC", '\n');
+
+ // Works with predicate
+ absl::StrSplit("ABC", "A", [](absl::string_view) { return true; });
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("ABC", 'A', [](absl::string_view) { return true; });
+
+ // Doesn't do anything with other strings lenghts.
+ absl::StrSplit("ABC", "AB");
+ absl::StrSplit("ABC", absl::ByAnyChar(""));
+ absl::StrSplit("ABC", absl::ByAnyChar(" \t"));
+
+ // Escapes a single quote in the resulting character literal.
+ absl::StrSplit("ABC", "'");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("ABC", '\'');
+
+ absl::StrSplit("ABC", absl::MaxSplits("\t", 1));
+ // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::MaxSplits()
+ // CHECK-FIXES: absl::StrSplit("ABC", absl::MaxSplits('\t', 1));
+
+ auto delim = absl::MaxSplits(absl::ByAnyChar(" "), 1);
+ // CHECK-MESSAGES: [[@LINE-1]]:48: warning: absl::MaxSplits()
+ // CHECK-FIXES: auto delim = absl::MaxSplits(' ', 1);
+}
+
+#define MACRO(str) absl::StrSplit("ABC", str)
+
+void Macro() {
+ MACRO("A");
+}
+
+template
+void FunctionTemplate() {
+ // This one should not warn because ByAnyChar is a dependent type.
+ absl::StrSplit("TTT", T("A"));
+
+ // This one will warn, but we are checking that we get a correct warning only
+ // once.
+ absl::StrSplit("TTT", "A");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("TTT", 'A');
+}
+
+void FunctionTemplateCaller() {
+ FunctionTemplate();
+ FunctionTemplate();
+}
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-faster-strsplit-delimiter
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
===
--- docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
+++ docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
@@ -0,0 +1,41 @@
+.. title:: clang-tidy - abseil-faster-strsplit-delimiter
+
+abseil-faster-strsplit-delimiter
+
+
+Finds instances of ``absl::StrSplit()`` or ``absl::MaxSplits()`` where the
+delimiter is a single character string literal and replaces with a character.
+The check will offer a suggestion to change the string literal into a character.
+It will also catch code using ``absl::ByAnyChar()`` for just a single character
+and will transform that into a single character as well.
+
+These changes will give the same result, but using characters rather than
+single character string literals is more efficient and readable.
+
+Examples:
+
+.. code-block:: c++
+
+ // Original - the argument is a string literal.
+ for (auto piece : absl::StrSplit(str, "B")) {
+
+ // Suggested - the argument is a character,
[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check
deannagarcia added inline comments.
Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:54
+ // Now replace the " with '.
+ auto Pos = Result.find_first_of('"');
+ if (Pos == Result.npos)
JonasToth wrote:
> deannagarcia wrote:
> > JonasToth wrote:
> > > This expects at max 2 `"` characters. couldnt there be more?
> > The only way this will be run is if the string is a single character, so
> > the only possibility if there are more than 2 " characters is that the
> > character that is the delimiter is actually " which is taken care of in the
> > check. Does that make sense/do you think I need to add a comment about this?
> Ok I see, you could add an assertion before this section. Having the
> precondition checked is always a good thing and usually adds clarity as well
> :)
I can't think of a simple thing to assert because most literals will look like:
"a" but there is also the possibility that it looks like "\"" and I can't think
of an easy way to combine those two. Do you have an idea/maybe I could just put
a comment at the beginning saying this is only meant for single character
string literals?
https://reviews.llvm.org/D50862
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check
deannagarcia updated this revision to Diff 161513.
https://reviews.llvm.org/D50862
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
clang-tidy/abseil/FasterStrsplitDelimiterCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
Index: test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
===
--- test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
+++ test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
@@ -0,0 +1,99 @@
+// RUN: %check_clang_tidy %s abseil-faster-strsplit-delimiter %t
+
+namespace absl {
+
+class string_view {
+ public:
+string_view();
+string_view(const char *);
+};
+
+namespace strings_internal {
+struct Splitter {};
+struct MaxSplitsImpl {
+ MaxSplitsImpl();
+ ~MaxSplitsImpl();
+};
+} //namespace strings_internal
+
+template
+strings_internal::Splitter StrSplit(absl::string_view, Delim) {
+ return {};
+}
+template
+strings_internal::Splitter StrSplit(absl::string_view, Delim, Pred) {
+ return {};
+}
+
+class ByAnyChar {
+ public:
+explicit ByAnyChar(absl::string_view);
+~ByAnyChar();
+};
+
+template
+strings_internal::MaxSplitsImpl MaxSplits(Delim, int) {
+ return {};
+}
+
+} //namespace absl
+
+void SplitDelimiters() {
+ absl::StrSplit("ABC", "A");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit() called with a string literal consisting of a single character; consider using the character overload [abseil-faster-strsplit-delimiter]
+ // CHECK-FIXES: absl::StrSplit("ABC", 'A');
+
+ absl::StrSplit("ABC", absl::ByAnyChar("\n"));
+ // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("ABC", '\n');
+
+ // Works with predicate
+ absl::StrSplit("ABC", "A", [](absl::string_view) { return true; });
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("ABC", 'A', [](absl::string_view) { return true; });
+
+ // Doesn't do anything with other strings lenghts.
+ absl::StrSplit("ABC", "AB");
+ absl::StrSplit("ABC", absl::ByAnyChar(""));
+ absl::StrSplit("ABC", absl::ByAnyChar(" \t"));
+
+ // Escapes a single quote in the resulting character literal.
+ absl::StrSplit("ABC", "'");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("ABC", '\'');
+
+ absl::StrSplit("ABC", "\"");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("ABC", '\"');
+
+ absl::StrSplit("ABC", absl::MaxSplits("\t", 1));
+ // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::MaxSplits()
+ // CHECK-FIXES: absl::StrSplit("ABC", absl::MaxSplits('\t', 1));
+
+ auto delim = absl::MaxSplits(absl::ByAnyChar(" "), 1);
+ // CHECK-MESSAGES: [[@LINE-1]]:48: warning: absl::MaxSplits()
+ // CHECK-FIXES: auto delim = absl::MaxSplits(' ', 1);
+}
+
+#define MACRO(str) absl::StrSplit("ABC", str)
+
+void Macro() {
+ MACRO("A");
+}
+
+template
+void FunctionTemplate() {
+ // This one should not warn because ByAnyChar is a dependent type.
+ absl::StrSplit("TTT", T("A"));
+
+ // This one will warn, but we are checking that we get a correct warning only
+ // once.
+ absl::StrSplit("TTT", "A");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("TTT", 'A');
+}
+
+void FunctionTemplateCaller() {
+ FunctionTemplate();
+ FunctionTemplate();
+}
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-faster-strsplit-delimiter
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
===
--- docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
+++ docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
@@ -0,0 +1,41 @@
+.. title:: clang-tidy - abseil-faster-strsplit-delimiter
+
+abseil-faster-strsplit-delimiter
+
+
+Finds instances of ``absl::StrSplit()`` or ``absl::MaxSplits()`` where the
+delimiter is a single character string literal and replaces with a character.
+The check will offer a suggestion to change the string literal into a character.
+It will also catch code using ``absl::ByAnyChar()`` for just a single character
+and will transform that into a single character as well.
+
+These changes will give the same result, but using characters rather than
+single character string literals is more efficient and readable.
+
+Examples:
+
+.. code-block:: c++
+
+ // Original - the argument is a string literal.
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
deannagarcia updated this revision to Diff 161526.
deannagarcia edited the summary of this revision.
deannagarcia added a comment.
This revision includes a matcher so that the warning does not trigger on
internal Abseil files.
https://reviews.llvm.org/D50580
Files:
clang-tidy/abseil/AbseilMatcher.h
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/NoNamespaceCheck.cpp
clang-tidy/abseil/NoNamespaceCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-no-namespace.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-namespace.cpp
Index: test/clang-tidy/abseil-no-namespace.cpp
===
--- test/clang-tidy/abseil-no-namespace.cpp
+++ test/clang-tidy/abseil-no-namespace.cpp
@@ -0,0 +1,26 @@
+// RUN: %check_clang_tidy %s abseil-no-namespace %t -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-namespace' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+// Warning will not be triggered on internal Abseil code that is included.
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+// 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
+// for implementation of the Abseil library and should not be opened in
+// user code [abseil-no-namespace]
+
+namespace absl {}
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: namespace 'absl' is reserved for
+// implementation of the Abseil library and should not be opened in the user
+// code [abseil-no-namespace]
+
+namespace absl {
+int i = 5;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: namespace 'absl'
+
+// Things that shouldn't trigger the check
+int i = 5;
+namespace std {}
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 @@
+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 @@
+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-namespace
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-namespace.rst
===
--- docs/clang-tidy/checks/abseil-no-namespace.rst
+++ docs/clang-tidy/checks/abseil-no-namespace.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - abseil-no-namespace
+
+abseil-no-namespace
+===
+
+This check gives users a warning if they attempt to open ``namespace absl``.
+Users should not open ``namespace absl`` as that conflicts with abseil's
+compatibility guidelines and may result in breakage.
+
+Any user that uses:
+
+.. code-block:: c++
+
+ namespace absl {
+ ...
+ }
+
+will be prompted with a warning.
+
+See `the full Abseil compatibility guidelines `_ for more information.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,7 +57,11 @@
Improvements to clang-tidy
--
-The improvements are...
+- New :doc:`abseil-no-namespace
+ ` check.
+
+ Checks to ensure code does not open `namespace absl` as that
+ violates Abseil's compatibility guidelines.
Improvements to include-fixer
-
Index: clang-tidy/abseil/NoNamespaceCheck.h
===
--- clang-tidy/abseil/NoNamespaceCheck.h
+++ clang-tidy/abseil/NoNamespaceCheck.h
@@ -0,0 +1,36 @@
+//===--- NoNamespaceCheck.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_NONAMESPACECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// This test ensures users don't open namespace absl, as that violates
+/// our compatibility guidelines.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/chec
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
deannagarcia added inline comments.
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23
+
+ Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);
hokein wrote:
> JonasToth wrote:
> > hugoeg wrote:
> > > 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
> > In principle it is ok, but I don't think ASTMatchers.h is the correct
> > place, because it is only of use to clang-tidy.
> >
> > There is a `util` directory in clang-tidy for this kind of stuff. Defining
> > you own matchers works the same as in ASTMatchers, you can grep through
> > clang-tidy checks (e.g. `AST_MATCHER`) to have some examples of private
> > matchers.
> `ASTMatchers.h` is not a reasonable place to put `asseil`-specific matchers.
> We have `clang-tidy/utils/Matchers.h` for putting clang-tidy specific
> matchers. I'm not sure whether it is reasonable to put abseil-specific
> matchers there. Maybe we create a `AbseilMatcher.h` file in
> `clang-tidy/abseil` directory?
I put it in clang-tidy/abseil for now but I can definitely move it to
clang-tidy/utils/ if you would prefer that
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] D50580: [clang-tidy] Abseil: no namespace check
deannagarcia updated this revision to Diff 161720.
deannagarcia marked 12 inline comments as done.
https://reviews.llvm.org/D50580
Files:
clang-tidy/abseil/AbseilMatcher.h
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/NoNamespaceCheck.cpp
clang-tidy/abseil/NoNamespaceCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-no-namespace.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-namespace.cpp
Index: test/clang-tidy/abseil-no-namespace.cpp
===
--- test/clang-tidy/abseil-no-namespace.cpp
+++ test/clang-tidy/abseil-no-namespace.cpp
@@ -0,0 +1,26 @@
+// RUN: %check_clang_tidy %s abseil-no-namespace %t -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-namespace' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+// Warning will not be triggered on internal Abseil code that is included.
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+// 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
+// for implementation of the Abseil library and should not be opened in
+// user code [abseil-no-namespace]
+
+namespace absl {}
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: namespace 'absl' is reserved for
+// implementation of the Abseil library and should not be opened in the user
+// code [abseil-no-namespace]
+
+namespace absl {
+int i = 5;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: namespace 'absl'
+
+// Things that shouldn't trigger the check
+int i = 5;
+namespace std {}
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 @@
+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 @@
+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-namespace
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-namespace.rst
===
--- docs/clang-tidy/checks/abseil-no-namespace.rst
+++ docs/clang-tidy/checks/abseil-no-namespace.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - abseil-no-namespace
+
+abseil-no-namespace
+===
+
+This check gives users a warning if they attempt to open ``namespace absl``.
+Users should not open ``namespace absl`` as that conflicts with abseil's
+compatibility guidelines and may result in breakage.
+
+Any user that uses:
+
+.. code-block:: c++
+
+ namespace absl {
+ ...
+ }
+
+will be prompted with a warning.
+
+See `the full Abseil compatibility guidelines `_ for more information.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,7 +57,11 @@
Improvements to clang-tidy
--
-The improvements are...
+- New :doc:`abseil-no-namespace
+ ` check.
+
+ Checks to ensure code does not open `namespace absl` as that
+ violates Abseil's compatibility guidelines.
Improvements to include-fixer
-
Index: clang-tidy/abseil/NoNamespaceCheck.h
===
--- clang-tidy/abseil/NoNamespaceCheck.h
+++ clang-tidy/abseil/NoNamespaceCheck.h
@@ -0,0 +1,36 @@
+//===--- NoNamespaceCheck.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_NONAMESPACECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// This test ensures users don't open namespace absl, as that violates
+/// our compatibility guidelines.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-no-namespace.html
+class NoNamespaceCheck : public ClangTidyCheck {
+public:
+ NoNamespaceCheck(StringRef Name, ClangTid
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
deannagarcia 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)");
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.
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:
> 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.
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] D50580: [clang-tidy] Abseil: no namespace check
deannagarcia updated this revision to Diff 161827.
deannagarcia marked 11 inline comments as done.
https://reviews.llvm.org/D50580
Files:
clang-tidy/abseil/AbseilMatcher.h
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/NoNamespaceCheck.cpp
clang-tidy/abseil/NoNamespaceCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-no-namespace.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-namespace.cpp
Index: test/clang-tidy/abseil-no-namespace.cpp
===
--- test/clang-tidy/abseil-no-namespace.cpp
+++ test/clang-tidy/abseil-no-namespace.cpp
@@ -0,0 +1,28 @@
+// RUN: %check_clang_tidy %s abseil-no-namespace %t -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-namespace' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+/// Warning will not be triggered on internal Abseil code that is included.
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+/// 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
+// for implementation of the Abseil library and should not be opened in user
+// code [abseil-no-namespace]
+
+namespace absl {}
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: namespace 'absl' is reserved for
+// implementation of the Abseil library and should not be opened in the user
+// code [abseil-no-namespace]
+
+namespace absl {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: namespace 'absl'
+namespace std {
+int i = 5;
+}
+}
+
+// Things that shouldn't trigger the check
+int i = 5;
+namespace std {}
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 @@
+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 @@
+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-duration-division
+ abseil-no-namespace
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-namespace.rst
===
--- docs/clang-tidy/checks/abseil-no-namespace.rst
+++ docs/clang-tidy/checks/abseil-no-namespace.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - abseil-no-namespace
+
+abseil-no-namespace
+===
+
+Ensures code does not open ``namespace absl`` as that violates Abseil's
+compatibility guidelines. Code should not open ``namespace absl`` as that
+conflicts with Abseil's compatibility guidelines and may result in breakage.
+
+Any code that uses:
+
+.. code-block:: c++
+
+ namespace absl {
+ ...
+ }
+
+will be prompted with a warning.
+
+See `the full Abseil compatibility guidelines `_ for more information.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,12 @@
Improvements to clang-tidy
--
+- New :doc:`abseil-no-namespace
+ ` check.
+
+ Ensures code does not open ``namespace absl`` as that violates Abseil's
+ compatibility guidelines.
+
- New :doc:`abseil-duration-division
` check.
@@ -70,6 +76,7 @@
Detects usage of magic numbers, numbers that are used as literals instead of
introduced via constants or symbols.
+>>> .r340314
Improvements to include-fixer
-
Index: clang-tidy/abseil/NoNamespaceCheck.h
===
--- clang-tidy/abseil/NoNamespaceCheck.h
+++ clang-tidy/abseil/NoNamespaceCheck.h
@@ -0,0 +1,36 @@
+//===--- NoNamespaceCheck.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_NONAMESPACECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// This check ensures users don't open namespace absl, as that violates
+/// Abseil's compatibility gui
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
deannagarcia updated this revision to Diff 161939.
deannagarcia marked 2 inline comments as done.
https://reviews.llvm.org/D50580
Files:
clang-tidy/abseil/AbseilMatcher.h
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/NoNamespaceCheck.cpp
clang-tidy/abseil/NoNamespaceCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-no-namespace.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-namespace.cpp
Index: test/clang-tidy/abseil-no-namespace.cpp
===
--- test/clang-tidy/abseil-no-namespace.cpp
+++ test/clang-tidy/abseil-no-namespace.cpp
@@ -0,0 +1,28 @@
+// RUN: %check_clang_tidy %s abseil-no-namespace %t -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-namespace' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+/// Warning will not be triggered on internal Abseil code that is included.
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+/// 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
+// for implementation of the Abseil library and should not be opened in user
+// code [abseil-no-namespace]
+
+namespace absl {}
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: namespace 'absl' is reserved for
+// implementation of the Abseil library and should not be opened in the user
+// code [abseil-no-namespace]
+
+namespace absl {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: namespace 'absl'
+namespace std {
+int i = 5;
+}
+}
+
+// Things that shouldn't trigger the check
+int i = 5;
+namespace std {}
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 @@
+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 @@
+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-duration-division
+ abseil-no-namespace
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-namespace.rst
===
--- docs/clang-tidy/checks/abseil-no-namespace.rst
+++ docs/clang-tidy/checks/abseil-no-namespace.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - abseil-no-namespace
+
+abseil-no-namespace
+===
+
+Ensures code does not open ``namespace absl`` as that violates Abseil's
+compatibility guidelines. Code should not open ``namespace absl`` as that
+conflicts with Abseil's compatibility guidelines and may result in breakage.
+
+Any code that uses:
+
+.. code-block:: c++
+
+ namespace absl {
+ ...
+ }
+
+will be prompted with a warning.
+
+See `the full Abseil compatibility guidelines `_ for more information.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -64,6 +64,12 @@
floating-point context, and recommends the use of a function that
returns a floating-point value.
+- New :doc:`abseil-no-namespace
+ ` check.
+
+ Ensures code does not open ``namespace absl`` as that violates Abseil's
+ compatibility guidelines.
+
- New :doc:`readability-magic-numbers
` check.
Index: clang-tidy/abseil/NoNamespaceCheck.h
===
--- clang-tidy/abseil/NoNamespaceCheck.h
+++ clang-tidy/abseil/NoNamespaceCheck.h
@@ -0,0 +1,36 @@
+//===--- NoNamespaceCheck.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_NONAMESPACECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// This check ensures users don't open namespace absl, as that violates
+/// Abseil's compatibility guidelines.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-no-namespace.html
+class NoNamespaceCheck : public ClangTi
[PATCH] D51100: [clang-tidy] Add Abseil prefix to documentation
deannagarcia created this revision. deannagarcia added reviewers: aaron.ballman, hokein. deannagarcia added a project: clang-tools-extra. Herald added a subscriber: xazax.hun. Adds the Abseil prefix to the list of prefixes in the documentation https://reviews.llvm.org/D51100 Files: docs/clang-tidy/index.rst Index: docs/clang-tidy/index.rst === --- docs/clang-tidy/index.rst +++ docs/clang-tidy/index.rst @@ -55,6 +55,7 @@ == = Name prefixDescription == = +``abseil-``Checks related to Abseil library. ``android-`` Checks related to Android. ``boost-`` Checks related to Boost library. ``bugprone-`` Checks that target bugprone code constructs. Index: docs/clang-tidy/index.rst === --- docs/clang-tidy/index.rst +++ docs/clang-tidy/index.rst @@ -55,6 +55,7 @@ == = Name prefixDescription == = +``abseil-``Checks related to Abseil library. ``android-`` Checks related to Android. ``boost-`` Checks related to Boost library. ``bugprone-`` Checks that target bugprone code constructs. ___ 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
deannagarcia updated this revision to Diff 162417.
deannagarcia added a comment.
Rebased the patch
https://reviews.llvm.org/D50580
Files:
clang-tidy/abseil/AbseilMatcher.h
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/NoNamespaceCheck.cpp
clang-tidy/abseil/NoNamespaceCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-no-namespace.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-namespace.cpp
Index: test/clang-tidy/abseil-no-namespace.cpp
===
--- test/clang-tidy/abseil-no-namespace.cpp
+++ test/clang-tidy/abseil-no-namespace.cpp
@@ -0,0 +1,28 @@
+// RUN: %check_clang_tidy %s abseil-no-namespace %t -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-namespace' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+/// Warning will not be triggered on internal Abseil code that is included.
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+/// 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
+// for implementation of the Abseil library and should not be opened in user
+// code [abseil-no-namespace]
+
+namespace absl {}
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: namespace 'absl' is reserved for
+// implementation of the Abseil library and should not be opened in the user
+// code [abseil-no-namespace]
+
+namespace absl {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: namespace 'absl'
+namespace std {
+int i = 5;
+}
+}
+
+// Things that shouldn't trigger the check
+int i = 5;
+namespace std {}
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 @@
+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 @@
+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-namespace
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-no-namespace.rst
===
--- docs/clang-tidy/checks/abseil-no-namespace.rst
+++ docs/clang-tidy/checks/abseil-no-namespace.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - abseil-no-namespace
+
+abseil-no-namespace
+===
+
+Ensures code does not open ``namespace absl`` as that violates Abseil's
+compatibility guidelines. Code should not open ``namespace absl`` as that
+conflicts with Abseil's compatibility guidelines and may result in breakage.
+
+Any code that uses:
+
+.. code-block:: c++
+
+ namespace absl {
+ ...
+ }
+
+will be prompted with a warning.
+
+See `the full Abseil compatibility guidelines `_ for more information.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -70,6 +70,12 @@
Finds instances of ``absl::StrSplit()`` or ``absl::MaxSplits()`` where the
delimiter is a single character string literal and replaces with a character.
+- New :doc:`abseil-no-namespace
+ ` check.
+
+ Ensures code does not open ``namespace absl`` as that violates Abseil's
+ compatibility guidelines.
+
- New :doc:`readability-magic-numbers
` check.
Index: clang-tidy/abseil/NoNamespaceCheck.h
===
--- clang-tidy/abseil/NoNamespaceCheck.h
+++ clang-tidy/abseil/NoNamespaceCheck.h
@@ -0,0 +1,36 @@
+//===--- NoNamespaceCheck.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_NONAMESPACECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// This check ensures users don't open namespace absl, as that violates
+/// Abseil's compatibility guidelines.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
deannagarcia added inline comments. Comment at: test/clang-tidy/abseil-no-namespace.cpp:10 +#include "absl/external-file.h" +// CHECK: absl/external-file.h:1:11: warning: namespace 'absl' is reserved +// for implementation of the Abseil library and should not be opened in user hokein wrote: > Does the test get passed on the first command `%check_clang_tidy %s > abseil-no-namespace %t -- -- -I %S/Inputs`? The first command will suppress > all warning in headers, I think it will fail? The test passes, and I'm pretty sure it's because this is a CHECK not a CHECK-MESSAGES https://reviews.llvm.org/D50580 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
