<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
On 2-11-2013 23:16, Alp Toker wrote:<br>
<blockquote cite="mid:527579AD.7040907@nuanti.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
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>
</blockquote>
<br>
Agreed :-)<br>
<br>
<blockquote cite="mid:527579AD.7040907@nuanti.com" type="cite"> <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>
</blockquote>
<br>
(I'm sorry, I'm not sure how to help with this. It's been only 3
days since I've first seen the codebase, so... :-)<br>
Adding a test case means adding to /llvm/tools/clang/test/Format ?<br>
I'll see if I can figure it out from the other tests in the
codebase.<br>
<br>
-Johan<br>
<br>
<br>
<blockquote cite="mid:527579AD.7040907@nuanti.com" type="cite"> 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 moz-do-not-send="true"
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 moz-do-not-send="true" class="moz-txt-link-abbreviated"
href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a>
<br>
<a moz-do-not-send="true" 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 moz-do-not-send="true" class="moz-txt-link-freetext" href="http://www.nuanti.com">http://www.nuanti.com</a>
the browser experts
</pre>
</blockquote>
<br>
</body>
</html>