[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace
aaron.ballman added inline comments.
Comment at: clang-tidy/abseil/SafelyScopedCheck.cpp:22
+ // The target using declaration is either:
+ // 1. not in any namespace declaration, or
+ // 2. in some namespace declaration but not in the innermost layer
aaron.ballman wrote:
> Why is this not also checking that the using declaration is not within a
> header file?
I'm still wondering about this. The Abseil tip specifically says `Never declare
namespace aliases or convenience using-declarations at namespace scope in
header files, only in .cc files.`, which is why I had figured I would see some
enforcement from this check.
Comment at: clang-tidy/abseil/SafelyScopedCheck.cpp:37
+ diag(MatchedDecl->getLocation(),
+ "using declaration %0 not declared in the innermost namespace.")
+ << MatchedDecl;
aaron.ballman wrote:
> You should remove the full stop at the end of the diagnostic.
The diagnostic still doesn't meet our (somewhat strange) requirements;
diagnostics should not be full sentences with capitalization and punctuation.
However, it also doesn't really tell the user what is wrong with their code,
just how to fix it to make the diagnostic go away.
I don't have a particularly great suggestion for a replacement, however. I'm
still trying to wrap my mind around the suggested changes to code, because it
seems like this advice isn't general purpose. Consider:
```
namespace Foo { // I control this namespace.
namespace details {
int func(int I);
} // namespace details
using Frobble::Bobble;
void f(SomeNameFromFrobbleBobble FB) {
FB.whatever(details::func(12));
}
} // namespace Foo
```
The suggestion to move the using declaration into the innermost namespace is
actually a poor one -- we don't want the interface of `f()` to be `void
f(details::SomeNameFromFrobbleBobble FB)`, which is what this check seems to
suggest we do. I don't see how we can determine whether the using declaration
should or should not be moved, so I can't really tell what the diagnostic text
should be.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55411/new/
https://reviews.llvm.org/D55411
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace
Ywicheng updated this revision to Diff 195661.
Ywicheng marked 3 inline comments as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55411/new/
https://reviews.llvm.org/D55411
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/SafelyScopedCheck.cpp
clang-tidy/abseil/SafelyScopedCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-safely-scoped.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-safely-scoped.cpp
Index: test/clang-tidy/abseil-safely-scoped.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-safely-scoped.cpp
@@ -0,0 +1,55 @@
+// RUN: %check_clang_tidy %s abseil-safely-scoped %t
+namespace bar {
+
+class A {};
+class B {};
+
+} // namespace bar
+
+namespace foo1 {
+
+// CHECK-MESSAGES: :[[@LINE+4]]:12: warning: using declaration 'A' is not
+// declared in the innermost namespace. Use discretion when moving using
+// declarations as it might necessitate moving lines containing relevant
+// aliases. [abseil-safely-scoped]
+using bar::A;
+void f(A a);
+
+namespace {} // anonymous namespace
+
+} // namespace foo1
+
+namespace foo2 {
+
+namespace {
+
+using ::bar::B;
+
+} // anonymous namespace
+
+void g(B b);
+
+} // namespace foo2
+
+// Check should not be triggered below when we are at
+// function (instead of namespace) scope.
+namespace outer {
+
+int fun_scope() {
+ using ::bar::A;
+ return 0;
+} // function scope
+
+class Base {
+public:
+ void f();
+}; // class scope
+
+class Derived : public Base {
+public:
+ using Base::f;
+}; // class scope
+
+namespace inner {} // namespace inner
+
+} // namespace outer
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
+ abseil-safely-scoped
abseil-str-cat-append
abseil-string-find-startswith
abseil-time-subtraction
Index: docs/clang-tidy/checks/abseil-safely-scoped.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-safely-scoped.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - abseil-safely-scoped
+
+abseil-safely-scoped
+
+
+Flags using declarations that are not contained in an innermost
+namespace, and suggests these declarations be moved elsewhere.
+
+Example:
+
+.. code-block:: c++
+
+ using something; // should be inside the innermost namespace bar below
+
+ namespace foo {
+ namespace bar {
+
+ } // bar
+
+ using something_else; // shoulw be inside the innermost namespace bar above
+
+ } // foo
+
+Placing convenience aliases in upper level namespaces can lead to ambiguity in
+which name the compiler should use.
+
+See https://abseil.io/tips/119 for more explanation.
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,12 @@
Improvements to clang-tidy
--
+- New :doc:`abseil-safely-scoped
+ ` check.
+
+ Finds instances of using declarations not in the innermost layer
+ of a series of namespaces.
+
- New :doc:`abseil-duration-addition
` check.
Index: clang-tidy/abseil/SafelyScopedCheck.h
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.h
@@ -0,0 +1,36 @@
+//===--- SafelyScopedCheck.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_SAFELYSCOPEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Detecting using declarations not in a namespace declaration or not in
+/// the innermost layer of namespace declarations.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-safely-scoped.html
+class SafelyScopedCheck : public ClangTidyCheck {
+public:
+ SafelyScopedCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
Index: clang-tidy/abseil/SafelyScopedCheck.cpp
===
---
[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace
aaron.ballman added inline comments. Comment at: clang-tidy/abseil/SafelyScopedCheck.cpp:37 + diag(MatchedDecl->getLocation(), + "using declaration %0 not declared in the innermost namespace.") + << MatchedDecl; You should remove the full stop at the end of the diagnostic. Comment at: test/clang-tidy/abseil-safely-scoped.cpp:15-16 +} // namespace foo1 +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: using declaration 'A' not +// declared in the innermost namespace. [abseil-safely-scoped] + This should be moved closer to where the actual warning will show up. However, the diagnostic doesn't help me to understand what is wrong with the code or how to fix it. I think it might be telling me that the using declaration is supposed to be inside of the anonymous namespace, but moving the using declaration there will break the declaration of `f()`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55411/new/ https://reviews.llvm.org/D55411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace
Ywicheng updated this revision to Diff 194631.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55411/new/
https://reviews.llvm.org/D55411
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/SafelyScopedCheck.cpp
clang-tidy/abseil/SafelyScopedCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-safely-scoped.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-safely-scoped.cpp
Index: test/clang-tidy/abseil-safely-scoped.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-safely-scoped.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s abseil-safely-scoped %t
+namespace bar {
+
+class A {};
+class B {};
+} // namespace bar
+
+namespace foo1 {
+
+using bar::A;
+void f(A a);
+
+namespace {} // anonymous namespace
+} // namespace foo1
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: using declaration 'A' not
+// declared in the innermost namespace. [abseil-safely-scoped]
+
+namespace foo2 {
+
+namespace {
+
+using ::bar::B;
+
+} // anonymous namespace
+void g(B b);
+} // namespace foo2
+
+
+// Check should not be triggered below when we are at
+// function (instead of namespace) scope.
+namespace outer {
+
+ int fun_scope() {
+using ::bar::A;
+return 0;
+ } // function scope
+
+ namespace inner {
+
+ } // namespace inner
+
+} // namespace outer
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
+ abseil-safely-scoped
abseil-str-cat-append
abseil-string-find-startswith
abseil-time-subtraction
Index: docs/clang-tidy/checks/abseil-safely-scoped.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-safely-scoped.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - abseil-safely-scoped
+
+abseil-safely-scoped
+
+
+Flags using declarations that are not contained in an innermost
+namespace, and suggests these declarations be moved elsewhere.
+
+Example:
+
+.. code-block:: c++
+
+ using something; // should be inside the innermost namespace bar below
+
+ namespace foo {
+ namespace bar {
+
+ } // bar
+
+ using something_else; // shoulw be inside the innermost namespace bar above
+
+ } // foo
+
+Placing convenience aliases in upper level namespaces can lead to ambiguity in
+which name the compiler should use.
+
+See https://abseil.io/tips/119 for more explanation.
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,12 @@
Improvements to clang-tidy
--
+- New :doc:`abseil-safely-scoped
+ ` check.
+
+ Finds instances of using declarations not in the innermost layer
+ of a series of namespaces.
+
- New :doc:`abseil-duration-addition
` check.
Index: clang-tidy/abseil/SafelyScopedCheck.h
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.h
@@ -0,0 +1,36 @@
+//===--- SafelyScopedCheck.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_SAFELYSCOPEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Detecting using declarations not in a namespace declaration or not in
+/// the innermost layer of namespace declarations.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-safely-scoped.html
+class SafelyScopedCheck : public ClangTidyCheck {
+public:
+ SafelyScopedCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
Index: clang-tidy/abseil/SafelyScopedCheck.cpp
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.cpp
@@ -0,0 +1,43 @@
+//===--- SafelyScopedCheck.cpp - clang-tidy ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open
[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace
Ywicheng updated this revision to Diff 194623.
Ywicheng marked 2 inline comments as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55411/new/
https://reviews.llvm.org/D55411
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/SafelyScopedCheck.cpp
clang-tidy/abseil/SafelyScopedCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-safely-scoped.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-safely-scoped.cpp
Index: test/clang-tidy/abseil-safely-scoped.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-safely-scoped.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s abseil-safely-scoped %t
+namespace bar {
+
+class A {};
+class B {};
+} // namespace bar
+
+namespace foo1 {
+
+using bar::A;
+void f(A a);
+
+namespace {} // anonymous namespace
+} // namespace foo1
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: using declaration 'A' not
+// declared in the innermost namespace. [abseil-safely-scoped]
+
+namespace foo2 {
+
+namespace {
+
+using ::bar::B;
+
+} // anonymous namespace
+void g(B b);
+} // namespace foo2
+
+
+// Check should not be triggered below when we are at
+// function (instead of namespace) scope.
+namespace outer {
+
+ int fun_scope() {
+using ::bar::A;
+return 0;
+ } // function scope
+
+ namespace inner {
+
+ } // namespace inner
+
+} // namespace outer
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
+ abseil-safely-scoped
abseil-str-cat-append
abseil-string-find-startswith
abseil-time-subtraction
Index: docs/clang-tidy/checks/abseil-safely-scoped.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-safely-scoped.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - abseil-safely-scoped
+
+abseil-safely-scoped
+
+
+Flags using declarations that are not contained in an innermost
+namespace, and suggests these declarations be moved elsewhere.
+
+Example:
+
+.. code-block:: c++
+
+ using something; // should be inside the innermost namespace bar below
+
+ namespace foo {
+ namespace bar {
+
+ } // bar
+
+ using something_else; // shoulw be inside the innermost namespace bar above
+
+ } // foo
+
+Placing convenience aliases in upper level namespaces can lead to ambiguity in
+which name the compiler should use.
+
+See https://abseil.io/tips/119 for more explanation.
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,8 +67,15 @@
Improvements to clang-tidy
--
+- New :doc:`abseil-safely-scoped
+ ` check.
+
+ Finds instances of using declarations not in the innermost layer
+ of a series of namespaces.
+
- New :doc:`abseil-duration-addition
` check.
+>>> master
Checks for cases where addition should be performed in the ``absl::Time``
domain.
Index: clang-tidy/abseil/SafelyScopedCheck.h
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.h
@@ -0,0 +1,36 @@
+//===--- SafelyScopedCheck.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_SAFELYSCOPEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Detecting using declarations not in a namespace declaration or not in
+/// the innermost layer of namespace declarations.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-safely-scoped.html
+class SafelyScopedCheck : public ClangTidyCheck {
+public:
+ SafelyScopedCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
Index: clang-tidy/abseil/SafelyScopedCheck.cpp
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.cpp
@@ -0,0 +1,43 @@
+//===--- SafelyScopedCheck.cpp - clang-tidy
[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace
Ywicheng updated this revision to Diff 194614.
Ywicheng marked an inline comment as done.
Ywicheng edited the summary of this revision.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55411/new/
https://reviews.llvm.org/D55411
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/SafelyScopedCheck.cpp
clang-tidy/abseil/SafelyScopedCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-safely-scoped.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-safely-scoped.cpp
Index: test/clang-tidy/abseil-safely-scoped.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-safely-scoped.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s abseil-safely-scoped %t
+namespace bar {
+
+class A {};
+class B {};
+} // namespace bar
+
+namespace foo1 {
+
+using bar::A;
+void f(A a);
+
+namespace {} // anonymous namespace
+} // namespace foo1
+// CHECK-MESSAGES: :[[@LINE-4]]:12: warning: using declaration 'A' not
+// declared in the innermost namespace. [abseil-safely-scoped]
+
+namespace foo2 {
+
+namespace {
+
+using ::bar::B;
+
+} // anonymous namespace
+void g(B b);
+} // namespace foo2
+
+
+// Check should not be triggered below when we are at
+// function (instead of namespace) scope.
+namespace outer {
+
+ int fun_scope() {
+using ::bar::A;
+return 0;
+ } // function scope
+
+ namespace inner {
+
+ } // namespace inner
+
+} // namespace outer
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
+ abseil-safely-scoped
abseil-str-cat-append
abseil-string-find-startswith
abseil-time-subtraction
Index: docs/clang-tidy/checks/abseil-safely-scoped.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-safely-scoped.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - abseil-safely-scoped
+
+abseil-safely-scoped
+
+
+Flags using declarations that are not contained in an innermost
+namespace, and suggests these declarations be moved elsewhere.
+
+Example:
+
+.. code-block:: c++
+
+ using something; // should be inside the innermost namespace bar below
+
+ namespace foo {
+ namespace bar {
+
+ } // bar
+
+ using something_else; // shoulw be inside the innermost namespace bar above
+
+ } // foo
+
+Placing convenience aliases in upper level namespaces can lead to ambiguity in
+which name the compiler should use.
+
+See https://abseil.io/tips/119 for more explanation.
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,8 +67,15 @@
Improvements to clang-tidy
--
+- New :doc:`abseil-safely-scoped
+ ` check.
+
+ Finds instances of using declarations not in the innermost layer
+ of a series of namespaces.
+
- New :doc:`abseil-duration-addition
` check.
+>>> master
Checks for cases where addition should be performed in the ``absl::Time``
domain.
Index: clang-tidy/abseil/SafelyScopedCheck.h
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.h
@@ -0,0 +1,36 @@
+//===--- SafelyScopedCheck.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_SAFELYSCOPEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Detecting using declarations not in a namespace declaration or not in
+/// the innermost layer of namespace declarations.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-safely-scoped.html
+class SafelyScopedCheck : public ClangTidyCheck {
+public:
+ SafelyScopedCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
Index: clang-tidy/abseil/SafelyScopedCheck.cpp
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.cpp
@@ -0,0 +1,43 @@
+//===---
[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace
aaron.ballman added inline comments.
Comment at: clang-tidy/abseil/SafelyScopedCheck.cpp:22
+ // The target using declaration is either:
+ // 1. not in any namespace declaration, or
+ // 2. in some namespace declaration but not in the innermost layer
Why is this not also checking that the using declaration is not within a header
file?
Comment at: clang-tidy/abseil/SafelyScopedCheck.cpp:24-27
+ Finder->addMatcher(usingDecl(eachOf(
+ usingDecl(unless(hasParent(namespaceDecl(,
+usingDecl(hasParent(namespaceDecl(forEach(namespaceDecl() )
+).bind("use"), this);
Did clang-format produce this? If not, you should run the patch through
clang-format.
Comment at: clang-tidy/abseil/SafelyScopedCheck.cpp:32
+ const auto *MatchedDecl = Result.Nodes.getNodeAs("use");
+ diag(MatchedDecl->getLocation(), "UsingDecl %0 should be in the innermost "
+"scope. See: https://abseil.io/tips/119;)
aaron.ballman wrote:
> Do not add links to diagnostic messages. They should stand on their own, and
> not be grammatical with punctuation. I would suggest: `using declaration not
> declared in the innermost namespace`
I suspect this diagnostic is going to be too aggressive and once you add more
tests will find it needs some additional constraints.
Comment at: clang-tidy/abseil/SafelyScopedCheck.cpp:32-33
+ const auto *MatchedDecl = Result.Nodes.getNodeAs("use");
+ diag(MatchedDecl->getLocation(), "UsingDecl %0 should be in the innermost "
+"scope. See: https://abseil.io/tips/119;)
+ << MatchedDecl;
Do not add links to diagnostic messages. They should stand on their own, and
not be grammatical with punctuation. I would suggest: `using declaration not
declared in the innermost namespace`
Comment at: test/clang-tidy/abseil-safely-scoped.cpp:1
+// RUN: %check_clang_tidy %s abseil-safely-scoped %t
+namespace bar {
You're missing a lot of tests, such as using declarations at function scope,
within a class scope, etc.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55411/new/
https://reviews.llvm.org/D55411
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace
JonasToth added a comment. Please upload the patch with full context. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55411/new/ https://reviews.llvm.org/D55411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace
Ywicheng created this revision.
Ywicheng added a reviewer: JonasToth.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
This patch adds one check corresponding to the Scope of the Alias section in
https://abseil.io/tips/119.
In particular, it is better to put aliases in the innermost namespace
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D55411
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/SafelyScopedCheck.cpp
clang-tidy/abseil/SafelyScopedCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-safely-scoped.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-safely-scoped.cpp
Index: test/clang-tidy/abseil-safely-scoped.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-safely-scoped.cpp
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy %s abseil-safely-scoped %t
+namespace bar {
+
+class something {
+
+};
+}
+
+namespace foo {
+
+using bar::something;
+
+namespace {
+
+} // anonymous namespace
+} // namespace foo
+// CHECK-MESSAGES: :[[@LINE-6]]:12: warning: UsingDecl 'something' should be in the innermost scope. See: https://abseil.io/tips/119 [abseil-safely-scoped]
+
+namespace foo {
+
+namespace {
+
+using ::bar::something;
+
+} // anonymous namespace
+} // namespace foo
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -10,8 +10,9 @@
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
- abseil-string-find-startswith
+ abseil-safely-scoped
abseil-str-cat-append
+ abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
@@ -151,6 +152,7 @@
hicpp-special-member-functions (redirects to cppcoreguidelines-special-member-functions)
hicpp-static-assert (redirects to misc-static-assert)
hicpp-undelegated-constructor (redirects to bugprone-undelegated-constructor)
+ hicpp-uppercase-literal-suffix (redirects to readability-uppercase-literal-suffix)
hicpp-use-auto (redirects to modernize-use-auto)
hicpp-use-emplace (redirects to modernize-use-emplace)
hicpp-use-equals-default (redirects to modernize-use-equals-default)
@@ -159,7 +161,6 @@
hicpp-use-nullptr (redirects to modernize-use-nullptr)
hicpp-use-override (redirects to modernize-use-override)
hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg)
- hicpp-uppercase-literal-suffix (redirects to readability-uppercase-literal-suffix)
llvm-header-guard
llvm-include-order
llvm-namespace-comment
Index: docs/clang-tidy/checks/abseil-safely-scoped.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-safely-scoped.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - abseil-safely-scoped
+
+abseil-safely-scoped
+
+
+Flags using declarations that are not contained in an innermost
+namespace, and suggests these declarations be moved elsewhere.
+
+Example:
+
+.. code-block:: c++
+
+ using something; // should be inside the innermost namespace bar below
+
+ namespace foo {
+ namespace bar {
+
+ } // bar
+
+ using something_else; // shoulw be inside the innermost namespace bar above
+
+ } // foo
+
+Placing convenience aliases in upper level namespaces can lead to ambiguity in
+which name the compiler should use.
+
+See https://abseil.io/tips/119 for more explanation.
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,12 @@
Improvements to clang-tidy
--
+- New :doc:`abseil-safely-scoped
+ ` check.
+
+ Finds instances of using declarations not in the innermost layer
+ of a series of namespaces.
+
- New :doc:`abseil-duration-division
` check.
Index: clang-tidy/abseil/SafelyScopedCheck.h
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.h
@@ -0,0 +1,36 @@
+//===--- SafelyScopedCheck.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_SAFELYSCOPEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Detecting using declarations not in a namespace declaration or not in
+/// the innermost layer of namespace declarations.
+///
+/// For the user-facing documentation
