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.

llvm-svn: 315580
This commit is contained in:
Alexander Kornienko 2017-10-12 14:25:16 +00:00
parent 4a5f522d4d
commit 5d62569b04
3 changed files with 10 additions and 77 deletions

View File

@ -23,7 +23,7 @@ NamespaceCommentCheck::NamespaceCommentCheck(StringRef Name,
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(const NamespaceDecl *ND,
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 MatchFinder::MatchResult &Result) {
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 MatchFinder::MatchResult &Result) {
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 MatchFinder::MatchResult &Result) {
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;
}

View File

@ -34,7 +34,6 @@ private:
llvm::Regex NamespaceCommentPattern;
const unsigned ShortNamespaceLines;
const unsigned SpacesBeforeComments;
llvm::SmallVector<SourceLocation, 4> Ends;
};
} // namespace readability

View File

@ -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