vedgy added a comment.
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.
In D143418#4130042 <https://reviews.llvm.org/D143418#4130042>, @aaron.ballman
wrote:
> In D143418#4126266 <https://reviews.llvm.org/D143418#4126266>, @vedgy wrote:
>
>>
>
> I've been trying to think of benefits for using a fixed-size integer type and
> the closest I can come is the consistency of the structure size across
> targets, but I don't think we need that consistency. I don't have a strong
> preference for `unsigned` vs `size_t`, so how about we go with your slight
> preference for `unsigned` unless someone finds a reason to use something else?
Sounds good. Here is my current WIP API version:
typedef struct CXIndexOptions {
/**
* The size of struct CIndexOptions used for option versioning.
*
* Always assign sizeof(CIndexOptions) to this member right after
* creating a CXIndexOptions object.
*/
unsigned Size;
/**
* \see clang_createIndex()
*/
int ExcludeDeclarationsFromPCH : 1;
int DisplayDiagnostics : 1;
/**
* The path to a directory, in which to store temporary PCH files. If null
or
* empty, the default system temporary directory is used. These PCH files
are
* deleted on clean exit but stay on disk if the program crashes or is
killed.
*
* Libclang does not create the directory at the specified path in the file
* system. Therefore it must exist, or storing PCH files will fail.
*/
const char *PreambleStoragePath;
/**
* Specifies a path which will contain log files for certain libclang
* invocations. A null value implies that libclang invocations are not
logged.
*/
const char *InvocationEmissionPath;
} CXIndexOptions;
/**
* Provides a shared context for creating translation units.
*
* Call this function instead of clang_createIndex() if you need to configure
* the additional options in CXIndexOptions.
*
* \returns The created index or null in case of error, such as an unsupported
* value of options->Size.
*
* For example:
* \code
* CXIndex createIndex(const char *ApplicationTemporaryPath) {
* const int ExcludeDeclarationsFromPCH = 1;
* const int DisplayDiagnostics = 1;
* #if CINDEX_VERSION_MINOR >= 64
* CIndexOptions Opts;
* Opts.Size = sizeof(CIndexOptions);
* Opts.ExcludeDeclarationsFromPCH = ExcludeDeclarationsFromPCH;
* Opts.DisplayDiagnostics = DisplayDiagnostics;
* Opts.PreambleStoragePath = ApplicationTemporaryPath;
* Opts.InvocationEmissionPath = NULL;
* CIndex Idx = clang_createIndexWithOptions(&Opts);
* if (Idx)
* return Idx;
* fprintf(stderr, "clang_createIndexWithOptions() failed. "
* "CINDEX_VERSION_MINOR = %d, sizeof(CIndexOptions) = %u",
* CINDEX_VERSION_MINOR, Opts.Size);
* #else
* (void)ApplicationTemporaryPath;
* #endif
*
* return clang_createIndex(ExcludeDeclarationsFromPCH, DisplayDiagnostics);
* }
* \endcode
*
* \sa clang_createIndex()
*/
CINDEX_LINKAGE CXIndex
clang_createIndexWithOptions(const CXIndexOptions *options);
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
