[PATCH] D150910: [libclang] Add CXBinaryOperatorKind and CXUnaryOperatorKind (implements 29138)

2023-06-08 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D150910#4405536 , @MineGame159 wrote: > I kinda thought the undefined reference error is just something I broke on my > machine but guess not. Don't really know what can cause it since it can link > to other functions from

[PATCH] D146275: [libclang] Fix documentation; NFC

2023-03-17 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a reviewer: aaron.ballman. vedgy added a comment. Noticed the mistake while reviewing the generated documentation . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D146275: [libclang] Fix documentation; NFC

2023-03-17 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy created this revision. Herald added subscribers: mikhail.ramalho, arphaman. Herald added a project: All. vedgy requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes cc929590ad305f0d068709c7c7999f5fc6118dc9

[PATCH] D145974: [libclang] Add index option to store preambles in memory

2023-03-15 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. @aaron.ballman, can you land this revision for me please? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145974/new/ https://reviews.llvm.org/D145974 ___ cfe-commits mailing list

[PATCH] D146039: [libclang] No longer attempt to get a dependent bit-width

2023-03-14 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy accepted this revision. vedgy added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146039/new/ https://reviews.llvm.org/D146039 ___ cfe-commits mailing list

[PATCH] D146039: [libclang] No longer attempt to get a dependent bit-width

2023-03-14 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added inline comments. Comment at: clang/include/clang-c/Index.h:3043 + * For example: + * if (clang_Cursor_isBitField(Cursor)) { + * int Width = clang_getFieldDeclBitWidth(Cursor); Surround the example with ` \code` and `\endcode` commands.

[PATCH] D130303: Fix include order in CXType.cpp

2023-03-14 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added inline comments. Comment at: clang/tools/libclang/CXType.cpp:374 +unsigned clang_isBitFieldDecl(CXCursor C) { + using namespace cxcursor; aaron.ballman wrote: > vedgy wrote: > > vedgy wrote: > > > I just noticed the `clang_Cursor_isBitField()`

[PATCH] D130303: Fix include order in CXType.cpp

2023-03-14 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added inline comments. Comment at: clang/tools/libclang/CXType.cpp:374 +unsigned clang_isBitFieldDecl(CXCursor C) { + using namespace cxcursor; vedgy wrote: > I just noticed the `clang_Cursor_isBitField()` function implemented [[ >

[PATCH] D130303: Fix include order in CXType.cpp

2023-03-14 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added inline comments. Comment at: clang/tools/libclang/CXType.cpp:374 +unsigned clang_isBitFieldDecl(CXCursor C) { + using namespace cxcursor; I just noticed the `clang_Cursor_isBitField()` function implemented [[

[PATCH] D145974: [libclang] Add index option to store preambles in memory

2023-03-13 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. I have implemented the setter for the new option locally and tested it in KDevelop. void clang_CXIndex_setStorePreamblesInMemory(CXIndex CIdx, int storePreamblesInMemory) { if (CIdx)

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

2023-03-13 Thread Igor Kushnir via Phabricator via cfe-commits
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] D145974: [libclang] Add index option to store preambles in memory

2023-03-13 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy created this revision. Herald added a subscriber: arphaman. Herald added a project: All. vedgy requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This commit allows libclang API users to opt into storing PCH in memory instead of

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

2023-03-10 Thread Igor Kushnir via Phabricator via cfe-commits
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

[PATCH] D145783: Reserve unused bits in struct CXIndexOptions; NFC

2023-03-10 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy created this revision. Herald added a subscriber: arphaman. Herald added a project: All. vedgy requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D145783 Files:

[PATCH] D145775: Improve documentation of CXIndexOptions; NFC

2023-03-10 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy created this revision. Herald added a subscriber: arphaman. Herald added a project: All. vedgy requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Document one more alternative way to initialize struct CXIndexOptions, which is used in

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

2023-03-09 Thread Igor Kushnir via Phabricator via cfe-commits
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`

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

2023-03-09 Thread Igor Kushnir via Phabricator via cfe-commits
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

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

2023-03-09 Thread Igor Kushnir via Phabricator via cfe-commits
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] D130303: Handle template parameter-dependent bit field widths in libclang

2023-03-09 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. Please update the commit message (there is no `clang_isFieldDeclBitWidthDependent` anymore) and update the revision with `arc diff --verbatim @~`. Comment at: clang/include/clang-c/Index.h:3552 + * If the cursor does not reference a bit field

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

2023-03-07 Thread Igor Kushnir via Phabricator via cfe-commits
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

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

2023-03-07 Thread Igor Kushnir via Phabricator via cfe-commits
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

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

2023-03-07 Thread Igor Kushnir via Phabricator via cfe-commits
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

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

2023-03-07 Thread Igor Kushnir via Phabricator via cfe-commits
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:

[PATCH] D130303: Handle template parameter-dependent bit field widths in libclang

2023-03-07 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added inline comments. Comment at: clang/include/clang-c/Index.h:3552 + * If the cursor does not reference a bit field declaration or if the bit + * field's width does not depend on template parameters, 0 is returned. + */ vedgy wrote: > collinbaker wrote:

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

2023-03-07 Thread Igor Kushnir via Phabricator via cfe-commits
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

[PATCH] D130303: Handle template parameter-dependent bit field widths in libclang

2023-03-07 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added inline comments. Comment at: clang/include/clang-c/Index.h:3552 + * If the cursor does not reference a bit field declaration or if the bit + * field's width does not depend on template parameters, 0 is returned. + */ collinbaker wrote: > vedgy wrote:

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

2023-03-06 Thread Igor Kushnir via Phabricator via cfe-commits
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

[PATCH] D130303: Handle template parameter-dependent bit field widths in libclang

2023-03-06 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added inline comments. Comment at: clang/include/clang-c/Index.h:3552 + * If the cursor does not reference a bit field declaration or if the bit + * field's width does not depend on template parameters, 0 is returned. + */ I just thought how the new API

[PATCH] D130303: Handle template parameter-dependent bit field widths in libclang

2023-03-06 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added inline comments. Comment at: clang/include/clang-c/Index.h:3560 + * If a cursor that is not a bit field declaration is passed in, or if the bit + * field's width expression cannot be evaluated, -1 is returned. */ Append the following line to the

[PATCH] D130303: Handle template parameter-dependent bit field widths in libclang

2023-03-06 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added subscribers: aaron.ballman, vedgy. vedgy added a comment. I just verified that this patch fixes a KDevelop crash reported by several users. Thank you! @aaron.ballman, could you please review this fix? Comment at:

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

2023-03-04 Thread Igor Kushnir via Phabricator via cfe-commits
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/

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

2023-03-02 Thread Igor Kushnir via Phabricator via cfe-commits
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: > > >

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

2023-03-02 Thread Igor Kushnir via Phabricator via cfe-commits
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: >

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

2023-03-02 Thread Igor Kushnir via Phabricator via cfe-commits
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

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

2023-03-01 Thread Igor Kushnir via Phabricator via cfe-commits
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

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

2023-03-01 Thread Igor Kushnir via Phabricator via cfe-commits
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

[PATCH] D143415: LibclangTest: remove libclang-test-* tmp dir reliably

2023-03-01 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D143415#4161460 , @aaron.ballman wrote: > In D143415#4160550 , @vedgy wrote: > >> Can someone merge/land this review? I don't have commit access. > > I'm happy to do that for you --

[PATCH] D143415: LibclangTest: remove libclang-test-* tmp dir reliably

2023-02-28 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. Can someone merge/land this review? I don't have commit access. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143415/new/ https://reviews.llvm.org/D143415 ___ cfe-commits mailing

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

2023-02-28 Thread Igor Kushnir via Phabricator via cfe-commits
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

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

2023-02-27 Thread Igor Kushnir via Phabricator via cfe-commits
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

2023-02-26 Thread Igor Kushnir via Phabricator via cfe-commits
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

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

2023-02-26 Thread Igor Kushnir via Phabricator via cfe-commits
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

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

2023-02-26 Thread Igor Kushnir via Phabricator via cfe-commits
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

[PATCH] D143415: LibclangTest: remove libclang-test-* tmp dir reliably

2023-02-26 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy updated this revision to Diff 500551. vedgy added a comment. Rebase on latest main branch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143415/new/ https://reviews.llvm.org/D143415 Files: clang/unittests/libclang/LibclangTest.cpp

[PATCH] D143415: LibclangTest: remove libclang-test-* tmp dir reliably

2023-02-25 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. @aaron.ballman, this test fix is independent from D143418 and can be reviewed separately. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143415/new/ https://reviews.llvm.org/D143415

[PATCH] D143418: [libclang] Add API to set preferred temp dir path

2023-02-24 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D143418#4150360 , @aaron.ballman wrote: > So my intuition is that the current behavior works well enough and I doubt > anyone's going to propose changes to it in the future. So adding the `GlobalOptions` member to

[PATCH] D143418: [libclang] Add API to set preferred temp dir path

2023-02-23 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D143418#4147578 , @aaron.ballman wrote: > In D143418#4131156 , @vedgy wrote: > >> On second thought, the proposed `clang_getDefaultGlobalOptions()` API >> already offers users a choice

[PATCH] D143418: [libclang] Add API to set preferred temp dir path

2023-02-15 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D143418#4125756 , @aaron.ballman wrote: >> 3. `clang_createIndex()` initializes global options based on environment >> variable values: >> >> if (getenv("LIBCLANG_BGPRIO_INDEX")) >>

[PATCH] D143418: [libclang] Add API to set preferred temp dir path

2023-02-14 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D143418#4125756 , @aaron.ballman wrote: > In D143418#4125098 , @vedgy wrote: > >>> `uint32_t Size; // = sizeof(struct CIndexOptions), used for option >>> versioning` >> >> 1.

[PATCH] D143418: [libclang] Add API to set preferred temp dir path

2023-02-13 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. > `uint32_t Size; // = sizeof(struct CIndexOptions), used for option versioning` `uint32_t` was introduced in C99. Can/should it be used in //Index.h//? Only built-in `[unsigned] (int|long)` types are currently used in this file. Repository: rG LLVM Github Monorepo

[PATCH] D143418: [libclang] Add API to set preferred temp dir path

2023-02-13 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D143418#4122821 , @aaron.ballman wrote: >> How about including existing options, which //should// be set in >> constructor, in the initial struct version and deprecating the corresponding >> setters? > > I think that makes a

[PATCH] D143418: [libclang] Add API to set preferred temp dir path

2023-02-10 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D143418#4118905 , @aaron.ballman wrote: > I don't think "tries hard" is a good enough guarantee for where files are > placed. I'm not comfortable with the security posture of that approach as it > could potentially leak

[PATCH] D143755: [clang-format] Add a space between an overloaded operator and '>'

2023-02-10 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. Hi @owenpan. Thank you for fixing this bug! Have you noticed this paragraph in my bug report? > I believe `clang_getTypeSpelling()`, or more likely `QualType::print()` used > by it, should insert a tab character between such tokens to pretty-print > compilable code. The

[PATCH] D143418: [libclang] Add API to set preferred temp dir path

2023-02-09 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a subscriber: dblaikie. vedgy added a comment. In D143418#4116290 , @aaron.ballman wrote: >> So how about a more sincere general solution: setPreferredTempDirPath()? The >> documentation would say that libclang tries its best to place

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

2023-02-09 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D140756#4115381 , @aaron.ballman wrote: > The spare moment came sooner than expected, I've made the changes in > de4321cf2cf28f2bae7fc0f1897957e73b726f89 >

[PATCH] D143418: [libclang] Add API to set preferred temp dir path

2023-02-07 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. I hoped to avoid applying an unrelated `git clang-format`'s include reordering, but the CI fails because of that. Will apply it along with changes requested during code review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D143418: [libclang] Add API to set preferred temp dir path

2023-02-06 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy updated this revision to Diff 495372. vedgy edited the summary of this revision. vedgy added a comment. Address an old inline review comment https://reviews.llvm.org/D139774#inline-1360634 and add a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

2023-02-06 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. The added, then reverted release note was lost when this revision landed the second time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140756/new/ https://reviews.llvm.org/D140756

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-02-06 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy abandoned this revision. vedgy added a comment. D143418 implements my latest idea described in the previous comment. Let us continue the discussion there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D143418: [libclang] Add API to set preferred temp dir path

2023-02-06 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy created this revision. Herald added subscribers: kadircet, arphaman. Herald added a project: All. vedgy requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added projects: clang, clang-tools-extra. TempPCHFile::create() calls

[PATCH] D143415: LibclangTest: remove libclang-test-* tmp dir reliably

2023-02-06 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy created this revision. Herald added a project: All. vedgy requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Temporary directories created by two LibclangReparseTest tests - ReparseWithModule and clang_parseTranslationUnit2FullArgv -

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

2023-02-02 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added inline comments. Comment at: clang/tools/libclang/libclang.map:419 clang_getSymbolGraphForUSR; +clang_CXXMethod_isExplicit; }; This line should be moved from under the `LLVM_16` tag to under a new `LLVM_17` tag. Alternatively this commit

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

2023-02-02 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. I meant that the commit message of https://reviews.llvm.org/rG79571aa2103c95760a07e3549d8636379e4948f0 misleadingly refers to this review's commit. But `CINDEX_VERSION_MINOR == 63` is for previous commits. This commit will require incrementing `CINDEX_VERSION_MINOR`

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

2023-02-02 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. > 79571aa2103c95760a07e3549d8636379e4948f0 > is the > commit on main and https://github.com/llvm/llvm-project/issues/60478 is for > the 16.x cherry-pick. Thank you Aaron! But note that this review's

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

2023-02-02 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. Hello Luca and Aaron. I have noticed that you recently implemented/reviewed multiple interesting libclang features, some of which can be used in KDevelop. However, `CINDEX_VERSION_MINOR` was last modified in Clang 13. This constant's documentation states: >

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-02-02 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. Another disadvantage of using `CINDEX_VERSION_MINOR` instead of the `sizeof` is that an older libclang version won't know about compatibility of newer versions with itself. But this reasoning brought me to a different thought: when a program is compiled against a

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-02-01 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. Perhaps the struct should contain the value of `CINDEX_VERSION_MINOR` instead of the `sizeof`? This way, libclang can change the meaning of a struct member without changing the size of the struct. For example, replace `PreambleStoragePath` with `TemporaryDirectoryPath`.

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-02-01 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D139774#4096695 , @aaron.ballman wrote: > There's three scenarios when a field is added to the structure: 1) library > and app are matching versions, 2) library is newer than app, 3) app is newer > than library. #1 is the

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-02-01 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D139774#4096527 , @aaron.ballman wrote: > That sounds like a good plan to me. I wonder if we want to name it something > like `clang_createIndexWithOptions` (or something generic like that), and > give it a versioned

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-31 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D139774#4094619 , @aaron.ballman wrote: > Is there a middle ground where, instead of #2 for general temporary storage, > we went with #2 but with compiler-specific directories instead of system > directories. e.g., don't let

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-30 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D139774#4091631 , @aaron.ballman wrote: > My preference is still for specific API names. If we want to do something > general to all temp files, I think `FileSystemOptions` is the way to go. > > My concern with not using a

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-25 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. 3 out of 4 alternatives remain: > 1. Add an option to store the preamble-*.pch files in RAM instead of /tmp and > add a corresponding option in KDevelop configuration UI. This would work > perfectly for me, provided I don't change my mind and decide to turn this >

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-25 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. Now that a consensus has been reached that this revision is to be discarded, can we please resume the discussion of which alternative should be implemented in the replacement revision? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-20 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. One idea discussed in comments to the KDevelop merge request, which I haven't mentioned here is this: remove the preamble files immediately after creating and opening them. This is safe on Unix-like OSes, because every file that is still open will not actually be deleted

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-19 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D139774#4066593 , @dblaikie wrote: >>> (1) seems OK-ish, I guess there's some question as to what the tradeoff is >>> for that option - does that blow out memory usage of the client/kdevelop? >>> But I guess it's probably

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-19 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D139774#4066519 , @dblaikie wrote: > (1) seems OK-ish, I guess there's some question as to what the tradeoff is > for that option - does that blow out memory usage of the client/kdevelop? But > I guess it's probably fine. At

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-19 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D139774#4065753 , @aaron.ballman wrote: > I lean towards #2b over #2a due to wanting individual overrides rather than a > blanket override (e.g., we should be able to put preamble files in a > different location than we put,

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-18 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. After a discussion under the corresponding KDevelop merge request, I can see 4-6 alternative ways to address the temporary directory issue: 1. Add an option to store the //preamble-*.pch// files in RAM instead of /tmp and add a corresponding option in KDevelop

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-12 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D139774#4047976 , @dblaikie wrote: > It seemed like the places where kdevelop would want to suppress the temp dir > cleanup would be enumerable/more within kdevelop's control than instances of > libraries kdevelop is using

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-12 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a subscriber: milianw. vedgy added a comment. In D139774#4045361 , @dblaikie wrote: > 1. Should clang be doing something better with these temp files anyway, no > matter the directory they go in? (setting them for cleanup at process exit or

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-11 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D139774#4041308 , @aaron.ballman wrote: > Is your concern essentially that someone will add a new use to put files in a > temp directory and not have access to this information from ASTUnit? Or do > you have other concerns in

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-10 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D139774#4040705 , @aaron.ballman wrote: > At a point where we have a `CIndexer` object, we eventually call > `ASTUnit::LoadFromCommandLine()` and `ASTUnit` has a member > `FileSystemOptions FileSystemOpts`, and

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-10 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D139774#4039975 , @aaron.ballman wrote: > Oh that is a good point! Apologies for not noticing that earlier -- that > makes me wonder if a better approach here is to add a `std::string` to the > `CIndexer` class to represent

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-10 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D139774#4036496 , @aaron.ballman wrote: > Given that we already have other setters to set global state, I'm less > concerned about adding another one. I hadn't realized we already introduced > the dangers here. But we should

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-09 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy marked an inline comment as not done. vedgy added a comment. In D139774#4036496 , @aaron.ballman wrote: > In terms of the C API, I think it'd make more sense to name in terms of > "override" rather than "set", but I don't feel as strongly about

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-06 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy marked an inline comment as done and 2 inline comments as not done. vedgy added inline comments. Comment at: llvm/include/llvm/Support/Path.h:423 +/// tempDirUtf8 pointer previously passed to this function. +void set_system_temp_directory_erased_on_reboot(const char

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-04 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D139774#4025356 , @vedgy wrote: > `clang_disposeIndex()` would have to un-override the temporary directory > then. I'll have to check whether this un-overriding happens too early (before > all //preamble-*.pch// files are

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-04 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment. In D139774#4023940 , @aaron.ballman wrote: > This feels like a configuration property of the libclang execution -- so it'd > be nice to require it to be set up from `clang_createIndex()` (IIRC that's > the entrypoint for CIndex

[PATCH] D139774: [libclang] Add API to set temporary directory location

2022-12-11 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy updated this revision to Diff 481913. vedgy edited the summary of this revision. vedgy added a comment. Extract identical code from the two Path.inc files into Path.cpp One of the Path.inc files is #include-d into this Path.cpp file and nowhere else. Repository: rG LLVM Github

[PATCH] D139774: [libclang] Add API to set temporary directory location

2022-12-10 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy created this revision. vedgy added a reviewer: aaron.ballman. Herald added subscribers: arphaman, hiraditya. Herald added a project: All. vedgy requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Fixes: