Ping :) On Fri, May 11, 2012 at 1:33 PM, Manuel Klimek <[email protected]> wrote: > On Tue, May 8, 2012 at 11:49 AM, Manuel Klimek <[email protected]> wrote: >> On Thu, May 3, 2012 at 6:40 PM, Manuel Klimek <[email protected]> wrote: >>> as requested by Doug in the clangRefactoring review... in addition, >>> this patch adds a unit test for the Rewriter (into Tooling/ so we >>> don't link yet another unit test), and pulls out a RewriterTestContext >>> which I need for clangRefactoring when it's checked in - but it's also >>> really nice to unit test Rewriter functionality... > > Inlining the patch in the hope that that speeds up review :) > > First, the changed code: > > Index: include/clang/Rewrite/Rewriter.h > =================================================================== > --- include/clang/Rewrite/Rewriter.h (revision 156059) > +++ include/clang/Rewrite/Rewriter.h (working copy) > @@ -279,6 +279,13 @@ > buffer_iterator buffer_begin() { return RewriteBuffers.begin(); } > buffer_iterator buffer_end() { return RewriteBuffers.end(); } > > + /// SaveFiles - Save all changed files to disk. > + /// > + /// Returns whether all changes were saves successfully. > + /// Outputs diagnostics via the source manager's diagnostic engine > + /// in case of an error. > + bool OverwriteChangedFiles(); > + > private: > unsigned getLocationOffsetAndFileID(SourceLocation Loc, FileID &FID) const; > }; > Index: lib/Rewrite/Rewriter.cpp > =================================================================== > --- lib/Rewrite/Rewriter.cpp (revision 156059) > +++ lib/Rewrite/Rewriter.cpp (working copy) > @@ -15,8 +15,10 @@ > #include "clang/Rewrite/Rewriter.h" > #include "clang/AST/Stmt.h" > #include "clang/AST/Decl.h" > +#include "clang/Basic/FileManager.h" > +#include "clang/Basic/SourceManager.h" > #include "clang/Lex/Lexer.h" > -#include "clang/Basic/SourceManager.h" > +#include "clang/Frontend/FrontendDiagnostic.h" > #include "llvm/ADT/SmallString.h" > using namespace clang; > > @@ -412,3 +414,23 @@ > > return false; > } > + > +bool Rewriter::OverwriteChangedFiles() { > + bool AllWritten = true; > + for (buffer_iterator I = buffer_begin(), E = buffer_end(); I != E; ++I) { > + const FileEntry *Entry = > + getSourceMgr().getFileEntryForID(I->first); > + std::string ErrorInfo; > + llvm::raw_fd_ostream FileStream( > + Entry->getName(), ErrorInfo, llvm::raw_fd_ostream::F_Binary); > + if (!ErrorInfo.empty()) { > + > getSourceMgr().getDiagnostics().Report(clang::diag::err_fe_unable_to_open_output) > + << Entry->getName() << ErrorInfo; > + AllWritten = false; > + continue; > + } > + I->second.write(FileStream); > + FileStream.flush(); > + } > + return AllWritten; > +} > > Second, the testing overhead; note that the RewriterTestContext will > also be used in a subsequent CL which is currently under review, which > is why I pulled out a class. > > > Index: unittests/Tooling/RewriterTest.cpp > =================================================================== > --- unittests/Tooling/RewriterTest.cpp (revision 0) > +++ unittests/Tooling/RewriterTest.cpp (revision 0) > @@ -0,0 +1,37 @@ > +//===- unittest/Tooling/RewriterTest.cpp > ----------------------------------===// > +// > +// The LLVM Compiler Infrastructure > +// > +// This file is distributed under the University of Illinois Open Source > +// License. See LICENSE.TXT for details. > +// > +//===----------------------------------------------------------------------===// > + > +#include "RewriterTestContext.h" > +#include "gtest/gtest.h" > + > +namespace clang { > + > +TEST(Rewriter, OverwritesChangedFiles) { > + RewriterTestContext Context; > + FileID ID = Context.createOnDiskFile("t.cpp", > "line1\nline2\nline3\nline4"); > + Context.Rewrite.ReplaceText(Context.getLocation(ID, 2, 1), 5, "replaced"); > + EXPECT_TRUE(Context.Rewrite.OverwriteChangedFiles()); > + EXPECT_EQ("line1\nreplaced\nline3\nline4", > + Context.getFileContentFromDisk("t.cpp")); > +} > + > +TEST(Rewriter, ContinuesOverwritingFilesOnError) { > + RewriterTestContext Context; > + FileID FailingID = Context.createInMemoryFile("invalid/failing.cpp", > "test"); > + Context.Rewrite.ReplaceText(Context.getLocation(FailingID, 1, 2), > 1, "other"); > + FileID WorkingID = Context.createOnDiskFile( > + "working.cpp", "line1\nline2\nline3\nline4"); > + Context.Rewrite.ReplaceText(Context.getLocation(WorkingID, 2, 1), 5, > + "replaced"); > + EXPECT_FALSE(Context.Rewrite.OverwriteChangedFiles()); > + EXPECT_EQ("line1\nreplaced\nline3\nline4", > + Context.getFileContentFromDisk("working.cpp")); > +} > + > +} // end namespace clang > > Index: unittests/Tooling/RewriterTestContext.h > =================================================================== > --- unittests/Tooling/RewriterTestContext.h (revision 0) > +++ unittests/Tooling/RewriterTestContext.h (revision 0) > @@ -0,0 +1,120 @@ > +//===--- RewriterTestContext.h ----------------------------------*- > C++ -*-===// > +// > +// The LLVM Compiler Infrastructure > +// > +// This file is distributed under the University of Illinois Open Source > +// License. See LICENSE.TXT for details. > +// > +//===----------------------------------------------------------------------===// > +// > +// This file defines a utility class for Rewriter related tests. > +// > +//===----------------------------------------------------------------------===// > + > +#ifndef LLVM_CLANG_REWRITER_TEST_CONTEXT_H > +#define LLVM_CLANG_REWRITER_TEST_CONTEXT_H > + > +#include "clang/Basic/Diagnostic.h" > +#include "clang/Basic/FileManager.h" > +#include "clang/Basic/LangOptions.h" > +#include "clang/Basic/SourceManager.h" > +#include "clang/Frontend/DiagnosticOptions.h" > +#include "clang/Frontend/TextDiagnosticPrinter.h" > +#include "clang/Rewrite/Rewriter.h" > +#include "llvm/Support/Path.h" > +#include "llvm/Support/raw_ostream.h" > + > +namespace clang { > + > +/// \brief A class that sets up a ready to use Rewriter. > +/// > +/// Useful in unit tests that need a Rewriter. Creates all dependencies > +/// of a Rewriter with default values for testing and provides convenience > +/// methods, which help with writing tests that change files. > +class RewriterTestContext { > + public: > + RewriterTestContext() > + : Diagnostics(llvm::IntrusiveRefCntPtr<DiagnosticIDs>()), > + DiagnosticPrinter(llvm::outs(), DiagnosticOptions()), > + Files((FileSystemOptions())), > + Sources(Diagnostics, Files), > + Rewrite(Sources, Options) { > + Diagnostics.setClient(&DiagnosticPrinter, false); > + } > + > + ~RewriterTestContext() { > + if (TemporaryDirectory.isValid()) { > + std::string ErrorInfo; > + TemporaryDirectory.eraseFromDisk(true, &ErrorInfo); > + assert(ErrorInfo.empty()); > + } > + } > + > + FileID createInMemoryFile(StringRef Name, StringRef Content) { > + const llvm::MemoryBuffer *Source = > + llvm::MemoryBuffer::getMemBuffer(Content); > + const FileEntry *Entry = > + Files.getVirtualFile(Name, Source->getBufferSize(), 0); > + Sources.overrideFileContents(Entry, Source, true); > + assert(Entry != NULL); > + return Sources.createFileID(Entry, SourceLocation(), SrcMgr::C_User); > + } > + > + FileID createOnDiskFile(StringRef Name, StringRef Content) { > + if (!TemporaryDirectory.isValid()) { > + std::string ErrorInfo; > + TemporaryDirectory = > llvm::sys::Path::GetTemporaryDirectory(&ErrorInfo); > + assert(ErrorInfo.empty()); > + } > + llvm::SmallString<1024> Path(TemporaryDirectory.str()); > + llvm::sys::path::append(Path, Name); > + std::string ErrorInfo; > + llvm::raw_fd_ostream OutStream(Path.c_str(), > + ErrorInfo, > llvm::raw_fd_ostream::F_Binary); > + assert(ErrorInfo.empty()); > + OutStream << Content; > + OutStream.close(); > + const FileEntry *File = Files.getFile(Path); > + assert(File != NULL); > + return Sources.createFileID(File, SourceLocation(), SrcMgr::C_User); > + } > + > + SourceLocation getLocation(FileID ID, unsigned Line, unsigned Column) { > + SourceLocation Result = Sources.translateFileLineCol( > + Sources.getFileEntryForID(ID), Line, Column); > + assert(Result.isValid()); > + return Result; > + } > + > + std::string getRewrittenText(FileID ID) { > + std::string Result; > + llvm::raw_string_ostream OS(Result); > + Rewrite.getEditBuffer(ID).write(OS); > + return Result; > + } > + > + std::string getFileContentFromDisk(StringRef Name) { > + llvm::SmallString<1024> Path(TemporaryDirectory.str()); > + llvm::sys::path::append(Path, Name); > + // We need to read directly from the FileManager without relaying through > + // a FileEntry, as otherwise we'd read through an already opened file > + // descriptor, which might not see the changes made. > + // FIXME: Figure out whether there is a way to get the SourceManger to > + // reopen the file. > + return Files.getBufferForFile(Path, NULL)->getBuffer(); > + } > + > + DiagnosticsEngine Diagnostics; > + TextDiagnosticPrinter DiagnosticPrinter; > + FileManager Files; > + SourceManager Sources; > + LangOptions Options; > + Rewriter Rewrite; > + > + // Will be set once on disk files are generated. > + llvm::sys::Path TemporaryDirectory; > +}; > + > +} // end namespace clang > + > +#endif > Index: unittests/CMakeLists.txt > =================================================================== > --- unittests/CMakeLists.txt (revision 156059) > +++ unittests/CMakeLists.txt (working copy) > @@ -70,5 +70,6 @@ > Tooling/CompilationDatabaseTest.cpp > Tooling/ToolingTest.cpp > Tooling/RecursiveASTVisitorTest.cpp > - USED_LIBS gtest gtest_main clangAST clangTooling > + Tooling/RewriterTest.cpp > + USED_LIBS gtest gtest_main clangAST clangTooling clangRewrite > )
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
