[clang-tools-extra] r315057 - Fix nested namespaces in google-readability-nested-namespace-comments.
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 12 07:01:22 PDT 2017
On Fri, Oct 6, 2017 at 2:57 PM, 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
> 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)) {
>
Now another issue with this revision: the check started triggering an
assertion failure and incredible slowness (infinite loops?) on some real
files. I've not yet come up with an isolated test case, but all this seems
to be happening around this loop.
I'm going to revert this revision.
> + 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/20171012/ac5f0923/attachment-0001.html>
More information about the cfe-commits
mailing list