vedgy added a comment. In D139774#4040705 <https://reviews.llvm.org/D139774#4040705>, @aaron.ballman wrote:
> At a point where we have a `CIndexer` object, we eventually call > `ASTUnit::LoadFromCommandLine()` and `ASTUnit` has a member > `FileSystemOptions FileSystemOpts`, and `FileSystemOptions` has a > `std::string` for the working directory to use. I think we should store the > temp directory in here as well, and when > `ASTUnit::getMainBufferWithPrecompiledPreamble()` build the preamble, it can > pass that information along to `TempPCHFile` to avoid needing to ask the > system for the temp directory. The path would have to be passed into several functions. `TempPCHFile::create()` would have to use `createUniqueFile()` instead of `createTemporaryFile()`. My greatest concern with this improved design is that libclang may use the temporary directory for other purposes - if not yet, then in the future. Another issue is with the `FileSystemOptions` part: this class is widely used in Clang. So adding a member to it could adversely affect performance of unrelated code. > This does mean we store two copies of the string (one in `CIndexer` and one > in `FileSystemOptions`, but I think the improved ownership semantics for the > C API makes that duplication somewhat reasonable. If LLVM does not have its own shared buffer type, a `std::shared_ptr<const char[]>` could be used instead of `std::string`. Or `SmallString` (not shared, but avoids allocations given a not too long overriding path). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139774/new/ https://reviews.llvm.org/D139774 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
