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

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 the

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

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 https://review

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

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

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 cf

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

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 cfe-commits@li

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

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

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()` func

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

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 [[ > https://github.com/ll

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

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 [[ https://github.com/llvm/llvm-project/c

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

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) static_cast(CIdx)->setStorePrea

[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] D145974: [libclang] Add index option to store preambles in memory

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 temporar

[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] D145783: Reserve unused bits in struct CXIndexOptions; NFC

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: clang/inc

[PATCH] D145775: Improve documentation of CXIndexOptions; NFC

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 Lib

[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

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] D130303: Handle template parameter-dependent bit field widths in libclang

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 declaratio

[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

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

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

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] D130303: Handle template parameter-dependent bit field widths in libclang

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

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] D130303: Handle template parameter-dependent bit field widths in libclang

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

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] D130303: Handle template parameter-dependent bit field widths in libclang

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 cou

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

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 com

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

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: clan

[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

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

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

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] D143415: LibclangTest: remove libclang-test-* tmp dir reliably

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 -- what

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

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 l

[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

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

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

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

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 clang/u

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

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

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 `CXIndexOpti

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

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

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")) >> CIdxr->setCXGlobalOptFlags(CIdxr->get

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

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. `uint32_t`

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

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 CH

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

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 l

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

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 sensit

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

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 t

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

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 tempor

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

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

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 https://reviews.llvm.org/D1

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

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 https://reviews

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

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

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 https://reviews.llvm.org/D139774

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

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 llvm::sys::fs::createT

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

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 - rem

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

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 ca

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

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` agai

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

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 co

[PATCH] D140756: Add clang_CXXMethod_isExplicit to libclang

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: > `CINDEX_VERSI

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

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 libclang

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

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`. S

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

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 happ

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

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 structure

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

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 t

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

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 cons

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

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 > option

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

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 https://reviews.llv

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

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

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 fine.

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

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 l

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

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, s

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

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 configurati

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

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 want

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

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

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

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 `FileSystemOpti

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

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 th

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

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 d

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

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 it

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

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 *tempDi

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

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 remov

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

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 f

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

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 Monorepo

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

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: https://github.co