[LLVMdev] [cfe-commits] r157260 - in /cfe/trunk: include/clang/Rewrite/Rewriter.h lib/Rewrite/Rewriter.cpp unittests/CMakeLists.txt unittests/Tooling/RewriterTest.cpp unittests/Tooling/RewriterTestContext.h

[Manuel Klimek]

Michael pointed out in IRC that we have unique_file in v2 which should
replace this. I'll change the code.

On Wed, May 30, 2012 at 1:12 AM, NAKAMURA Takumi <geek4civic at gmail.com>wrote:

> 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 <klimek at google.com>:
> > 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 <klimek at google.com>
> wrote:
> >>
> >> On Sun, May 27, 2012 at 4:01 PM, NAKAMURA Takumi <geek4civic at gmail.com>
> >> wrote:
> >>>
> >>> 2012/5/23 Manuel Klimek <klimek at google.com>:
> >>> > 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
> >>
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20120530/a3a636c1/attachment.html>