[PATCH] D25397: [change-namespace] don't miss comments in the beginning of a namespace block.
This revision was automatically updated to reflect the committed changes. Closed by commit rL284011: [change-namespace] don't miss comments in the beginning of a namespace block. (authored by ioeric). Changed prior to commit: https://reviews.llvm.org/D25397?vs=74038&id=74368#toc Repository: rL LLVM https://reviews.llvm.org/D25397 Files: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp Index: clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp === --- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp +++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp @@ -552,6 +552,38 @@ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); } +TEST_F(ChangeNamespaceTest, CommentsBeforeMovedClass) { + std::string Code = "namespace na {\n" + "namespace nb {\n" + "\n\n" + "// Wild comments.\n" + "\n" + "// Comments.\n" + "// More comments.\n" + "class B {\n" + " // Private comments.\n" + " int a;\n" + "};\n" + "}\n" + "}"; + std::string Expected = "\n" + "\n" + "namespace x {\n" + "namespace y {\n" + "\n\n" + "// Wild comments.\n" + "\n" + "// Comments.\n" + "// More comments.\n" + "class B {\n" + " // Private comments.\n" + " int a;\n" + "};\n" + "} // namespace y\n" + "} // namespace x\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + } // anonymous namespace } // namespace change_namespace } // namespace clang Index: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp === --- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp +++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp @@ -82,33 +82,42 @@ return CurrentNs; } -// FIXME: get rid of this helper function if this is supported in clang-refactor -// library. -SourceLocation getStartOfNextLine(SourceLocation Loc, const SourceManager &SM, - const LangOptions &LangOpts) { +static std::unique_ptr +getLexerStartingFromLoc(SourceLocation Loc, const SourceManager &SM, +const LangOptions &LangOpts) { if (Loc.isMacroID() && !Lexer::isAtEndOfMacroExpansion(Loc, SM, LangOpts, &Loc)) -return SourceLocation(); +return nullptr; // Break down the source location. std::pair LocInfo = SM.getDecomposedLoc(Loc); // Try to load the file buffer. bool InvalidTemp = false; llvm::StringRef File = SM.getBufferData(LocInfo.first, &InvalidTemp); if (InvalidTemp) -return SourceLocation(); +return nullptr; const char *TokBegin = File.data() + LocInfo.second; // Lex from the start of the given location. - Lexer Lex(SM.getLocForStartOfFile(LocInfo.first), LangOpts, File.begin(), -TokBegin, File.end()); + return llvm::make_unique(SM.getLocForStartOfFile(LocInfo.first), + LangOpts, File.begin(), TokBegin, File.end()); +} +// FIXME: get rid of this helper function if this is supported in clang-refactor +// library. +static SourceLocation getStartOfNextLine(SourceLocation Loc, + const SourceManager &SM, + const LangOptions &LangOpts) { + std::unique_ptr Lex = getLexerStartingFromLoc(Loc, SM, LangOpts); + if (!Lex.get()) +return SourceLocation(); llvm::SmallVector Line; // FIXME: this is a bit hacky to get ReadToEndOfLine work. - Lex.setParsingPreprocessorDirective(true); - Lex.ReadToEndOfLine(&Line); + Lex->setParsingPreprocessorDirective(true); + Lex->ReadToEndOfLine(&Line); auto End = Loc.getLocWithOffset(Line.size()); - return SM.getLocForEndOfFile(LocInfo.first) == End ? End - : End.getLocWithOffset(1); + return SM.getLocForEndOfFile(SM.getDecomposedLoc(Loc).first) == End + ? End + : End.getLocWithOffset(1); } // Returns `R` with new range that refers to code after `Replaces` being @@ -365,6 +374,23 @@ } } +static SourceLocation getLocAfterNamespaceLBrace(const NamespaceDecl *NsDecl, + const SourceManager &SM, + const Lan
[PATCH] D25397: [change-namespace] don't miss comments in the beginning of a namespace block.
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. LGTM. Comment at: change-namespace/ChangeNamespace.cpp:387 + Token Tok; + while (!Lex->LexFromRawLexer(Tok) && Tok.isNot(tok::TokenKind::l_brace)) { + } ioeric wrote: > hokein wrote: > > Maybe we can use `findLocationAfterToken` here? > This doesn't seem to work in this case. Note that we are lexing from the > beginning of `NsDecl`, which is `namespace` or `inline`, and the next token > is not necessarily `{`. > > /// \brief Checks that the given token is the first token that occurs after > the > /// given location (this excludes comments and whitespace). Returns the > location > /// immediately after the specified token. If the token is not found or the > /// location is inside a macro, the returned source location will be > invalid. > SourceLocation Lexer::findLocationAfterToken(...); > I see. Thanks for the clarification. https://reviews.llvm.org/D25397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25397: [change-namespace] don't miss comments in the beginning of a namespace block.
ioeric added inline comments. Comment at: change-namespace/ChangeNamespace.cpp:387 + Token Tok; + while (!Lex->LexFromRawLexer(Tok) && Tok.isNot(tok::TokenKind::l_brace)) { + } hokein wrote: > Maybe we can use `findLocationAfterToken` here? This doesn't seem to work in this case. Note that we are lexing from the beginning of `NsDecl`, which is `namespace` or `inline`, and the next token is not necessarily `{`. /// \brief Checks that the given token is the first token that occurs after the /// given location (this excludes comments and whitespace). Returns the location /// immediately after the specified token. If the token is not found or the /// location is inside a macro, the returned source location will be invalid. SourceLocation Lexer::findLocationAfterToken(...); https://reviews.llvm.org/D25397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25397: [change-namespace] don't miss comments in the beginning of a namespace block.
hokein added inline comments. Comment at: change-namespace/ChangeNamespace.cpp:387 + Token Tok; + while (!Lex->LexFromRawLexer(Tok) && Tok.isNot(tok::TokenKind::l_brace)) { + } Maybe we can use `findLocationAfterToken` here? https://reviews.llvm.org/D25397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25397: [change-namespace] don't miss comments in the beginning of a namespace block.
ioeric created this revision. ioeric added a reviewer: hokein. ioeric added a subscriber: cfe-commits. https://reviews.llvm.org/D25397 Files: change-namespace/ChangeNamespace.cpp unittests/change-namespace/ChangeNamespaceTests.cpp Index: unittests/change-namespace/ChangeNamespaceTests.cpp === --- unittests/change-namespace/ChangeNamespaceTests.cpp +++ unittests/change-namespace/ChangeNamespaceTests.cpp @@ -552,6 +552,38 @@ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); } +TEST_F(ChangeNamespaceTest, CommentsBeforeMovedClass) { + std::string Code = "namespace na {\n" + "namespace nb {\n" + "\n\n" + "// Wild comments.\n" + "\n" + "// Comments.\n" + "// More comments.\n" + "class B {\n" + " // Private comments.\n" + " int a;\n" + "};\n" + "}\n" + "}"; + std::string Expected = "\n" + "\n" + "namespace x {\n" + "namespace y {\n" + "\n\n" + "// Wild comments.\n" + "\n" + "// Comments.\n" + "// More comments.\n" + "class B {\n" + " // Private comments.\n" + " int a;\n" + "};\n" + "} // namespace y\n" + "} // namespace x\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + } // anonymous namespace } // namespace change_namespace } // namespace clang Index: change-namespace/ChangeNamespace.cpp === --- change-namespace/ChangeNamespace.cpp +++ change-namespace/ChangeNamespace.cpp @@ -82,33 +82,42 @@ return CurrentNs; } -// FIXME: get rid of this helper function if this is supported in clang-refactor -// library. -SourceLocation getStartOfNextLine(SourceLocation Loc, const SourceManager &SM, - const LangOptions &LangOpts) { +static std::unique_ptr +getLexerStartingFromLoc(SourceLocation Loc, const SourceManager &SM, +const LangOptions &LangOpts) { if (Loc.isMacroID() && !Lexer::isAtEndOfMacroExpansion(Loc, SM, LangOpts, &Loc)) -return SourceLocation(); +return nullptr; // Break down the source location. std::pair LocInfo = SM.getDecomposedLoc(Loc); // Try to load the file buffer. bool InvalidTemp = false; llvm::StringRef File = SM.getBufferData(LocInfo.first, &InvalidTemp); if (InvalidTemp) -return SourceLocation(); +return nullptr; const char *TokBegin = File.data() + LocInfo.second; // Lex from the start of the given location. - Lexer Lex(SM.getLocForStartOfFile(LocInfo.first), LangOpts, File.begin(), -TokBegin, File.end()); + return llvm::make_unique(SM.getLocForStartOfFile(LocInfo.first), + LangOpts, File.begin(), TokBegin, File.end()); +} +// FIXME: get rid of this helper function if this is supported in clang-refactor +// library. +static SourceLocation getStartOfNextLine(SourceLocation Loc, + const SourceManager &SM, + const LangOptions &LangOpts) { + std::unique_ptr Lex = getLexerStartingFromLoc(Loc, SM, LangOpts); + if (!Lex.get()) +return SourceLocation(); llvm::SmallVector Line; // FIXME: this is a bit hacky to get ReadToEndOfLine work. - Lex.setParsingPreprocessorDirective(true); - Lex.ReadToEndOfLine(&Line); + Lex->setParsingPreprocessorDirective(true); + Lex->ReadToEndOfLine(&Line); auto End = Loc.getLocWithOffset(Line.size()); - return SM.getLocForEndOfFile(LocInfo.first) == End ? End - : End.getLocWithOffset(1); + return SM.getLocForEndOfFile(SM.getDecomposedLoc(Loc).first) == End + ? End + : End.getLocWithOffset(1); } // Returns `R` with new range that refers to code after `Replaces` being @@ -365,6 +374,23 @@ } } +static SourceLocation getLocAfterNamespaceLBrace(const NamespaceDecl *NsDecl, + const SourceManager &SM, + const LangOptions &LangOpts) { + std::unique_ptr Lex = + getLexerStartingFromLoc(NsDecl->getLocStart(), SM, LangOpts); + assert(Lex.get() && + "Failed to create lexer from the beginning of namespace."); + if (!Lex.get()) +return SourceLocation(); + Token Tok; + while (!Lex->LexFromRawLexer(Tok) && Tok.isNot(tok::TokenKind::l_brace)) { + } + return Tok.isNot(tok::TokenKind::l_bra