<div dir="auto"><div><br><div class="gmail_extra"><br><div class="gmail_quote">On 6 Oct 2017 14:59, "Aaron Ballman via cfe-commits" <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: aaronballman<br>
Date: Fri Oct 6 05:57:28 2017<br>
New Revision: 315057<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=315057&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=315057&view=rev</a><br>
Log:<br>
Fix nested namespaces in google-readability-nested-<wbr>namespace-comments.<br>
<br>
Fixes PR34701.<br>
<br>
Patch by Alexandru Octavian Buțiu.<br>
<br>
Added:<br>
clang-tools-extra/trunk/test/<wbr>clang-tidy/google-readability-<wbr>nested-namespace-comments.cpp<br></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">Can we rename the test so it starts with the check name? E.g. "google-readability-namespace-comments-cxx17" or something else that makes sense?</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Modified:<br>
clang-tools-extra/trunk/clang-<wbr>tidy/readability/<wbr>NamespaceCommentCheck.cpp<br>
clang-tools-extra/trunk/clang-<wbr>tidy/readability/<wbr>NamespaceCommentCheck.h<br>
<br>
Modified: clang-tools-extra/trunk/clang-<wbr>tidy/readability/<wbr>NamespaceCommentCheck.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp?rev=315057&r1=315056&r2=315057&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/clang-tools-extra/<wbr>trunk/clang-tidy/readability/<wbr>NamespaceCommentCheck.cpp?rev=<wbr>315057&r1=315056&r2=315057&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- clang-tools-extra/trunk/clang-<wbr>tidy/readability/<wbr>NamespaceCommentCheck.cpp (original)<br>
+++ clang-tools-extra/trunk/clang-<wbr>tidy/readability/<wbr>NamespaceCommentCheck.cpp Fri Oct 6 05:57:28 2017<br>
@@ -23,7 +23,7 @@ NamespaceCommentCheck::<wbr>NamespaceCommentC<br>
ClangTidyContext *Context)<br>
: ClangTidyCheck(Name, Context),<br>
NamespaceCommentPattern("^/[/*<wbr>] *(end (of )?)? *(anonymous|unnamed)? *"<br>
- "namespace( +([a-zA-Z0-9_]+))?\\.? *(\\*/)?$",<br>
+ "namespace( +([a-zA-Z0-9_:]+))?\\.? *(\\*/)?$",<br>
llvm::Regex::IgnoreCase),<br>
ShortNamespaceLines(Options.<wbr>get("ShortNamespaceLines", 1u)),<br>
SpacesBeforeComments(Options.<wbr>get("SpacesBeforeComments", 1u)) {}<br>
@@ -56,6 +56,15 @@ static std::string getNamespaceComment(c<br>
return Fix;<br>
}<br>
<br>
+static std::string getNamespaceComment(const std::string &NameSpaceName,<br>
+ bool InsertLineBreak) {<br>
+ std::string Fix = "// namespace ";<br>
+ Fix.append(NameSpaceName);<br>
+ if (InsertLineBreak)<br>
+ Fix.append("\n");<br>
+ return Fix;<br>
+}<br>
+<br>
void NamespaceCommentCheck::check(<wbr>const MatchFinder::MatchResult &Result) {<br>
const auto *ND = Result.Nodes.getNodeAs<<wbr>NamespaceDecl>("namespace");<br>
const SourceManager &Sources = *Result.SourceManager;<br>
@@ -74,11 +83,38 @@ void NamespaceCommentCheck::check(<wbr>const<br>
SourceLocation AfterRBrace = ND->getRBraceLoc().<wbr>getLocWithOffset(1);<br>
SourceLocation Loc = AfterRBrace;<br>
Token Tok;<br>
+ SourceLocation LBracketLocation = ND->getLocation();<br>
+ SourceLocation NestedNamespaceBegin = LBracketLocation;<br>
+<br>
+ // Currently for nested namepsace (n1::n2::...) the AST matcher will match foo<br>
+ // then bar instead of a single match. So if we got a nested namespace we have<br>
+ // to skip the next ones.<br>
+ for (const auto &EndOfNameLocation : Ends) {<br>
+ if (Sources.<wbr>isBeforeInTranslationUnit(<wbr>NestedNamespaceBegin,<br>
+ EndOfNameLocation))<br>
+ return;<br>
+ }<br>
+ while (Lexer::getRawToken(<wbr>LBracketLocation, Tok, Sources, getLangOpts()) ||<br>
+ !Tok.is(tok::l_brace)) {<br>
+ LBracketLocation = LBracketLocation.<wbr>getLocWithOffset(1);<br>
+ }<br>
+<br>
+ auto TextRange =<br>
+ Lexer::getAsCharRange(<wbr>SourceRange(<wbr>NestedNamespaceBegin, LBracketLocation),<br>
+ Sources, getLangOpts());<br>
+ auto NestedNamespaceName =<br>
+ Lexer::getSourceText(<wbr>TextRange, Sources, getLangOpts()).rtrim();<br>
+ bool IsNested = NestedNamespaceName.contains('<wbr>:');<br>
+<br>
+ if (IsNested)<br>
+ Ends.push_back(<wbr>LBracketLocation);<br>
+<br>
// Skip whitespace until we find the next token.<br>
while (Lexer::getRawToken(Loc, Tok, Sources, getLangOpts()) ||<br>
Tok.is(tok::semi)) {<br>
Loc = Loc.getLocWithOffset(1);<br>
}<br>
+<br>
if (!locationsInSameFile(Sources, ND->getRBraceLoc(), Loc))<br>
return;<br>
<br>
@@ -98,10 +134,14 @@ void NamespaceCommentCheck::check(<wbr>const<br>
StringRef NamespaceNameInComment = Groups.size() > 5 ? Groups[5] : "";<br>
StringRef Anonymous = Groups.size() > 3 ? Groups[3] : "";<br>
<br>
- // Check if the namespace in the comment is the same.<br>
- if ((ND->isAnonymousNamespace() && NamespaceNameInComment.empty()<wbr>) ||<br>
- (ND->getNameAsString() == NamespaceNameInComment &&<br>
- Anonymous.empty())) {<br>
+ if (IsNested && NestedNamespaceName == NamespaceNameInComment) {<br>
+ // C++17 nested namespace.<br>
+ return;<br>
+ } else if ((ND->isAnonymousNamespace() &&<br>
+ NamespaceNameInComment.empty()<wbr>) ||<br>
+ (ND->getNameAsString() == NamespaceNameInComment &&<br>
+ Anonymous.empty())) {<br>
+ // Check if the namespace in the comment is the same.<br>
// FIXME: Maybe we need a strict mode, where we always fix namespace<br>
// comments with different format.<br>
return;<br>
@@ -131,13 +171,16 @@ void NamespaceCommentCheck::check(<wbr>const<br>
std::string NamespaceName =<br>
ND->isAnonymousNamespace()<br>
? "anonymous namespace"<br>
- : ("namespace '" + ND->getNameAsString() + "'");<br>
+ : ("namespace '" + NestedNamespaceName.str() + "'");<br>
<br>
diag(AfterRBrace, Message)<br>
- << NamespaceName << FixItHint::CreateReplacement(<br>
- CharSourceRange::getCharRange(<wbr>OldCommentRange),<br>
- std::string(<wbr>SpacesBeforeComments, ' ') +<br>
- getNamespaceComment(ND, NeedLineBreak));<br>
+ << NamespaceName<br>
+ << FixItHint::CreateReplacement(<br>
+ CharSourceRange::getCharRange(<wbr>OldCommentRange),<br>
+ std::string(<wbr>SpacesBeforeComments, ' ') +<br>
+ (IsNested<br>
+ ? getNamespaceComment(<wbr>NestedNamespaceName, NeedLineBreak)<br>
+ : getNamespaceComment(ND, NeedLineBreak)));<br>
diag(ND->getLocation(), "%0 starts here", DiagnosticIDs::Note)<br>
<< NamespaceName;<br>
}<br>
<br>
Modified: clang-tools-extra/trunk/clang-<wbr>tidy/readability/<wbr>NamespaceCommentCheck.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h?rev=315057&r1=315056&r2=315057&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/clang-tools-extra/<wbr>trunk/clang-tidy/readability/<wbr>NamespaceCommentCheck.h?rev=<wbr>315057&r1=315056&r2=315057&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- clang-tools-extra/trunk/clang-<wbr>tidy/readability/<wbr>NamespaceCommentCheck.h (original)<br>
+++ clang-tools-extra/trunk/clang-<wbr>tidy/readability/<wbr>NamespaceCommentCheck.h Fri Oct 6 05:57:28 2017<br>
@@ -34,6 +34,7 @@ private:<br>
llvm::Regex NamespaceCommentPattern;<br>
const unsigned ShortNamespaceLines;<br>
const unsigned SpacesBeforeComments;<br>
+ llvm::SmallVector<<wbr>SourceLocation, 4> Ends;<br>
};<br>
<br>
} // namespace readability<br>
<br>
Added: clang-tools-extra/trunk/test/<wbr>clang-tidy/google-readability-<wbr>nested-namespace-comments.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp?rev=315057&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/clang-tools-extra/<wbr>trunk/test/clang-tidy/google-<wbr>readability-nested-namespace-<wbr>comments.cpp?rev=315057&view=<wbr>auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- clang-tools-extra/trunk/test/<wbr>clang-tidy/google-readability-<wbr>nested-namespace-comments.cpp (added)<br>
+++ clang-tools-extra/trunk/test/<wbr>clang-tidy/google-readability-<wbr>nested-namespace-comments.cpp Fri Oct 6 05:57:28 2017<br>
@@ -0,0 +1,17 @@<br>
+// RUN: %check_clang_tidy %s google-readability-namespace-<wbr>comments %t -- -std=c++17<br>
+<br>
+namespace n1::n2 {<br>
+namespace n3 {<br>
+<br>
+ // So that namespace is not empty.<br>
+ void f();<br>
+<br>
+<br>
+// CHECK-MESSAGES: :[[@LINE+4]]:2: warning: namespace 'n3' not terminated with<br>
+// CHECK-MESSAGES: :[[@LINE-7]]:11: note: namespace 'n3' starts here<br>
+// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: namespace 'n1::n2' not terminated with a closing comment [google-readability-namespace-<wbr>comments]<br>
+// CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'n1::n2' starts here<br>
+}}<br>
+// CHECK-FIXES: } // namespace n3<br>
+// CHECK-FIXES: } // namespace n1::n2<br>
+<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div></div>