vedgy added a comment.

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.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to