aaron.ballman added inline comments.
================
Comment at: clang-tidy/readability/NamespaceCommentCheck.cpp:86
+ SourceLocation LBracketLocation = ND->getLocation();
+ auto NestedNamespaceBegin = LBracketLocation;
+
----------------
Do not use `auto` here (the type is not spelled out in the initialization,
despite being relatively obvious. Same below.
================
Comment at: clang-tidy/readability/NamespaceCommentCheck.cpp:90
+ // then bar instead of a single match. So if we got a nested namespace we
have
+ // to skip the next ones
+ for (const auto &i : Ends) {
----------------
Missing a period at the end of the sentence.
================
Comment at: clang-tidy/readability/NamespaceCommentCheck.cpp:91
+ // to skip the next ones
+ for (const auto &i : Ends) {
+ if (Sources.isBeforeInTranslationUnit(NestedNamespaceBegin, i)) {
----------------
`i` does not meet the usual naming conventions; you might want to pick a
slightly better name.
================
Comment at: clang-tidy/readability/NamespaceCommentCheck.cpp:92
+ for (const auto &i : Ends) {
+ if (Sources.isBeforeInTranslationUnit(NestedNamespaceBegin, i)) {
+ return;
----------------
You can elide the braces from the `if` statement.
================
Comment at: clang-tidy/readability/NamespaceCommentCheck.cpp:109
+
+ if (IsNested) {
+ Ends.push_back(LBracketLocation);
----------------
Elide braces
================
Comment at: clang-tidy/readability/NamespaceCommentCheck.cpp:137
- // Check if the namespace in the comment is the same.
- if ((ND->isAnonymousNamespace() && NamespaceNameInComment.empty()) ||
- (ND->getNameAsString() == NamespaceNameInComment &&
- Anonymous.empty())) {
+ // C++17 nested namespace
+ if (IsNested && NestedNamespaceName == NamespaceNameInComment) {
----------------
Missing period in the comment
================
Comment at: clang-tidy/readability/NamespaceCommentCheck.cpp:140-141
+ return;
+ } // Check if the namespace in the comment is the same.
+ else if ((ND->isAnonymousNamespace() && NamespaceNameInComment.empty())
||
+ (ND->getNameAsString() == NamespaceNameInComment &&
----------------
I'd move the comments inside of the braces (the braces can be elided, but if
the comments are in the body instead of out of it, I think they're fine to
leave).
================
Comment at: clang-tidy/readability/NamespaceCommentCheck.h:37
const unsigned SpacesBeforeComments;
+ llvm::SmallVector<SourceLocation, 7> Ends;
};
----------------
Why 7?
================
Comment at: test/clang-tidy/google-readability-nested-namespace-comments.cpp:6
+
+ //so that namespace is not empty
+ void f();
----------------
Comment should start with a capital letter and end with punctuation.
================
Comment at:
test/clang-tidy/google-readability-nested-namespace-comments.cpp:8-14
+
+
+
+
+
+
+
----------------
Can remove the extra whitespace
================
Comment at: test/clang-tidy/google-readability-nested-namespace-comments.cpp:23
+// CHECK-FIXES: } // namespace n1::n2
\ No newline at end of file
----------------
Please make sure there's a newline at the end of the file.
https://reviews.llvm.org/D38284
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
