[clang-tools-extra] r315057 - Fix nested namespaces in google-readability-nested-namespace-comments.
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 6 06:29:48 PDT 2017
On Fri, Oct 6, 2017 at 9:27 AM, Alexander Kornienko <alexfh at google.com> wrote:
>
>
> 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?
Sure! I've commit in r315060.
~Aaron
>
> 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
>
>
More information about the cfe-commits
mailing list