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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

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

> In D143418#4147578 <https://reviews.llvm.org/D143418#4147578>, @aaron.ballman wrote:
>
>> In D143418#4131156 <https://reviews.llvm.org/D143418#4131156>, @vedgy wrote:
>>
>>> On second thought, the proposed `clang_getDefaultGlobalOptions()` API already offers users a choice to either prefer or override each option value from the env. variables, just like the existing `clang_CXIndex_[gs]etGlobalOptions` API. The environment variables are binary options: an existence, not value, of an env. variable determines an initial option value. So I don't understand what are the two different ways to expose these options.
>>
>> I was thinking of the env variable as determining the initial option value, so the two options I saw were "the the env variable provides the final resulting value used by the program" and "the env variable provides the default value used by the program". But you're right, the current behavior is that the presence of the env variable determines the behavior, not the value of the env variable.
>>
>> The question really boils down to: if the user passes in an option which says "don't use thread background priority" to the call to create index, AND there is an environment variable saying "use thread background priority", who should win? And does the answer differ if the option says "use thread background priority" and the environment variable does not exist?
>
> The purpose of adding the `GlobalOptions` member to `CXIndexOptions` and deprecating `clang_CXIndex_setGlobalOptions` is to improve libclang's thread safety. The proposed approach keeps the meaning of the environment variables and enables straightforward porting of existing uses. A good reason to add the `GlobalOptions` member now is to reduce the number of supported `struct CXIndexOptions` versions.
>
> If someone decides to prioritize API users vs environment variables differently, then new 3-state (unset, disabled, enabled) environment variables could be introduced and (possibly) the existing ones deprecated. Such a change may require deprecating the proposed `clang_getDefaultGlobalOptions()` API addition and introducing a more informative replacement.
>
> The question is whether such environment variable behavior changes are likely to be proposed any time soon and how important is this possibility compared to the potential thread safety improvement. I suppose if no one has proposed such environment variable changes yet, then the current behavior works well enough for all libclang users. Just searched for `LIBCLANG_BGPRIO_INDEX`, `LIBCLANG_BGPRIO_EDIT`, `CXGlobalOpt_ThreadBackgroundPriorityForIndexing`, `CXGlobalOpt_ThreadBackgroundPriorityForEditing`, `clang_CXIndex_setGlobalOptions` and `clang_CXIndex_getGlobalOptions` on the LLVM Project's issue tracker. Found no results for any of these strings.

I'm not seeing a whole lot of usage here either: https://sourcegraph.com/search?q=context:global+lang:c+-file:.*libclang.*+-file:Index.h+LIBCLANG_BGPRIO_INDEX%7CLIBCLANG_BGPRIO_EDIT%7CCXGlobalOpt_ThreadBackgroundPriorityForIndexing%7CCXGlobalOpt_ThreadBackgroundPriorityForEditing%7Cclang_CXIndex_setGlobalOptions%7Cclang_CXIndex_getGlobalOptions&patternType=regexp&sm=1&groupBy=repo

So my intuition is that the current behavior works well enough and I doubt anyone's going to propose changes to it in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418