[PATCH] D30385: clang-format: Don't leave behind temp files in -i mode on Windows, PR26125, reloaded

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.


================
Comment at: include/clang/Rewrite/Core/Rewriter.h:188-197
+  /// prewrite(FID) is called after all changes to FID have been written to a
+  /// temporary file, but before that temporary file is moved into FID's
+  /// location.  Windows's ReplaceFileW(to, from) internally creates (another)
+  /// temporary file and swaps the data streams of that file with the one of
+  /// |to| so that it's possible to replace an opened file.  However, if |to|
+  /// _was_ opened, the temp file now has that open data stream, and
+  /// ReplaceFileW() fails to delete it, causing a tempo file leak.  To fix,
----------------
klimek wrote:
> llvm.org seems to be down, so I cannot access the original patch - but I'm somewhat opposed to making this interface more complex. If we have a coupling of MemoryBuffer to the Rewriter, can we make that explicit instead?
I think this generally requires per-app knowledge on when and how it's safe to free up memory buffers (for example, the fixit rewriter can't easily drop its inputs currently as far as I can tell without further refactorings). If you have a good suggestion, I'm all ears, but this seems like a fairly minor change that attempts to fix a fairly major bug. So if there's some better way of factoring this that I currently don't see, it's almost no work to move to that then.


https://reviews.llvm.org/D30385