[PATCH] D139774: [libclang] Add API to set temporary directory location

Tue Jan 10 12:21:16 PST 2023

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