aaron.ballman added inline comments.
================
Comment at: clang/include/clang-c/Index.h:329
+ * CXIndexOptions Opts = { sizeof(CXIndexOptions),
+ * clang_getDefaultGlobalOptions() };
+ * \endcode
----------------
vedgy wrote:
> When I almost finished the requested changes, I remembered that the return
> value of `clang_getDefaultGlobalOptions()` depends on environment variables,
> and thus `0` is not necessarily the default. Adjusted the changes and updated
> this revision.
>
> Does the extra requirement to non-zero initialize this second member sway
> your opinion on the usefulness of the helper function `inline CXIndexOptions
> clang_getDefaultIndexOptions()`? Note that there may be same (environment) or
> other important reasons why future new options couldn't be zeroes by default.
Thinking out loud a bit... (potentially bad idea incoming)
What if we dropped `clang_getDefaultGlobalOptions()` and instead made a change
to `CXGlobalOptFlags`:
```
typedef enum {
/**
* Used to indicate that the default CXIndex options are used. By default, no
* global options will be used. However, environment variables may change
which
* global options are in effect at runtime.
*/
CXGlobalOpt_Default = 0x0,
/**
* Used to indicate that threads that libclang creates for indexing
* purposes should use background priority.
*
* Affects #clang_indexSourceFile, #clang_indexTranslationUnit,
* #clang_parseTranslationUnit, #clang_saveTranslationUnit.
*/
CXGlobalOpt_ThreadBackgroundPriorityForIndexing = 0x1,
/**
* Used to indicate that threads that libclang creates for editing
* purposes should use background priority.
*
* Affects #clang_reparseTranslationUnit, #clang_codeCompleteAt,
* #clang_annotateTokens
*/
CXGlobalOpt_ThreadBackgroundPriorityForEditing = 0x2,
/**
* Used to indicate that all threads that libclang creates should use
* background priority.
*/
CXGlobalOpt_ThreadBackgroundPriorityForAll =
CXGlobalOpt_ThreadBackgroundPriorityForIndexing |
CXGlobalOpt_ThreadBackgroundPriorityForEditing,
/**
* Used to indicate that no global options should be used, even
* in the presence of environment variables.
*/
CXGlobalOpt_None = 0xFFFFFFFF
} CXGlobalOptFlags;
```
so that when the user passes `0` they get the previous behavior.
`clang_CXIndex_setGlobalOptions()` would remain deprecated.
`clang_CXIndex_getGlobalOptions()` would be interesting though -- would it
return `CXGlobalOpt_None` or `CXGlobalOpt_Default` in the event the index was
created without any global options? Hmmm.
Err, actually, I suppose this won't work too well because then code silently
changes behavior if it does `clang_CXIndex_setGlobalOptions(CXGlobalOpt_None);`
because that would change from "do what the environment says" to "ignore the
environment". But I have to wonder whether anyone actually *does* that or
not... my intuition is that folks would not call
`clang_CXIndex_setGlobalOptions()` at all unless they were setting an option to
a non-none value. We could rename `CXGlobalOpt_None` to `CXGlobalOpt_Nothing`
(or something along those lines) to force a compilation error, but that's a bit
of a nuclear option for what's supposed to be a stable API.
So I'm on the fence, I guess. I'd still prefer for zero to give sensible
defaults and I don't think there's enough use of the global options +
environment variables to matter. But I also don't like silently breaking code,
so my idea above may be a nonstarter.
I suppose another possible idea is: deprecate the notion of global options enum
and setter/getter entirely, add two new fields to `CXIndexOptions`:
```
typedef enum {
CXChoice_Default = 0,
CXChoice_Enabled = 1,
CXChoice_Disabled = 2
} CXChoice;
...
unsigned ThreadPriorityBackgroundForIndexing;
unsigned ThreadPriorityBackgroundForEditing;
...
```
so that `0` gives the correct default behavior based on environment variable.
There would be no global setter or getter for this information (and we'd
eventually remove `clang_CXIndex_[gs]etGlobalOptions()`).
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
