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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 1 06:43:29 PST 2023


aaron.ballman added a comment.

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

> In D139774#4094619 <https://reviews.llvm.org/D139774#4094619>, @aaron.ballman wrote:
>
>> Is there a middle ground where, instead of #2 for general temporary storage, we went with #2 but with compiler-specific directories instead of system directories. e.g., don't let the caller set the temp directory, but do let the caller set the preamble directory (which defaults to the temp directory) so long as it's set before invoking the compiler? This still won't let you change options mid-run, but it also seems like it should have less risk of affecting other components while still solving the thread safety issues. I'm not certain if it's any easier to implement, but I think it does save you from modifying `FileSystemOptions`. As a separate item, we could then consider adding a new C API to let you toggle the in-memory vs on-disk functionality after exploring that it won't cause other problems because nobody considered the possibility that it's not a stable value for the compiler invocation.
>
> OK, so I'm going to implement overriding the preamble directory in `clang_createIndexWithPreambleStoragePath` (or `clang_createIndex2` or `clang_createIndexExt` or?); try to keep it simple and not modify `FileSystemOptions`; deal with the in-memory option separately later. Abandon this revision now and create another one once ready?

That sounds like a good plan to me. I wonder if we want to name it something like `clang_createIndexWithOptions` (or something generic like that), and give it a versioned structure of options along the lines of:

  struct CIndexOptions {
    uint32_t Size; // sizeof(struct CIndexOptions), used for option versioning
    const char *PreambleStoragePath; // This pointer can be freed after creating the index
  };

and define the function to return an error if `Size < sizeof(struct CIndexOptions)`. This should allow us to add additional options to the structure without having to introduce a new constructor API each time.


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