[clang-tools-extra] r315580 - Revert "Fix nested namespaces in google-readability-nested-namespace-comments."
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 12 07:25:16 PDT 2017
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
-
More information about the cfe-commits
mailing list