hokein closed this revision.
hokein added a comment.
I have committed the patch for you, https://reviews.llvm.org/rL340800.
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' i
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
--
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
cl
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
Looks good.
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 reserve
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
Eugene.Zelenko added inline comments.
Comment at: docs/ReleaseNotes.rst:60
+- New :doc:`abseil-no-namespace
+ ` check.
Please sort new checks list alphabetically.
Comment at: docs/ReleaseNotes.rst:79
+>>> .r340314
Improvements to incl
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
clan
Eugene.Zelenko added a comment.
Please rebase from trunk.
Comment at: docs/ReleaseNotes.rst:63
+
+ Checks to ensure code does not open `namespace absl` as that
+ violates Abseil's compatibility guidelines.
Just Ensures. Please use `` for language constructs.
hokein added a comment.
Thanks for the updates. Looks mostly good, a few nits.
Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+
+AST_POLYMORPHIC_MATCHER(isExpansionInAbseilHeader,
+AST_POLYMORPHIC_SUPPORTED_TYPES(
nit: this matcher nam
hugoeg added inline comments.
Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+ auto Filename = FileEntry->getName();
+ llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+ "synchronization|types|utiliy)");
deannagarci
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)");
lebed
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
clan
lebedev.ri 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)");
hokein
hokein 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
lebedev.ri 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)");
hokein
hokein added inline comments.
Comment at: clang-tidy/abseil/AbseilMatcher.h:16
+
+/// Matches AST nodes that were expanded within abseil-header-files.
+AST_POLYMORPHIC_MATCHER(isExpansionNotInAbseilHeader,
nit: We need proper documentation for this matcher, since
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:
> > > deannaga
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/AbseilMa
JonasToth added a comment.
That sounds good as well, just not in clang and best in `clang-tidy/utils`
> `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 re
hokein added inline comments.
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23
+
+ Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);
JonasToth wrote:
> hugoeg wrote:
> > deannagarcia wrote:
> > > aaron.ba
JonasToth added inline comments.
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23
+
+ Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);
hugoeg wrote:
> deannagarcia wrote:
> > aaron.ballman wrote:
> > > h
hugoeg added inline comments.
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23
+
+ Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);
deannagarcia wrote:
> aaron.ballman wrote:
> > hokein wrote:
> > > aaro
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:
> >
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
do
aaron.ballman added inline comments.
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23
+
+ Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);
hokein wrote:
> aaron.ballman wrote:
> > I think this needs a `n
hokein added a comment.
The check is missing its document, please add one in `docs/clang-tidy/checks/`.
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23
+
+ Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);
lebedev.ri added a comment.
> 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
aaron.ballman added inline comments.
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23
+
+ Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);
I think this needs a `not(isExpansionInSystemHeader())` in there
JonasToth added a comment.
Could it happen that some template specializations or so need to land in `absl`?
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:28
+void NoNamespaceCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *decl = Result.Nodes.getNodeAs
Eugene.Zelenko added a comment.
Check documentation is missing.
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:21
+void NoNamespaceCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus) return;
+
Please place return in separate line
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 guid
31 matches
Mail list logo
