[PATCH] D143418: [libclang] Add API to set preferred temp dir path

Mon Feb 13 10:26:59 PST 2023

aaron.ballman added a comment.

In D143418#4123468 <https://reviews.llvm.org/D143418#4123468>, @vedgy wrote:

> In D143418#4122821 <https://reviews.llvm.org/D143418#4122821>, @aaron.ballman wrote:
>
>>> How about including existing options, which //should// be set in constructor, in the initial struct version and deprecating the corresponding setters?
>>
>> I think that makes a lot of sense.
>
> How to deprecate the setters? Add `DEPRECATED` in the documentations as is already done in two places in //Index.h//?

I think that is reasonable. (Maybe someday in the future we should use attributes to give users better diagnostics, but not as part of this patch.)

>>>   `const char *preferredTempDirPath;`
>>
>> In terms of documenting the structure, should we document this member as only impacting preamble files currently, should we rename this to be preamble-path specific, or should we try to use this preferred path in all the places we need the temp directory?
>>
>> (I lean towards renaming to preamble-path specific -- then users don't have the problem of upgrading to a newer CIndex potentially changing the behavior of where non-preamble files are stored, and we don't have to do the work up-front to audit other temp file uses.)
>
> Looks like an imperfect general implementation is unacceptable. So I'll rename this data member to `PreambleStoragePath`.

I like that direction, thank you.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418