aaron.ballman added a comment. In D143418#4131156 <https://reviews.llvm.org/D143418#4131156>, @vedgy wrote:
> In D143418#4125756 <https://reviews.llvm.org/D143418#4125756>, @aaron.ballman > wrote: > >>> 3. `clang_createIndex()` initializes global options based on environment >>> variable values: >>> >>> if (getenv("LIBCLANG_BGPRIO_INDEX")) >>> CIdxr->setCXGlobalOptFlags(CIdxr->getCXGlobalOptFlags() | >>> >>> CXGlobalOpt_ThreadBackgroundPriorityForIndexing); >>> if (getenv("LIBCLANG_BGPRIO_EDIT")) >>> CIdxr->setCXGlobalOptFlags(CIdxr->getCXGlobalOptFlags() | >>> >>> CXGlobalOpt_ThreadBackgroundPriorityForEditing); >>> >>> The recommended in documentation usage of `clang_CXIndex_setGlobalOptions` >>> is: >>> >>> * \code >>> * CXIndex idx = ...; >>> * clang_CXIndex_setGlobalOptions(idx, >>> * clang_CXIndex_getGlobalOptions(idx) | >>> * CXGlobalOpt_ThreadBackgroundPriorityForIndexing); >>> * \endcode >>> >>> So making these options part of `struct CXIndexOptions` and deprecating >>> `clang_CXIndex_setGlobalOptions` requires introducing another global >>> function that would read the environment variables: >>> >>> CINDEX_LINKAGE unsigned clang_getDefaultGlobalOptions(); >>> >>> Is this the right approach? >> >> Hmm, to make this patch easier, I think we might want to leave the >> environment variable behavior alone and not shift these into the options >> structure (yet?). Naively, I think it makes sense for these to eventually >> live in the options structure, but we could expose them in a few different >> ways (an option to prefer the env variable over a manual value as it is >> today or an option to prefer the manual value over the env variable for >> folks who want more hermetic behavior). WDYT? My opinion here isn't super >> strong, so if you have a strong desire to deprecate and add a replacement >> API, I think that's a defensible course to take. > > 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? > Sounds good. Here is my current WIP API version: This looks sensible to me. 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
