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

[Igor Kushnir via Phabricator via cfe-commits]

vedgy added a comment.

In D139774#4091631 <https://reviews.llvm.org/D139774#4091631>, @aaron.ballman wrote:

> My preference is still for specific API names. If we want to do something general to all temp files, I think `FileSystemOptions` is the way to go.
>
> My concern with not using a constructor is that, because this is the C API, we will be supporting the functionality for a long time even if we do switch to using a global override in `FileSystemOptions` where you would need to set the information up before executing the compiler. To me, these paths are something that should not be possible to change once the compiler has started executing. (That also solves the multithreading problem where multiple threads are fighting over what the temp directory should be and stepping on each other's toes.)

As I understand, this leaves two choices:

1. Specific preamble API names, two separate non-constructor setters; the option values are stored in a separate struct (or even as two separate data members), not inside `FileSystemOptions`.
2. General temporary-storage arguments (possibly combined in a struct) to a new overloaded constructor function; the option values are stored inside the `FileSystemOptions` struct.

The second alternative is likely more difficult to implement, more risky and less convenient to use (the store-in-memory bool option cannot be modified at any time). Perhaps it should be delayed until (and if) we learn of other temporary files libclang creates? A downside of implementing the first option now is that the specific API would have to be supported for a long time, even after the general temporary-file API is implemented.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774