[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy added a comment. Just created the follow-up `StorePreamblesInMemory` revision: D145974 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143418/new/ https://reviews.llvm.org/D143418 __

[PATCH] D143418: [libclang] Add API to override preamble storage path

aaron.ballman added inline comments. Comment at: clang/include/clang-c/Index.h:365 + */ + int ExcludeDeclarationsFromPCH : 1; + /** vedgy wrote: > aaron.ballman wrote: > > vedgy wrote: > > > vedgy wrote: > > > > aaron.ballman wrote: > > > > > vedgy wrote: > >

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy added inline comments. Comment at: clang/include/clang-c/Index.h:365 + */ + int ExcludeDeclarationsFromPCH : 1; + /** aaron.ballman wrote: > vedgy wrote: > > vedgy wrote: > > > aaron.ballman wrote: > > > > vedgy wrote: > > > > > Assigning `true` to `int

[PATCH] D143418: [libclang] Add API to override preamble storage path

aaron.ballman added inline comments. Comment at: clang/include/clang-c/Index.h:365 + */ + int ExcludeDeclarationsFromPCH : 1; + /** vedgy wrote: > vedgy wrote: > > aaron.ballman wrote: > > > vedgy wrote: > > > > Assigning `true` to `int : 1` bit-fields in C++

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy marked an inline comment as not done. vedgy added inline comments. Comment at: clang/include/clang-c/Index.h:365 + */ + int ExcludeDeclarationsFromPCH : 1; + /** vedgy wrote: > aaron.ballman wrote: > > vedgy wrote: > > > Assigning `true` to `int : 1` bi

[PATCH] D143418: [libclang] Add API to override preamble storage path

aaron.ballman added a comment. In D143418#4175887 , @vedgy wrote: > In D143418#4175628 , @aaron.ballman > wrote: > >> Hmmm, don't relaxed loads and stores still have the potential to be racey? I >> thought you n

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy marked an inline comment as not done. vedgy added inline comments. Comment at: clang/include/clang-c/Index.h:365 + */ + int ExcludeDeclarationsFromPCH : 1; + /** vedgy wrote: > Assigning `true` to `int : 1` bit-fields in C++ code produces a GCC warning:

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy added inline comments. Comment at: clang/include/clang-c/Index.h:365 + */ + int ExcludeDeclarationsFromPCH : 1; + /** Assigning `true` to `int : 1` bit-fields in C++ code produces a GCC warning: ``` warning: overflow in conversion from ‘int’ to ‘signed

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy added a comment. In D143418#4175628 , @aaron.ballman wrote: > 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 t

[PATCH] D143418: [libclang] Add API to override preamble storage path

aaron.ballman added a comment. In 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 r

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy added a comment. 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

[PATCH] D143418: [libclang] Add API to override preamble storage path

aaron.ballman added a comment. In D143418#4175188 , @vedgy wrote: > Thank you for the quick build fix. You're welcome! > KDevelop's CMakeLists.txt disables this warning by adding the > `-Wno-missing-field-initializers` flag. That's why I haven't notice

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy added a comment. Thank you for the quick build fix. KDevelop's CMakeLists.txt disables this warning by adding the `-Wno-missing-field-initializers` flag. That's why I haven't noticed the warning while building KDevelop. Either I missed the warning while building LLVM or it is also disable

[PATCH] D143418: [libclang] Add API to override preamble storage path

aaron.ballman added a comment. In D143418#4175128 , @vedgy wrote: > A clang-ppc64le-rhel build > failed: > > 54.897 [28/169/148] Building CXX object > tools/clang/unittests/libclang/CMakeFiles

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy added a comment. A clang-ppc64le-rhel build failed: 54.897 [28/169/148] Building CXX object tools/clang/unittests/libclang/CMakeFiles/libclangTests.dir/LibclangTest.cpp.o FAILED: tools/clang/unittests/libclang/CMakeFiles/libc

[PATCH] D143418: [libclang] Add API to override preamble storage path

aaron.ballman added a comment. In D143418#4174521 , @vedgy wrote: > In D143418#4172587 , @aaron.ballman > wrote: > >> Thank you, this LGTM! I have to head out shortly, so I'll land this on your >> behalf tomorro

[PATCH] D143418: [libclang] Add API to override preamble storage path

This revision was automatically updated to reflect the committed changes. Closed by commit rGcc929590ad30: [libclang] Add API to override preamble storage path (authored by vedgy, committed by aaron.ballman). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy added a comment. In D143418#4172587 , @aaron.ballman wrote: > Thank you, this LGTM! I have to head out shortly, so I'll land this on your > behalf tomorrow when I have the time to babysit the postcommit build farm. > However, if you'd like to req

[PATCH] D143418: [libclang] Add API to override preamble storage path

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Thank you, this LGTM! I have to head out shortly, so I'll land this on your behalf tomorrow when I have the time to babysit the postcommit build farm. However, if you'd like to r

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy updated this revision to Diff 502632. vedgy edited the summary of this revision. vedgy added a comment. Clean rebase on main branch where the base revision D143415 has already landed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D143418: [libclang] Add API to override preamble storage path

aaron.ballman added a comment. The changes look correct to me, but precommit CI isn't able to apply the patch cleanly to test. Can you rebase the patch so it applies so we can smoke test it before acceptance? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy updated this revision to Diff 502376. vedgy added a comment. Replace `clang_getDefaultGlobalOptions()` with `CXChoice` as discussed. A few unrelated small improvements. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143418/new/ https://reviews

[PATCH] D143418: [libclang] Add API to override preamble storage path

aaron.ballman added inline comments. Comment at: clang/include/clang-c/Index.h:329 + * CXIndexOptions Opts = { sizeof(CXIndexOptions), + * clang_getDefaultGlobalOptions() }; + * \endcode vedgy wrote: > aaron.ballman wrote: > > vedgy wrote:

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy added inline comments. Comment at: clang/include/clang-c/Index.h:329 + * CXIndexOptions Opts = { sizeof(CXIndexOptions), + * clang_getDefaultGlobalOptions() }; + * \endcode aaron.ballman wrote: > vedgy wrote: > > vedgy wrote: > > > a

[PATCH] D143418: [libclang] Add API to override preamble storage path

aaron.ballman added inline comments. Comment at: clang/include/clang-c/Index.h:329 + * CXIndexOptions Opts = { sizeof(CXIndexOptions), + * clang_getDefaultGlobalOptions() }; + * \endcode vedgy wrote: > vedgy wrote: > > aaron.ballman wrote:

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy marked an inline comment as not done. vedgy added inline comments. Comment at: clang/include/clang-c/Index.h:329 + * CXIndexOptions Opts = { sizeof(CXIndexOptions), + * clang_getDefaultGlobalOptions() }; + * \endcode vedgy wrote: > a

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy added inline comments. Comment at: clang/include/clang-c/Index.h:329 + * CXIndexOptions Opts = { sizeof(CXIndexOptions), + * clang_getDefaultGlobalOptions() }; + * \endcode aaron.ballman wrote: > vedgy wrote: > > When I almost finish

[PATCH] D143418: [libclang] Add API to override preamble storage path

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

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy marked 2 inline comments as done. vedgy added inline comments. Comment at: clang/include/clang-c/Index.h:329 + * CXIndexOptions Opts = { sizeof(CXIndexOptions), + * clang_getDefaultGlobalOptions() }; + * \endcode When I almost finish

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy updated this revision to Diff 501559. vedgy added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143418/new/ https://reviews.llvm.org/D143418 Files: clang-tools-extra/clangd/Preamble.cpp clang/docs/Relea

[PATCH] D143418: [libclang] Add API to override preamble storage path

aaron.ballman added inline comments. Comment at: clang/tools/c-index-test/c-index-test.c:79 +Opts.PreambleStoragePath = NULL; +Opts.InvocationEmissionPath = getenv("CINDEXTEST_INVOCATION_EMISSION_PATH"); + vedgy wrote: > aaron.ballman wrote: > > vedgy wr

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy added inline comments. Comment at: clang/tools/c-index-test/c-index-test.c:79 +Opts.PreambleStoragePath = NULL; +Opts.InvocationEmissionPath = getenv("CINDEXTEST_INVOCATION_EMISSION_PATH"); + aaron.ballman wrote: > vedgy wrote: > > aaron.ballman wr

[PATCH] D143418: [libclang] Add API to override preamble storage path

aaron.ballman added inline comments. Comment at: clang/tools/c-index-test/c-index-test.c:79 +Opts.PreambleStoragePath = NULL; +Opts.InvocationEmissionPath = getenv("CINDEXTEST_INVOCATION_EMISSION_PATH"); + vedgy wrote: > aaron.ballman wrote: > > vedgy wr

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy added inline comments. Comment at: clang/tools/c-index-test/c-index-test.c:79 +Opts.PreambleStoragePath = NULL; +Opts.InvocationEmissionPath = getenv("CINDEXTEST_INVOCATION_EMISSION_PATH"); + aaron.ballman wrote: > vedgy wrote: > > When a libclang

[PATCH] D143418: [libclang] Add API to override preamble storage path

aaron.ballman added inline comments. Comment at: clang/include/clang-c/Index.h:319 + */ + size_t Size; + /** vedgy wrote: > The type is `size_t` instead of the agreed upon `unsigned`, because the > addition of `unsigned GlobalOptions` below means that `unsig

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy added inline comments. Comment at: clang/include/clang-c/Index.h:319 + */ + size_t Size; + /** The type is `size_t` instead of the agreed upon `unsigned`, because the addition of `unsigned GlobalOptions` below means that `unsigned Size` no longer redu

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy added inline comments. Comment at: clang/tools/c-index-test/c-index-test.c:79 +Opts.PreambleStoragePath = NULL; +Opts.InvocationEmissionPath = getenv("CINDEXTEST_INVOCATION_EMISSION_PATH"); + When a libclang user needs to override a single option i

[PATCH] D143418: [libclang] Add API to override preamble storage path

vedgy updated this revision to Diff 500552. vedgy retitled this revision from "[libclang] Add API to set preferred temp dir path" to "[libclang] Add API to override preamble storage path". vedgy edited the summary of this revision. vedgy added a comment. Reimplement the API as discussed in review