aaron.ballman added a comment. In D143418#4175381 <https://reviews.llvm.org/D143418#4175381>, @vedgy wrote:
> I am implementing the `StorePreamblesInMemory` bool option discussed earlier. > It would be nice to be able to modify it at any time, because it can be an > option in an IDE's UI and requiring to restart an IDE for the option change > to take effect is undesirable. In order to make the setter thread-safe, the > option can be stored as `std::atomic<bool> StorePreamblesInMemory` in `class > CIndexer` and stored/loaded with `memory_order_relaxed`. Setting this option > would apply only to subsequent `clang_parseTranslationUnit_Impl()` calls. Hmmm, don't relaxed loads and stores still have the potential to be racey? I thought you needed a release on the store and an acquire on the load (or sequential consistency), but this is definitely not my area of expertise. > The option can also be added to `CXIndexOptions` in order to allow setting > its initial value reliably (not worrying whether it could be used before the > setter gets called after index construction). > > Is adding both the setter and the `CXIndexOptions` member OK or would you > prefer only one of these two? It's a bit odd to me that we're deprecating the global option setters to turn around and add a new global option setter. The old interfaces are not thread safe and the new one is thread safe, so I get why the desire exists and is reasonable, but (personally) I'd like to get rid of the option state setters entirely someday. What I was envisioning was that the index creation would get global options and if we wanted per-TU options, we'd add an interface akin to `clang_parseTranslationUnit()` which takes another options struct for per-TU options. (Perhaps the default values for those options come from the global options -- e.g., `DisplayDiagnostics` is set at the global level but could be overridden for a single TU). Do you have thoughts about that approach? 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
