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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 10 08:04:20 PST 2023


aaron.ballman added a comment.

In D139774#4039018 <https://reviews.llvm.org/D139774#4039018>, @vedgy wrote:

> In D139774#4036496 <https://reviews.llvm.org/D139774#4036496>, @aaron.ballman wrote:
>
>> Given that we already have other setters to set global state, I'm less concerned about adding another one. I hadn't realized we already introduced the dangers here. But we should document the expectation that the call be made before creating the index.
>
> There is a difference: `clang_CXIndex_setGlobalOptions()` and `clang_CXIndex_setInvocationEmissionPathOption()` take a `CXIndex` argument and thus can only be called **after** creating an index. So requiring to call `clang_overrideTemporaryDirectory()` before creating an index, plus one more time with `nullptr` argument after disposing of the index, wouldn't be particularly consistent with other setters.

Oh that is a good point! Apologies for not noticing that earlier -- that makes me wonder if a better approach here is to add a `std::string` to the `CIndexer` class to represent the temp path to use. I started investigating that idea and then I realized I have no idea what is calling `system_temp_directory()` in the first place. It doesn't seem to be anything from the C indexing API but it's possible this is coming from one of the other library components. Can you help me track down the call stack where we start creating the temporary files so I can better understand? My hope is that we can thread the information through rather than using a global setter, if possible.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139774/new/

https://reviews.llvm.org/D139774



More information about the cfe-commits mailing list