ilya-biryukov added inline comments.
================ Comment at: lib/Frontend/PrecompiledPreamble.cpp:44 + SmallString<64> Path; + llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true, Path); + llvm::sys::path::append(Path, "___clang_inmemory_preamble___"); ---------------- ilya-biryukov wrote: > klimek wrote: > > ilya-biryukov wrote: > > > klimek wrote: > > > > ilya-biryukov wrote: > > > > > klimek wrote: > > > > > > erasedOnReboot seems weird for something that we don't want to have > > > > > > on the actual file system. Why do we even want a temp directory? > > > > > Yeah, `erasedOnReboot` seems weird here. > > > > > What I really wanted is a filepath that's valid on all platforms and > > > > > is on an existing drive for Windows, so the value of `erasedOnReboot` > > > > > doesn't really matter. `system_temp_directory` might be the wrong > > > > > thing to use for that in the first place. Is there a better > > > > > alternative I'm missing? > > > > Why does it need to get an existing path / existing drive? > > > Oh, maybe it doesn't have to be. But I expect that a probability of > > > something breaking is lower if we use an existing path. > > > Maybe I'm wrong and it will all work out just fine, but that seems to be > > > a rather good compromise adding an extra bit of confidence that things > > > will be working in the long-term. > > I'd on the contrary say that this is more likely to fail. If we don't have > > a real filesystem, or have a read-only view of a real file system, getting > > a temp dir might well fail, while we can easily overlay any path we want in > > memory. > That's true in general, but LLVM's `system_temp_directory` will return the > default (`/var/tmp` or `/tmp` on Linux, `C:\Temp` on Windows) if it fails to > find the real temp dir. > So in the very worst case we'll be using hard-coded paths. > > On the other hand, if that's one of the possible behaviors, we'd probably > want to not break in that case anyway. I'll update the patch to follow your > suggestions. Thanks! This should do the trick. https://reviews.llvm.org/D39842 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
