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<UsingDecl>("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<UsingDecl>("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
