<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    On 02/11/2013 20:48, Johan Engelen wrote:<br>
    <blockquote cite="mid:52756514.6090806@swissonline.ch" type="cite">Hi
      Alp,
      <br>
        The patch solves the problem, thanks!
      <br>
    </blockquote>
    <br>
    Glad to hear it!<br>
    <br>
    <blockquote cite="mid:52756514.6090806@swissonline.ch" type="cite">
      <br>
      One comment: the format function returns 'false' when no errors
      occurred. So I believe it should read
      <br>
            if (Rewrite.overwriteChangedFiles()) {
      <br>
              return false;
      <br>
            }
      <br>
    </blockquote>
    <br>
    Disagree. My patch is fine :-)<br>
    <br>
    It's just that the return value of overwriteChangedFiles() was
    mis-documented until my fix last week (r193594) so that might have
    caused confusion.<br>
    <br>
    But I'll appreciate if you can confirm:<br>
    <code><br>
    </code><code>
        /// overwriteChangedFiles - Save all changed files to disk.</code><code><br>
    </code><code>
        ///</code><code><br>
    </code><code>
        /// Returns true if any files were not saved successfully.</code><code><br>
    </code><code>
        /// Outputs diagnostics via the source manager's diagnostic
      engine</code><code><br>
    </code><code>
        /// in case of an error.</code><code><br>
    </code><code>
        bool overwriteChangedFiles();</code><code><br>
    </code><code>
    </code><code><br>
    </code><code>// Returns true on error.</code><code><br>
    </code><code>
      static bool format(std::string FileName) {</code><code><br>
    </code><code>
      ...</code><code><br>
    </code><code>
            if (Rewrite.overwriteChangedFiles())</code><code><br>
    </code><code>
              return true;</code><code><br>
    </code><code>
      ...</code><code><br>
    </code><code>
      }</code><br>
    <br>
    To land this fix we'll need a test case. I don't want to check in a
    16k+ source file into SVN so we'll have to either generate a test
    input or use one of the existing large file inputs for that.<br>
    <br>
    <br>
    Alp.<br>
    <br>
    <br>
    <br>
    <blockquote cite="mid:52756514.6090806@swissonline.ch" type="cite">
      <br>
      Cheers,
      <br>
        Johan
      <br>
      <br>
      <br>
      On 2-11-2013 21:05, Alp Toker wrote:
      <br>
      <blockquote type="cite">Hello Johan,
        <br>
        <br>
        I've attached a patch I'm using locally to reduce flicker in my
        IDE
        <br>
        while running clang-format using atomic file save.
        <br>
        <br>
        I suspect it resolves you issue too, could you try it and report
        back?
        <br>
        <br>
        Alp.
        <br>
        <br>
        <br>
        On 01/11/2013 21:12, Johan Engelen wrote:
        <br>
        <blockquote type="cite">Hello all,
          <br>
             I've been looking into why clang-format cannot write
          in-place for
          <br>
          large files (> 16 kB) on Windows [1].
          <br>
          I've found the culprit, but need help actually fixing it; the
          file
          <br>
          handling code is somewhat complex for a new-comer like me. :)
          <br>
          <br>
          The difference between small and large files is the returned
          value by
          <br>
          shouldUseMmap in /llvm/lib/Support/MemoryBuffer.cpp. For small
          files,
          <br>
          shouldUseMmap returns false and the already created normal
          <br>
          MemoryBuffer is kept and all is fine. But for larger files,
          <br>
          shouldUseMmap returns true and the already created normal
          MemoryBuffer
          <br>
          is destroyed and a new memory mapped one is created. This
          becomes the
          <br>
          OwningPtr<MemoryBuffer> Code memorybuffer in
          ClangFormat.cpp.
          <br>
          Now when the time comes to write the reformatted text in-place
          to the
          <br>
          input file, llvm::raw_fd_ostream FileStream cannot be opened
          if the
          <br>
          MemoryBuffer is a memory mapped one. Apparently Windows does
          not allow
          <br>
          that. For small files, no Mmap buffer was created and all is
          fine.
          <br>
          <br>
          I do not know nearly enough about the file handling internals
          to
          <br>
          propose a good fix.
          <br>
          If shouldUseMmap is modified to always return false,
          clang-format
          <br>
          works fine on large files. But that would be a very course
          "fix". I
          <br>
          wanted to try releasing the Mmap interface to the file, right
          before
          <br>
          writing back to it. But I was not able to figure out how to do
          that.
          <br>
          <br>
          Thanks for the help,
          <br>
               Johan
          <br>
          <br>
          [1]  <a class="moz-txt-link-freetext" href="http://llvm.org/bugs/show_bug.cgi?id=17216">http://llvm.org/bugs/show_bug.cgi?id=17216</a>
          <br>
          _______________________________________________
          <br>
          cfe-dev mailing list
          <br>
          <a class="moz-txt-link-abbreviated" href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a>
          <br>
          <a class="moz-txt-link-freetext" href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a>
          <br>
        </blockquote>
      </blockquote>
      <br>
    </blockquote>
    <br>
    <pre class="moz-signature" cols="72">-- 
<a class="moz-txt-link-freetext" href="http://www.nuanti.com">http://www.nuanti.com</a>
the browser experts
</pre>
  </body>
</html>