Michael pointed out in IRC that we have unique_file in v2 which should replace this. I'll change the code.<br><br><div class="gmail_quote">On Wed, May 30, 2012 at 1:12 AM, NAKAMURA Takumi <span dir="ltr"><<a href="mailto:geek4civic@gmail.com" target="_blank">geek4civic@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Manuel,<br>
<br>
After the discussion at last night, I have agreed that<br>
GetTemporaryDirectory() on Win32 would do bad thing, thank you.<br>
<br>
dir = GetTemporaryDirectory();<br>
dir.eraseFromDisk(erase_contents = true);<br>
dir = GetTemporaryDirectory(); /* It doesn't create anything on Win32<br>
due to caching */<br>
<br>
I suppose Manuel wants GetTemporaryDirectory() to keep semantics<br>
similar mkdtemp(3).<br>
<br>
Though it is in PathV1, could we fix?<br>
<br>
...Takumi<br>
<br>
2012/5/29 Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>>:<br>
<div class="HOEnZb"><div class="h5">> Additionally, from looking at the PathV1.h comments, it looks like the<br>
> Windows version actually violates the contract "@brief Construct a path to<br>
> an new, unique, existing temporary directory."<br>
><br>
> Cheers,<br>
> /Manuel<br>
><br>
><br>
> On Tue, May 29, 2012 at 8:50 AM, Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:<br>
>><br>
>> On Sun, May 27, 2012 at 4:01 PM, NAKAMURA Takumi <<a href="mailto:geek4civic@gmail.com">geek4civic@gmail.com</a>><br>
>> wrote:<br>
>>><br>
>>> 2012/5/23 Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>>:<br>
>>> > Author: klimek<br>
>>> > Date: Tue May 22 12:01:35 2012<br>
>>> > New Revision: 157260<br>
>>> ><br>
>>> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=157260&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=157260&view=rev</a><br>
>>> > Log:<br>
>>> > Adds a method overwriteChangedFiles to the Rewriter. This is<br>
>>> > implemented by<br>
>>> > first writing the changed files to a temporary location and then<br>
>>> > overwriting<br>
>>> > the original files atomically.<br>
>>> ><br>
>>> > Also adds a RewriterTestContext to aid unit testing rewrting logic in<br>
>>> > general.<br>
>>> ><br>
>>> ><br>
>>> > Added:<br>
>>> > cfe/trunk/unittests/Tooling/RewriterTest.cpp (with props)<br>
>>> > cfe/trunk/unittests/Tooling/RewriterTestContext.h (with props)<br>
>>> > Modified:<br>
>>> > cfe/trunk/include/clang/Rewrite/Rewriter.h<br>
>>> > cfe/trunk/lib/Rewrite/Rewriter.cpp<br>
>>> > cfe/trunk/unittests/CMakeLists.txt<br>
>>><br>
>>><br>
>>> > Added: cfe/trunk/unittests/Tooling/RewriterTestContext.h<br>
>>> > URL:<br>
>>> > <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RewriterTestContext.h?rev=157260&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RewriterTestContext.h?rev=157260&view=auto</a><br>
>>> ><br>
>>> > ==============================================================================<br>
>>> > --- cfe/trunk/unittests/Tooling/RewriterTestContext.h (added)<br>
>>> > +++ cfe/trunk/unittests/Tooling/RewriterTestContext.h Tue May 22<br>
>>> > 12:01:35 2012<br>
>>> > @@ -0,0 +1,120 @@<br>
>>> > +//===--- RewriterTestContext.h ----------------------------------*-<br>
>>> > C++ -*-===//<br>
>>> > +//<br>
>>> > +// The LLVM Compiler Infrastructure<br>
>>> > +//<br>
>>> > +// This file is distributed under the University of Illinois Open<br>
>>> > Source<br>
>>> > +// License. See LICENSE.TXT for details.<br>
>>> > +//<br>
>>> ><br>
>>> > +//===----------------------------------------------------------------------===//<br>
>>> > +//<br>
>>> > +// This file defines a utility class for Rewriter related tests.<br>
>>> > +//<br>
>>> ><br>
>>> > +//===----------------------------------------------------------------------===//<br>
>>> > +<br>
>>> > +#ifndef LLVM_CLANG_REWRITER_TEST_CONTEXT_H<br>
>>> > +#define LLVM_CLANG_REWRITER_TEST_CONTEXT_H<br>
>>> > +<br>
>>> > +#include "clang/Basic/Diagnostic.h"<br>
>>> > +#include "clang/Basic/FileManager.h"<br>
>>> > +#include "clang/Basic/LangOptions.h"<br>
>>> > +#include "clang/Basic/SourceManager.h"<br>
>>> > +#include "clang/Frontend/DiagnosticOptions.h"<br>
>>> > +#include "clang/Frontend/TextDiagnosticPrinter.h"<br>
>>> > +#include "clang/Rewrite/Rewriter.h"<br>
>>> > +#include "llvm/Support/Path.h"<br>
>>> > +#include "llvm/Support/raw_ostream.h"<br>
>>> > +<br>
>>> > +namespace clang {<br>
>>> > +<br>
>>> > +/// \brief A class that sets up a ready to use Rewriter.<br>
>>> > +///<br>
>>> > +/// Useful in unit tests that need a Rewriter. Creates all<br>
>>> > dependencies<br>
>>> > +/// of a Rewriter with default values for testing and provides<br>
>>> > convenience<br>
>>> > +/// methods, which help with writing tests that change files.<br>
>>> > +class RewriterTestContext {<br>
>>> > + public:<br>
>>> > + RewriterTestContext()<br>
>>> > + : Diagnostics(llvm::IntrusiveRefCntPtr<DiagnosticIDs>()),<br>
>>> > + DiagnosticPrinter(llvm::outs(), DiagnosticOptions()),<br>
>>> > + Files((FileSystemOptions())),<br>
>>> > + Sources(Diagnostics, Files),<br>
>>> > + Rewrite(Sources, Options) {<br>
>>> > + Diagnostics.setClient(&DiagnosticPrinter, false);<br>
>>> > + }<br>
>>> > +<br>
>>> > + ~RewriterTestContext() {<br>
>>> > + if (TemporaryDirectory.isValid()) {<br>
>>> > + std::string ErrorInfo;<br>
>>> > + TemporaryDirectory.eraseFromDisk(true, &ErrorInfo);<br>
>>> > + assert(ErrorInfo.empty());<br>
>>> > + }<br>
>>> > + }<br>
>>><br>
>>> Don't try to remove the TemporaryDirectory given by PathV1. Fixed in<br>
>>> r157530.<br>
>>> See also r157529.<br>
>><br>
>><br>
>> The Windows and Unix implementation of GetTemporaryDirectory seem to<br>
>> disagree on the contract here.<br>
>> From what I can see the Unix implementation neither reuses the path, nor<br>
>> deletes it on exit (both need to be done).<br>
>><br>
>> Why does the Windows implementation re-use the temporary directory?<br>
>><br>
>> Cheers,<br>
>> /Manuel<br>
>><br>
><br>
</div></div></blockquote></div><br>