[cfe-dev] Help solve bug 17216 for clang-format on Windows

[Alp Toker]

Hello Johan,

I've attached a patch I'm using locally to reduce flicker in my IDE
while running clang-format using atomic file save.

I suspect it resolves you issue too, could you try it and report back?

Alp.


On 01/11/2013 21:12, Johan Engelen wrote:
> Hello all,
>   I've been looking into why clang-format cannot write in-place for
> large files (> 16 kB) on Windows [1].
> I've found the culprit, but need help actually fixing it; the file
> handling code is somewhat complex for a new-comer like me. :)
>
> The difference between small and large files is the returned value by
> shouldUseMmap in /llvm/lib/Support/MemoryBuffer.cpp. For small files,
> shouldUseMmap returns false and the already created normal
> MemoryBuffer is kept and all is fine. But for larger files,
> shouldUseMmap returns true and the already created normal MemoryBuffer
> is destroyed and a new memory mapped one is created. This becomes the
> OwningPtr<MemoryBuffer> Code memorybuffer in ClangFormat.cpp.
> Now when the time comes to write the reformatted text in-place to the
> input file, llvm::raw_fd_ostream FileStream cannot be opened if the
> MemoryBuffer is a memory mapped one. Apparently Windows does not allow
> that. For small files, no Mmap buffer was created and all is fine.
>
> I do not know nearly enough about the file handling internals to
> propose a good fix.
> If shouldUseMmap is modified to always return false, clang-format
> works fine on large files. But that would be a very course "fix". I
> wanted to try releasing the Mmap interface to the file, right before
> writing back to it. But I was not able to figure out how to do that.
>
> Thanks for the help,
>     Johan
>
> [1]  http://llvm.org/bugs/show_bug.cgi?id=17216
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

-- 
http://www.nuanti.com
the browser experts

-------------- next part --------------
commit 70d3c1e925b82a24e0643e8eae2b542112c6b54e
Author: Alp Toker <alp at nuanti.com>
Date:   Sat Nov 2 04:23:44 2013 +0000

    clang-format: Write files atomically
    
    Avoids editor flicker and fixes Win bug?
    
    Use overwriteChangedFiles()

diff --git a/tools/clang-format/ClangFormat.cpp b/tools/clang-format/ClangFormat.cpp
index 2ca2bed..85c47ab 100644
--- a/tools/clang-format/ClangFormat.cpp
+++ b/tools/clang-format/ClangFormat.cpp
@@ -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(