[PATCH] D50580: [clang-tidy] Abseil: no namespace check
Haojian Wu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 21 08:28:36 PDT 2018
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 name now should be `isInAbseilFile`.
================
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:26
+ Finder->addMatcher(
+ namespaceDecl(hasName("absl"), unless(isExpansionInAbseilHeader()))
+ .bind("abslNamespace"),
----------------
Does the `absl` namespace is always the top level namespace? If so, I think it would be safer to use qualified name "::absl" here.
================
Comment at: clang-tidy/abseil/NoNamespaceCheck.h:19
+
+/// This test ensures users don't open namespace absl, as that violates
+/// our compatibility guidelines.
----------------
nit: test => check.
================
Comment at: clang-tidy/abseil/NoNamespaceCheck.h:20
+/// This test ensures users don't open namespace absl, as that violates
+/// our compatibility guidelines.
+///
----------------
nit: our = abseil?
================
Comment at: test/clang-tidy/abseil-no-namespace.cpp:22
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: namespace 'absl'
+
----------------
nit: put it immediately after the above `name absl {`.
https://reviews.llvm.org/D50580
More information about the cfe-commits
mailing list