aaron.ballman requested changes to this revision.
aaron.ballman added inline comments.
This revision now requires changes to proceed.
================
Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.cpp:32
+void AnonymousEnclosedAliasesCheck::check(const MatchFinder::MatchResult
&Result) {
+
+ const auto *MatchedUsingDecl =
----------------
Spurious newline.
================
Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.cpp:41
+ // to the vector containing all candidate using declarations.
+ if (AnonymousNamespaceDecl) {
+ diag(MatchedUsingDecl->getLocation(),
----------------
I don't think this logic works in practice because there's no way to determine
that the anonymous namespace is even a candidate for putting the using
declaration into it. Consider a common scenario where there's an anonymous
namespace declared in a header file (like an STL header outside of the user's
control), and a using declaration inside of an implementation file. Just
because the STL declared an anonymous namespace doesn't mean that the user
could have put their using declaration in it.
================
Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.cpp:43-44
+ diag(MatchedUsingDecl->getLocation(),
+ "UsingDecl %0 should be in the anonymous namespace.
See: "
+ "https://abseil.io/tips/119";)
+ << MatchedUsingDecl;
----------------
Diagnostics should not be complete sentences or contain hyperlinks. (Same
below.)
================
Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.h:20
+
+/// Detecting incorrect practice of putting using declarations outside an
+/// anonymous namespace when there exists one.
----------------
Detecting incorrect -> Detecting the incorrect
================
Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.h:21
+/// Detecting incorrect practice of putting using declarations outside an
+/// anonymous namespace when there exists one.
+/// For the user-facing documentation see:
----------------
when there exists one -> when one exists
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55409/new/
https://reviews.llvm.org/D55409
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
