klimek 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:
> > > > 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.


https://reviews.llvm.org/D39842



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to