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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
