klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
LG
================
Comment at: lib/Frontend/PrecompiledPreamble.cpp:490
PreprocessorOpts.DisablePCHValidation = true;
+ if (Storage.getKind() == PCHStorage::Kind::TempFile) {
+ const TempPCHFile &PCHFile = Storage.asFile();
----------------
ilya-biryukov wrote:
> klimek wrote:
> > ilya-biryukov wrote:
> > > klimek wrote:
> > > > This looks a bit like we should push it into the PCHStorage.
> > > I've extracted a function here to make the code read simpler.
> > > However, I placed it directly into the `PrecompiledPreamble` instead of
> > > `PCHStorage` to keep `PCHStorage` responsible for just one thing:
> > > managing the `variant`-like union.
> > It being called PCHStorage makes it sound like it handles the abstraction
> > for how the preamble is stored. Given that the variant-like union is
> > basically an interface with an implementation switch, I think all switching
> > on it is also the responsibility of that class. Otherwise we'd need another
> > abstraction on top of it?
> Abstraction on top of it is `PrecompiledPreamble` itself. And `PCHStorage` is
> just an implementation detail.
> Does that sound reasonable?
That makes more sense now, thx for explaining. It still seems a bit off to me,
but I can't come up with a better solution, so LG.
================
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___");
----------------
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?
https://reviews.llvm.org/D39842
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
