The patch itself looks good (I don't know why we didn't do that in the first place).
On Fri, Nov 8, 2013 at 12:07 AM, Alp Toker <[email protected]> wrote: > Author: alp > Date: Fri Nov 8 02:07:19 2013 > New Revision: 194250 > > URL: http://llvm.org/viewvc/llvm-project?rev=194250&view=rev > Log: > clang-format: Write files atomically > > Switch clang-format over to Rewriter::overwriteChangedFiles(). > > The previous implementation was attempting to stream back directly to the > original file and failing if it was already memory mapped by MemoryBuffer, > an operation unsupported by Windows. > > MemoryBuffer generally mmaps files larger than the physical page size so > this will have been difficult to reproduce consistently. > > This change also reduces flicker in code editors and IDEs on all platforms > when reformatting in-place. > I think it is generally a bad idea for IDEs to use in-place reformatting, but ok .. Note that other incorrect uses of MemoryBuffer exist in LLVM/clang and > will need a similar fix. > > A test should be added for Windows when libFormat performance issues are > fixed (it takes longer than a day to format a 1MB file at present!) > Is it the write that takes so long (i.e. fixed with this patch)? If not, can the file be shared (or changed enough so that it can be shared)? Modified: > cfe/trunk/tools/clang-format/ClangFormat.cpp > > Modified: cfe/trunk/tools/clang-format/ClangFormat.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/ClangFormat.cpp?rev=194250&r1=194249&r2=194250&view=diff > > ============================================================================== > --- cfe/trunk/tools/clang-format/ClangFormat.cpp (original) > +++ cfe/trunk/tools/clang-format/ClangFormat.cpp Fri Nov 8 02:07:19 2013 > @@ -213,18 +213,8 @@ static bool format(std::string FileName) > Rewriter Rewrite(Sources, LangOptions()); > tooling::applyAllReplacements(Replaces, Rewrite); > if (Inplace) { > - if (Replaces.size() == 0) > - return false; // Nothing changed, don't touch the file. > - > - std::string ErrorInfo; > - llvm::raw_fd_ostream FileStream(FileName.c_str(), ErrorInfo, > - llvm::sys::fs::F_Binary); > - if (!ErrorInfo.empty()) { > - llvm::errs() << "Error while writing file: " << ErrorInfo << "\n"; > + if (Rewrite.overwriteChangedFiles()) > return true; > - } > - Rewrite.getEditBuffer(ID).write(FileStream); > - FileStream.flush(); > } else { > if (Cursor.getNumOccurrences() != 0) > outs() << "{ \"Cursor\": " << tooling::shiftedCodePosition( > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
