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