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