r264745 - Added formatAndApplyAllReplacements that works on multiple files in libTooling.
Teresa Johnson via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 29 11:08:22 PDT 2016
Hi Eric,
I'm getting a build error:
In file included from
/usr/local/google/home/tejohnson/llvm/llvm_16/tools/clang/lib/Tooling/Refactoring.cpp:21:
/usr/local/google/home/tejohnson/llvm/llvm_16/tools/clang/include/clang/Tooling/Refactoring.h:32:1:
error: class 'FormatStyle' was previously declared as a struct
[-Werror,-Wmismatched-tags]
class FormatStyle;
^
/usr/local/google/home/tejohnson/llvm/llvm_16/tools/clang/include/clang/Format/Format.h:43:8:
note: previous use is here
struct FormatStyle {
^
/usr/local/google/home/tejohnson/llvm/llvm_16/tools/clang/include/clang/Tooling/Refactoring.h:32:1:
note: did you mean struct here?
class FormatStyle;
^~~~~
struct
1 error generated.
ninja: build stopped: subcommand failed.
Looks like you added the "class FormatStyle" in Refactoring.h, looks like
this should be a struct instead.
Thanks,
Teresa
On Tue, Mar 29, 2016 at 9:31 AM, Eric Liu via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Author: ioeric
> Date: Tue Mar 29 11:31:53 2016
> New Revision: 264745
>
> URL: http://llvm.org/viewvc/llvm-project?rev=264745&view=rev
> Log:
> Added formatAndApplyAllReplacements that works on multiple files in
> libTooling.
>
> Summary:
> formatAndApplyAllReplacements takes a set of Replacements, applies them on
> a
> Rewriter, and reformats the changed code.
>
> Reviewers: klimek, djasper
>
> Subscribers: ioeric, klimek, cfe-commits
>
> Differential Revision: http://reviews.llvm.org/D17852
>
> Modified:
> cfe/trunk/include/clang/Basic/SourceManager.h
> cfe/trunk/include/clang/Format/Format.h
> cfe/trunk/include/clang/Tooling/Core/Replacement.h
> cfe/trunk/include/clang/Tooling/Refactoring.h
> cfe/trunk/lib/Format/Format.cpp
> cfe/trunk/lib/Tooling/CMakeLists.txt
> cfe/trunk/lib/Tooling/Core/Replacement.cpp
> cfe/trunk/lib/Tooling/Refactoring.cpp
> cfe/trunk/unittests/Format/FormatTest.cpp
> cfe/trunk/unittests/Tooling/CMakeLists.txt
> cfe/trunk/unittests/Tooling/RefactoringTest.cpp
>
> Modified: cfe/trunk/include/clang/Basic/SourceManager.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=264745&r1=264744&r2=264745&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/SourceManager.h (original)
> +++ cfe/trunk/include/clang/Basic/SourceManager.h Tue Mar 29 11:31:53 2016
> @@ -797,6 +797,15 @@ public:
> IncludeLoc, FileCharacter, LoadedID,
> LoadedOffset);
> }
>
> + /// \brief Get the FileID for \p SourceFile if it exists. Otherwise,
> create a
> + /// new FileID for the \p SourceFile.
> + FileID getOrCreateFileID(const FileEntry *SourceFile,
> + SrcMgr::CharacteristicKind FileCharacter) {
> + FileID ID = translateFile(SourceFile);
> + return ID.isValid() ? ID : createFileID(SourceFile, SourceLocation(),
> + FileCharacter);
> + }
> +
> /// \brief Return a new SourceLocation that encodes the
> /// fact that a token from SpellingLoc should actually be referenced
> from
> /// ExpansionLoc, and that it represents the expansion of a macro
> argument
>
> Modified: cfe/trunk/include/clang/Format/Format.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=264745&r1=264744&r2=264745&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Format/Format.h (original)
> +++ cfe/trunk/include/clang/Format/Format.h Tue Mar 29 11:31:53 2016
> @@ -766,22 +766,6 @@ tooling::Replacements formatReplacements
> const tooling::Replacements
> &Replaces,
> const FormatStyle &Style);
>
> -/// \brief In addition to applying all replacements in \p Replaces to \p
> Code,
> -/// this function also reformats the changed code after applying
> replacements.
> -///
> -/// \pre Replacements must be for the same file and conflict-free.
> -///
> -/// Replacement applications happen independently of the success of
> -/// other applications.
> -///
> -/// \returns the changed code with all replacements applied and
> formatted, if
> -/// successful. An empty string otherwise.
> -///
> -/// See also "include/clang/Tooling/Core/Replacements.h".
> -std::string applyAllReplacementsAndFormat(StringRef Code,
> - const tooling::Replacements
> &Replaces,
> - const FormatStyle &Style);
> -
> /// \brief Reformats the given \p Ranges in the file \p ID.
> ///
> /// Each range is extended on either end to its next bigger logic unit,
> i.e.
>
> Modified: cfe/trunk/include/clang/Tooling/Core/Replacement.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Core/Replacement.h?rev=264745&r1=264744&r2=264745&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Tooling/Core/Replacement.h (original)
> +++ cfe/trunk/include/clang/Tooling/Core/Replacement.h Tue Mar 29 11:31:53
> 2016
> @@ -22,6 +22,7 @@
> #include "clang/Basic/LangOptions.h"
> #include "clang/Basic/SourceLocation.h"
> #include "llvm/ADT/StringRef.h"
> +#include <map>
> #include <set>
> #include <string>
> #include <vector>
> @@ -220,12 +221,16 @@ bool applyAllReplacements(const std::vec
> /// replacements cannot be applied, this returns an empty \c string.
> std::string applyAllReplacements(StringRef Code, const Replacements
> &Replaces);
>
> -/// \brief Calculate the ranges in a single file that are affected by the
> +/// \brief Calculates the ranges in a single file that are affected by the
> /// Replacements.
> ///
> /// \pre Replacements must be for the same file.
> -std::vector<tooling::Range>
> -calculateChangedRangesInFile(const tooling::Replacements &Replaces);
> +std::vector<Range> calculateChangedRanges(const Replacements &Replaces);
> +
> +/// \brief Groups a random set of replacements by file path. Replacements
> +/// related to the same file entry are put into the same vector.
> +std::map<std::string, Replacements>
> +groupReplacementsByFile(const Replacements &Replaces);
>
> /// \brief Merges two sets of replacements with the second set referring
> to the
> /// code after applying the first set. Within both 'First' and 'Second',
>
> Modified: cfe/trunk/include/clang/Tooling/Refactoring.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring.h?rev=264745&r1=264744&r2=264745&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Tooling/Refactoring.h (original)
> +++ cfe/trunk/include/clang/Tooling/Refactoring.h Tue Mar 29 11:31:53 2016
> @@ -27,6 +27,12 @@ namespace clang {
>
> class Rewriter;
>
> +namespace format {
> +
> +class FormatStyle;
> +
> +} // namespace format
> +
> namespace tooling {
>
> /// \brief A tool to run refactorings.
> @@ -68,6 +74,24 @@ private:
> Replacements Replace;
> };
>
> +/// \brief Groups \p Replaces by the file path and applies each group of
> +/// Replacements on the related file in \p Rewriter. In addition to
> applying
> +/// given Replacements, this function also formats the changed code.
> +///
> +/// \pre Replacements must be conflict-free.
> +///
> +/// Replacement applications happen independently of the success of other
> +/// applications.
> +///
> +/// \param[in] Replaces Replacements to apply.
> +/// \param[in] Rewrite The `Rewritter` to apply replacements on.
> +/// \param[in] Style The style name used for reformatting. See
> ```getStyle``` in
> +/// "include/clang/Format/Format.h" for all possible style forms.
> +///
> +/// \returns true if all replacements applied and formatted. false
> otherwise.
> +bool formatAndApplyAllReplacements(const Replacements &Replaces,
> + Rewriter &Rewrite, StringRef Style =
> "file");
> +
> } // end namespace tooling
> } // end namespace clang
>
>
> Modified: cfe/trunk/lib/Format/Format.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=264745&r1=264744&r2=264745&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Format/Format.cpp (original)
> +++ cfe/trunk/lib/Format/Format.cpp Tue Mar 29 11:31:53 2016
> @@ -1998,7 +1998,7 @@ tooling::Replacements formatReplacements
>
> std::string NewCode = applyAllReplacements(Code, Replaces);
> std::vector<tooling::Range> ChangedRanges =
> - tooling::calculateChangedRangesInFile(Replaces);
> + tooling::calculateChangedRanges(Replaces);
> StringRef FileName = Replaces.begin()->getFilePath();
> tooling::Replacements FormatReplaces =
> reformat(Style, NewCode, ChangedRanges, FileName);
> @@ -2009,16 +2009,6 @@ tooling::Replacements formatReplacements
> return MergedReplacements;
> }
>
> -std::string applyAllReplacementsAndFormat(StringRef Code,
> - const tooling::Replacements
> &Replaces,
> - const FormatStyle &Style) {
> - tooling::Replacements NewReplacements =
> - formatReplacements(Code, Replaces, Style);
> - if (NewReplacements.empty())
> - return Code; // Exit early to avoid overhead in
> `applyAllReplacements`.
> - return applyAllReplacements(Code, NewReplacements);
> -}
> -
> tooling::Replacements reformat(const FormatStyle &Style,
> SourceManager &SourceMgr, FileID ID,
> ArrayRef<CharSourceRange> Ranges,
>
> Modified: cfe/trunk/lib/Tooling/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/CMakeLists.txt?rev=264745&r1=264744&r2=264745&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Tooling/CMakeLists.txt (original)
> +++ cfe/trunk/lib/Tooling/CMakeLists.txt Tue Mar 29 11:31:53 2016
> @@ -17,6 +17,7 @@ add_clang_library(clangTooling
> clangASTMatchers
> clangBasic
> clangDriver
> + clangFormat
> clangFrontend
> clangLex
> clangRewrite
>
> Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=264745&r1=264744&r2=264745&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original)
> +++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Tue Mar 29 11:31:53 2016
> @@ -58,14 +58,8 @@ bool Replacement::apply(Rewriter &Rewrit
> const FileEntry *Entry = SM.getFileManager().getFile(FilePath);
> if (!Entry)
> return false;
> - FileID ID;
> - // FIXME: Use SM.translateFile directly.
> - SourceLocation Location = SM.translateFileLineCol(Entry, 1, 1);
> - ID = Location.isValid() ?
> - SM.getFileID(Location) :
> - SM.createFileID(Entry, SourceLocation(), SrcMgr::C_User);
> - // FIXME: We cannot check whether Offset + Length is in the file, as
> - // the remapping API is not public in the RewriteBuffer.
> +
> + FileID ID = SM.getOrCreateFileID(Entry, SrcMgr::C_User);
> const SourceLocation Start =
> SM.getLocForStartOfFile(ID).
> getLocWithOffset(ReplacementRange.getOffset());
> @@ -256,6 +250,8 @@ bool applyAllReplacements(const std::vec
> }
>
> std::string applyAllReplacements(StringRef Code, const Replacements
> &Replaces) {
> + if (Replaces.empty()) return Code;
> +
> IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem(
> new vfs::InMemoryFileSystem);
> FileManager Files(FileSystemOptions(), InMemoryFileSystem);
> @@ -282,7 +278,7 @@ std::string applyAllReplacements(StringR
> return Result;
> }
>
> -std::vector<Range> calculateChangedRangesInFile(const Replacements
> &Replaces) {
> +std::vector<Range> calculateChangedRanges(const Replacements &Replaces) {
> std::vector<Range> ChangedRanges;
> int Shift = 0;
> for (const Replacement &R : Replaces) {
> @@ -390,6 +386,15 @@ private:
> };
> } // namespace
>
> +std::map<std::string, Replacements>
> +groupReplacementsByFile(const Replacements &Replaces) {
> + std::map<std::string, Replacements> FileToReplaces;
> + for (const auto &Replace : Replaces) {
> + FileToReplaces[Replace.getFilePath()].insert(Replace);
> + }
> + return FileToReplaces;
> +}
> +
> Replacements mergeReplacements(const Replacements &First,
> const Replacements &Second) {
> if (First.empty() || Second.empty())
>
> Modified: cfe/trunk/lib/Tooling/Refactoring.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring.cpp?rev=264745&r1=264744&r2=264745&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Tooling/Refactoring.cpp (original)
> +++ cfe/trunk/lib/Tooling/Refactoring.cpp Tue Mar 29 11:31:53 2016
> @@ -14,6 +14,7 @@
> #include "clang/Basic/DiagnosticOptions.h"
> #include "clang/Basic/FileManager.h"
> #include "clang/Basic/SourceManager.h"
> +#include "clang/Format/Format.h"
> #include "clang/Frontend/TextDiagnosticPrinter.h"
> #include "clang/Lex/Lexer.h"
> #include "clang/Rewrite/Core/Rewriter.h"
> @@ -61,5 +62,29 @@ int RefactoringTool::saveRewrittenFiles(
> return Rewrite.overwriteChangedFiles() ? 1 : 0;
> }
>
> +bool formatAndApplyAllReplacements(const Replacements &Replaces,
> + Rewriter &Rewrite, StringRef Style) {
> + SourceManager &SM = Rewrite.getSourceMgr();
> + FileManager &Files = SM.getFileManager();
> +
> + auto FileToReplaces = groupReplacementsByFile(Replaces);
> +
> + bool Result = true;
> + for (auto &FileAndReplaces : FileToReplaces) {
> + const std::string FilePath = FileAndReplaces.first;
> + auto &CurReplaces = FileAndReplaces.second;
> +
> + const FileEntry *Entry = Files.getFile(FilePath);
> + FileID ID = SM.getOrCreateFileID(Entry, SrcMgr::C_User);
> + StringRef Code = SM.getBufferData(ID);
> +
> + format::FormatStyle CurStyle = format::getStyle(Style, FilePath,
> "LLVM");
> + Replacements NewReplacements =
> + format::formatReplacements(Code, CurReplaces, CurStyle);
> + Result = applyAllReplacements(NewReplacements, Rewrite) && Result;
> + }
> + return Result;
> +}
> +
> } // end namespace tooling
> } // end namespace clang
>
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=264745&r1=264744&r2=264745&view=diff
>
> ==============================================================================
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Mar 29 11:31:53 2016
> @@ -11270,7 +11270,8 @@ TEST_F(ReplacementTest, FormatCodeAfterR
>
> format::FormatStyle Style = format::getLLVMStyle();
> Style.ColumnLimit = 20; // Set column limit to 20 to increase
> readibility.
> - EXPECT_EQ(Expected, applyAllReplacementsAndFormat(Code, Replaces,
> Style));
> + EXPECT_EQ(Expected, applyAllReplacements(
> + Code, formatReplacements(Code, Replaces,
> Style)));
> }
>
> } // end namespace
>
> Modified: cfe/trunk/unittests/Tooling/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/CMakeLists.txt?rev=264745&r1=264744&r2=264745&view=diff
>
> ==============================================================================
> --- cfe/trunk/unittests/Tooling/CMakeLists.txt (original)
> +++ cfe/trunk/unittests/Tooling/CMakeLists.txt Tue Mar 29 11:31:53 2016
> @@ -30,6 +30,7 @@ target_link_libraries(ToolingTests
> clangAST
> clangASTMatchers
> clangBasic
> + clangFormat
> clangFrontend
> clangLex
> clangRewrite
>
> Modified: cfe/trunk/unittests/Tooling/RefactoringTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringTest.cpp?rev=264745&r1=264744&r2=264745&view=diff
>
> ==============================================================================
> --- cfe/trunk/unittests/Tooling/RefactoringTest.cpp (original)
> +++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp Tue Mar 29 11:31:53
> 2016
> @@ -18,6 +18,7 @@
> #include "clang/Basic/FileManager.h"
> #include "clang/Basic/LangOptions.h"
> #include "clang/Basic/SourceManager.h"
> +#include "clang/Format/Format.h"
> #include "clang/Frontend/CompilerInstance.h"
> #include "clang/Frontend/FrontendAction.h"
> #include "clang/Frontend/TextDiagnosticPrinter.h"
> @@ -166,6 +167,39 @@ TEST_F(ReplacementTest, ApplyAllFailsIfO
> EXPECT_EQ("z", Context.getRewrittenText(IDz));
> }
>
> +TEST_F(ReplacementTest, MultipleFilesReplaceAndFormat) {
> + // Column limit is 20.
> + std::string Code1 = "Long *a =\n"
> + " new Long();\n"
> + "long x = 1;";
> + std::string Expected1 = "auto a = new Long();\n"
> + "long x =\n"
> + " 12345678901;";
> + std::string Code2 = "int x = 123;\n"
> + "int y = 0;";
> + std::string Expected2 = "int x =\n"
> + " 1234567890123;\n"
> + "int y = 10;";
> + FileID ID1 = Context.createInMemoryFile("format_1.cpp", Code1);
> + FileID ID2 = Context.createInMemoryFile("format_2.cpp", Code2);
> +
> + tooling::Replacements Replaces;
> + // Scrambled the order of replacements.
> + Replaces.insert(tooling::Replacement(
> + Context.Sources, Context.getLocation(ID2, 1, 12), 0, "4567890123"));
> + Replaces.insert(tooling::Replacement(
> + Context.Sources, Context.getLocation(ID1, 1, 1), 6, "auto "));
> + Replaces.insert(tooling::Replacement(
> + Context.Sources, Context.getLocation(ID2, 2, 9), 1, "10"));
> + Replaces.insert(tooling::Replacement(
> + Context.Sources, Context.getLocation(ID1, 3, 10), 1,
> "12345678901"));
> +
> + EXPECT_TRUE(formatAndApplyAllReplacements(
> + Replaces, Context.Rewrite, "{BasedOnStyle: LLVM, ColumnLimit:
> 20}"));
> + EXPECT_EQ(Expected1, Context.getRewrittenText(ID1));
> + EXPECT_EQ(Expected2, Context.getRewrittenText(ID2));
> +}
> +
> TEST(ShiftedCodePositionTest, FindsNewCodePosition) {
> Replacements Replaces;
> Replaces.insert(Replacement("", 0, 1, ""));
> @@ -426,7 +460,7 @@ TEST(Range, CalculateRangesOfReplacement
> Replaces.insert(Replacement("foo", 10, 1, "zzzzzz"));
> Replaces.insert(Replacement("foo", 11, 0, "oooooooooooooooo"));
>
> - std::vector<Range> Ranges = calculateChangedRangesInFile(Replaces);
> + std::vector<Range> Ranges = calculateChangedRanges(Replaces);
>
> EXPECT_EQ(3ul, Ranges.size());
> EXPECT_TRUE(Ranges[0].getOffset() == 0);
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
--
Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160329/0e039052/attachment-0001.html>
More information about the cfe-commits
mailing list