Author: alexfh Date: Thu Oct 12 07:25:16 2017 New Revision: 315580 URL: http://llvm.org/viewvc/llvm-project?rev=315580&view=rev Log: Revert "Fix nested namespaces in google-readability-nested-namespace-comments."
This reverts r315057. The revision introduces assertion failures: assertion failed at llvm/tools/clang/include/clang/Basic/SourceManager.h:428 in const clang::SrcMgr::ExpansionInfo &clang::SrcMgr::SLocEntry::getExpansion() const: isExpansion() && "Not a macro expansion SLocEntry!" Stack trace: __assert_fail clang::SrcMgr::SLocEntry::getExpansion() clang::SourceManager::getExpansionLocSlowCase() clang::SourceManager::getExpansionLoc() clang::Lexer::getRawToken() clang::tidy::readability::NamespaceCommentCheck::check() clang::ast_matchers::internal::(anonymous namespace)::MatchASTVisitor::MatchVisitor::visitMatch() clang::ast_matchers::internal::BoundNodesTreeBuilder::visitMatches() clang::ast_matchers::internal::(anonymous namespace)::MatchASTVisitor::matchWithFilter() clang::ast_matchers::internal::(anonymous namespace)::MatchASTVisitor::matchDispatch() clang::ast_matchers::internal::(anonymous namespace)::MatchASTVisitor::TraverseDecl() clang::RecursiveASTVisitor<>::TraverseDeclContextHelper() clang::RecursiveASTVisitor<>::TraverseDecl() clang::RecursiveASTVisitor<>::TraverseDeclContextHelper() clang::RecursiveASTVisitor<>::TraverseDecl() clang::RecursiveASTVisitor<>::TraverseDeclContextHelper() clang::RecursiveASTVisitor<>::TraverseDecl() clang::ast_matchers::MatchFinder::matchAST() clang::MultiplexConsumer::HandleTranslationUnit() clang::ParseAST() clang::FrontendAction::Execute() clang::CompilerInstance::ExecuteAction() clang::tooling::FrontendActionFactory::runInvocation() clang::tooling::ToolInvocation::runInvocation() clang::tooling::ToolInvocation::run() Still working on an isolated test case. Removed: clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments-cxx17.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=315580&r1=315579&r2=315580&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp Thu Oct 12 07:25:16 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,15 +56,6 @@ 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; @@ -83,38 +74,11 @@ 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; @@ -134,14 +98,10 @@ void NamespaceCommentCheck::check(const StringRef NamespaceNameInComment = Groups.size() > 5 ? Groups[5] : ""; StringRef Anonymous = Groups.size() > 3 ? Groups[3] : ""; - 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. + // Check if the namespace in the comment is the same. + if ((ND->isAnonymousNamespace() && NamespaceNameInComment.empty()) || + (ND->getNameAsString() == NamespaceNameInComment && + Anonymous.empty())) { // FIXME: Maybe we need a strict mode, where we always fix namespace // comments with different format. return; @@ -171,16 +131,13 @@ void NamespaceCommentCheck::check(const std::string NamespaceName = ND->isAnonymousNamespace() ? "anonymous namespace" - : ("namespace '" + NestedNamespaceName.str() + "'"); + : ("namespace '" + ND->getNameAsString() + "'"); diag(AfterRBrace, Message) - << NamespaceName - << FixItHint::CreateReplacement( - CharSourceRange::getCharRange(OldCommentRange), - std::string(SpacesBeforeComments, ' ') + - (IsNested - ? getNamespaceComment(NestedNamespaceName, NeedLineBreak) - : getNamespaceComment(ND, NeedLineBreak))); + << NamespaceName << FixItHint::CreateReplacement( + CharSourceRange::getCharRange(OldCommentRange), + std::string(SpacesBeforeComments, ' ') + + 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=315580&r1=315579&r2=315580&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h (original) +++ clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h Thu Oct 12 07:25:16 2017 @@ -34,7 +34,6 @@ private: llvm::Regex NamespaceCommentPattern; const unsigned ShortNamespaceLines; const unsigned SpacesBeforeComments; - llvm::SmallVector<SourceLocation, 4> Ends; }; } // namespace readability Removed: clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments-cxx17.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments-cxx17.cpp?rev=315579&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments-cxx17.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments-cxx17.cpp (removed) @@ -1,23 +0,0 @@ -// RUN: %check_clang_tidy %s google-readability-namespace-comments %t -- -- -std=c++17 - -namespace n1::n2 { -namespace n3 { - // So that namespace is not empty and has at least 10 lines. - // 1 - // 2 - // 3 - // 3 - // 4 - // 5 - // 6 - // 7 - // 8 - void f(); -}} -// CHECK-MESSAGES: :[[@LINE-1]]:2: warning: namespace 'n3' not terminated with -// CHECK-MESSAGES: :[[@LINE-14]]:11: note: namespace 'n3' starts here -// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: namespace 'n1::n2' not terminated with a closing comment [google-readability-namespace-comments] -// CHECK-MESSAGES: :[[@LINE-17]]:11: note: namespace 'n1::n2' starts here -// CHECK-FIXES: } // namespace n3 -// CHECK-FIXES: } // namespace n1::n2 - _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits