r264745 - Added formatAndApplyAllReplacements that works on multiple files in libTooling.
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 29 11:27:22 PDT 2016
Hi Teresa,
Thanks a lot! I have patched the error.
Thanks,
Eric
On Tue, Mar 29, 2016 at 8:08 PM Teresa Johnson <tejohnson at google.com> wrote:
> 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/296d2e39/attachment-0001.html>
More information about the cfe-commits
mailing list