[PATCH] D54760: [clangd] Cleanup: make the diags callback global in TUScheduler

ilya-biryukov planned changes to this revision. ilya-biryukov added a comment. Would like to land https://reviews.llvm.org/D54829 first and rebase on top of it. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54760 ___ cfe-commits m

[PATCH] D54829: [clangd] Cleanup: make diagnostics callbacks from TUScheduler non-racy

ilya-biryukov added inline comments. Comment at: clangd/TUScheduler.cpp:241 ParseInputs FileInputs; + /// Used to indicate to the tasks running inside the worker queue that AST is + /// being destroyed and the AST queue is being drained. Note to myself: shou

[PATCH] D54829: [clangd] Cleanup: make diagnostics callbacks from TUScheduler non-racy

ilya-biryukov updated this revision to Diff 175037. ilya-biryukov added a comment. - Ensure consistency for diagnostic callbacks with a critical section. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54829 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/TUSch

[PATCH] D54829: [clangd] Cleanup: make diagnostics callbacks from TUScheduler non-racy

ilya-biryukov updated this revision to Diff 175042. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. - Added comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54829 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/TUScheduler

[PATCH] D54829: [clangd] Cleanup: make diagnostics callbacks from TUScheduler non-racy

ilya-biryukov added inline comments. Comment at: clangd/TUScheduler.cpp:424 if (*AST) { OnUpdated((*AST)->getDiagnostics()); trace::Span Span("Running main AST callback"); sammccall wrote: > as discussed offline, this doesn't guarantee we're no

[PATCH] D52273: [clangd] Initial implementation of expected types

ilya-biryukov updated this revision to Diff 175050. ilya-biryukov marked 11 inline comments as done. ilya-biryukov added a comment. - Address comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52273 Files: clangd/CMakeLists.txt clangd/ExpectedTypes.cpp clangd/Expecte

[PATCH] D52273: [clangd] Initial implementation of expected types

ilya-biryukov added inline comments. Comment at: clangd/ExpectedTypes.h:32 +/// this allows the representation to be stored in the index and compared with +/// types coming from a different AST later. +class OpaqueType { sammccall wrote: > ilya-biryukov wrote: >

[PATCH] D54829: [clangd] Cleanup: make diagnostics callbacks from TUScheduler non-racy

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.h:132 /// Remove \p File from list of tracked files, schedule a request to free - /// resources associated with it. + /// resources associated with it. After this function returns, we guarantee + /// that

[PATCH] D54829: [clangd] Cleanup: make diagnostics callbacks from TUScheduler non-racy

ilya-biryukov updated this revision to Diff 175053. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. - Address the comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54829 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/TUSch

[PATCH] D54760: [clangd] Cleanup: make the diags callback global in TUScheduler

ilya-biryukov updated this revision to Diff 175056. ilya-biryukov added a comment. - Rebase - Remove makeUpdateCallbacks, expose the callback class instead Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54760 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/TUS

[PATCH] D52273: [clangd] Initial implementation of expected types

ilya-biryukov updated this revision to Diff 175098. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Add using namespace llvm, get rid of llvm:: Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52273 Files: clangd/CMakeLists.txt clangd/Expected

[PATCH] D52274: [clangd] Collect and store expected types in the index

ilya-biryukov updated this revision to Diff 175103. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Remove accidental changes - Add types to binary serialization Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52274 Files: clangd/index/Index.h

[PATCH] D52274: [clangd] Collect and store expected types in the index

ilya-biryukov updated this revision to Diff 175104. ilya-biryukov added a comment. - Remove another accidental change Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52274 Files: clangd/index/Index.h clangd/index/Serialization.cpp clangd/index/SymbolCollector.cpp clangd/

[PATCH] D52274: [clangd] Collect and store expected types in the index

ilya-biryukov added inline comments. Comment at: clangd/index/Index.cpp:118 +static void own(Symbol &S, UniqueStringSaver &Strings, +BumpPtrAllocator &Arena) { visitStrings(S, [&](StringRef &V) { V = Strings.save(V); }); sammccall wrote: > why

[PATCH] D52276: [clangd] Add type boosting in code completion

ilya-biryukov updated this revision to Diff 175114. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Increase the multiplier - Move the member out of the file proximity group Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52276 Files: clangd/Co

[PATCH] D52276: [clangd] Add type boosting in code completion

ilya-biryukov added inline comments. Comment at: clangd/Quality.cpp:373 + if (TypeMatchesPreferred) +Score *= 2.0; + sammccall wrote: > is 2 really enough? Changed to 5. We can tweak it later if that turns out to be too big. Comment at: cl

[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol

ilya-biryukov updated this revision to Diff 175116. ilya-biryukov added a comment. - Do not drop anonymous enums Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52311 Files: clangd/AST.cpp clangd/AST.h clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdSe

[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol

ilya-biryukov added inline comments. Comment at: unittests/clangd/FindSymbolsTests.cpp:442 +SymNameRange(Main.range("decl"), + AllOf(WithName("f"), WithKind(SymbolKind::Method), +SymN

[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol

ilya-biryukov updated this revision to Diff 175130. ilya-biryukov added a comment. - Handle 'using namespace' in printName - Do not mix-in symbols with locations from other files. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52311 Files: clangd/AST.cpp clangd/AST.h clan

[PATCH] D54878: [clangd] NFC: Eliminate the unused variable warning.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. Ah, sorry, landed a cleanup fixing the warnings (rL347539 ) before seeing this change. But given that `isa<>` are still better than `dyn_cast<>`, this change might still be worth landing. R

[PATCH] D52276: [clangd] Add type boosting in code completion

ilya-biryukov marked 4 inline comments as done. ilya-biryukov added inline comments. Comment at: clangd/Quality.h:98 + /// Whether the item matches the type expected in the completion context. + bool TypeMatchesPreferred = false; /// FIXME: unify with index proximity score -

[PATCH] D52276: [clangd] Add type boosting in code completion

ilya-biryukov updated this revision to Diff 175223. ilya-biryukov added a comment. - Address comments, add more flags to signals Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52276/new/ https://reviews.llvm.org/D52276 Files: clangd/CodeComplete.c

[PATCH] D54796: [clangd] C++ API for emitting file status

ilya-biryukov added a comment. In D54796#1306960 , @hokein wrote: > There is one thing I'm not certain: should we stop emitting the file status > when the file is removed (similar to the behavior of diagnostics)? For > example, the file is removed while

[PATCH] D54872: [clangd] Add ability to not use -resource-dir at all

ilya-biryukov added a comment. We have to point clangd into the resource dir, corresponding to the version of the headers it was built with. It's important we pick the exact version of the built-in headers that clangd was built for. Note that resource-dir should **only** be used for the built-in

[PATCH] D54878: [clangd] NFC: Prefer `isa<>` to `dyn_cast<>` to do the checking.

ilya-biryukov added a comment. In D54878#1307845 , @MTC wrote: > We can land this change this time or do the cleaning job in other patches in > the future, it's all up to you guys, the active clangd contributors :). Please land this, it's a useful clean

[PATCH] D52274: [clangd] Collect and store expected types in the index

ilya-biryukov marked 3 inline comments as done. ilya-biryukov added inline comments. Comment at: clangd/index/Index.h:283 + /// Type of the symbol, used for scoring purposes. + llvm::StringRef Type; sammccall wrote: > either call this OpaqueType or point at i

[PATCH] D52274: [clangd] Collect and store expected types in the index

ilya-biryukov updated this revision to Diff 175246. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Bump the RIFF version number - Place the Type field after the ReturnType - Address other comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION

[PATCH] D45478: [clangd] Merge symbols in global-sym-builder on the go

ilya-biryukov abandoned this revision. ilya-biryukov added a comment. Herald added subscribers: kadircet, arphaman. This landed as a different change. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45478/new/ https://reviews.llvm.org/D45478 __

[PATCH] D52274: [clangd] Collect and store expected types in the index

ilya-biryukov updated this revision to Diff 175254. ilya-biryukov added a comment. - Remove unused #include Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52274/new/ https://reviews.llvm.org/D52274 Files: clangd/index/Index.h clangd/index/Serial

[PATCH] D52276: [clangd] Add type boosting in code completion

ilya-biryukov updated this revision to Diff 175257. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52276/new/ https://reviews.llvm.org/D52276 Files:

[PATCH] D52276: [clangd] Add type boosting in code completion

ilya-biryukov updated this revision to Diff 175258. ilya-biryukov added a comment. - Add a FIXME to FuzzyFindRequest Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52276/new/ https://reviews.llvm.org/D52276 Files: clangd/CodeComplete.cpp clangd/

[PATCH] D54630: Move detection of libc++ include dirs to Driver on MacOS

ilya-biryukov added a comment. In D54630#1308671 , @dexonsmith wrote: > I'm a little skeptical that splitting this logic between cc1 and the driver > will simplify things, but I haven't looked in detail and I'll defer to your > (collective) judgement.

[PATCH] D54781: [clangd] Add 'Switch header/source' command in clangd-vscode

ilya-biryukov added a comment. +1 to the new release D52311 (documentSymbol) needs a bump of the dependencies' versions. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54781/new/ https://reviews.llvm.org/D54781

[PATCH] D54872: [clangd] Add ability to not use -resource-dir at all

ilya-biryukov added a comment. In D54872#1309075 , @malaperle wrote: > In D54872#1307775 , @ilya-biryukov > wrote: > > > We have to point clangd into the resource dir, corresponding to the version > > of the heade

[PATCH] D54952: [clangd] DO NOT SUBMIT. Draft interfaces for build system integration.

ilya-biryukov created this revision. ilya-biryukov added reviewers: klimek, sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric. A proposal for the interface of the new build system integration. Similar to CompilationDatabase, but will provide more functionality and l

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

ilya-biryukov requested changes to this revision. ilya-biryukov added a comment. This revision now requires changes to proceed. I don't think isVolatile is the right default at the FileManager level, even on Windows. E.g. memory mapping is probably optimal for compiler, even though it definitely

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

ilya-biryukov added a comment. In D54995#1311010 , @yvvan wrote: > @ilya-biryukov > > I have the reported evidence but didn't yet have time to investigate myself. > This is probably caused by our upgrade to Clang-7 which shows that we can't > rely on the

[PATCH] D54998: [clangd] Build and test IndexBenchmark in check-clangd

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54998/new/ https://reviews.llvm.org/D54998 __

[PATCH] D54999: [clangd] Populate include graph during static indexing action.

ilya-biryukov added inline comments. Comment at: clangd/index/IndexAction.cpp:23 +// Collects the nodes and edges of include graph during indexing action. +struct IncludeGraphCollector : public PPCallbacks { +public: Could we add an assertion that the final graph

[PATCH] D54952: [clangd] DO NOT SUBMIT. Draft interfaces for build system integration.

ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. Thanks for taking a look and sorry for the confusion with naming. I should've renamed `Integration` before sending this patch. Comment at: clangd/BuildSystem.h:29 +/// Default compilation database u

[PATCH] D54781: [clangd] Add 'Switch header/source' command in clangd-vscode

ilya-biryukov added a comment. In D54781#1311038 , @hokein wrote: > Just made a new release v0.0.7, please try to use it (it works on my machine). Many thanks! Works for me Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

ilya-biryukov added a subscriber: zturner. ilya-biryukov added a comment. cross-posting @zturner's comment from the mailing thread for the record: > I’m afraid this is going to be too severe a performance regression if we > change the default. So I don’t think this should be an LLVM-wide default

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

ilya-biryukov added a comment. In D54995#1311303 , @yvvan wrote: > "could we figure out the exact cause of the issue?" > And this review was not meant to proceed immediately but rather state the > intention and get some feedback because I still don't kno

[PATCH] D54999: [clangd] Populate include graph during static indexing action.

ilya-biryukov added inline comments. Comment at: clangd/index/IndexAction.cpp:45 +auto &Node = I->getValue(); +if (auto Digest = digestFile(SM, FileID)) + Node.Digest = std::move(*Digest); kadircet wrote: > ilya-biryukov wrote: > > What happens if we

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

ilya-biryukov added a comment. In D54995#1311427 , @zturner wrote: > My first thought would be that it depends on the flags to CreateFile moreso > (and perhaps entirely) rather than the flags to MapViewOfFile or > CreateFileMapping. Specifically, the `F

[PATCH] D54999: [clangd] Populate include graph during static indexing action.

ilya-biryukov added inline comments. Comment at: clangd/index/IndexAction.cpp:12 +llvm::Optional URIFromFileEntry(const FileEntry *File) { + if (!File) NIT: maybe call it `toURI`? Comment at: clangd/index/IndexAction.cpp:23 +// Collects the

[PATCH] D55052: Fix junk output in clangd vscode plugin

ilya-biryukov added a comment. +1 to the change, this is annoying for me too. In D55052#1312753 , @hokein wrote: > @jfindley I'd like to understand how do these log messages noise you? Does > the window prompt up to you automatically (this only happens w

[PATCH] D55052: Fix junk output in clangd vscode plugin

ilya-biryukov added inline comments. Comment at: clangd/clients/clangd-vscode/src/extension.ts:58 +}, +// Avoid lots of junk in output +revealOutputChannelOn: vscodelc.RevealOutputChannelOn.Never Could we rephrase this? Something like `//

[PATCH] D55052: Fix junk output in clangd vscode plugin

ilya-biryukov added inline comments. Comment at: clangd/clients/clangd-vscode/src/extension.ts:58 +}, +// Avoid lots of junk in output +revealOutputChannelOn: vscodelc.RevealOutputChannelOn.Never ilya-biryukov wrote: > Could we rephrase th

[PATCH] D55052: Fix junk output in clangd vscode plugin

ilya-biryukov added a comment. In D55052#1312766 , @hokein wrote: > For cancellation errors, it might be reasonable, but what if other > unrecoverable errors in clangd (e.g. clangd crashes), in these cases, users > don't know what happens, and would stil

[PATCH] D54999: [clangd] Populate include graph during static indexing action.

ilya-biryukov added inline comments. Comment at: clangd/index/IndexAction.cpp:31 + // Populates the following fields of the corresponding node for a new include: + // - Digest -> SHA1 hash of the file. + // - IsTU -> true if the file is the main file the indexing action has b

[PATCH] D54999: [clangd] Populate include graph during static indexing action.

ilya-biryukov added inline comments. Comment at: clangd/Headers.h:64 +// Important: The graph generated by those callbacks might contain cycles and +// self edges. using IncludeGraph = llvm::StringMap; And multi-edges too, right? Even though they're not useful.

[PATCH] D54999: [clangd] Populate include graph during static indexing action.

ilya-biryukov added inline comments. Comment at: unittests/clangd/IndexActionTests.cpp:168 + std::string MainFilePath = testPath("main.cpp"); + std::pair CommonHeader = {testPath("common.h"), + R"cpp( kadirce

[PATCH] D55054: [clang] Fill RealPathName for virtual files.

ilya-biryukov added inline comments. Comment at: include/clang/Basic/FileManager.h:177 + /// Fills the RealPathName entry in UFE by converting the given filename to an + /// absolute path, returns true if FileName was modified. + bool fillRealPathName(FileEntry *UFE, llvm::St

[PATCH] D55062: [clangd] Partition include graph on auto-index.

ilya-biryukov added inline comments. Comment at: clangd/index/Background.cpp:184 +// We keep only the node "Path" and its edges. +IncludeGraph getSubGraph(llvm::StringRef Path, const IncludeGraph &FullGraph) { + IncludeGraph IG; We should make the function stati

[PATCH] D54796: [clangd] C++ API for emitting file status

ilya-biryukov added a comment. > @ilya-biryukov, I hope the current patch is not too big for you to review, > happy to chat offline if you want (sam and I had a lot of discussions before > he is OOO). Picking it up. Mostly NITs from my side and a few questions to better understand the design.

[PATCH] D54796: [clangd] C++ API for emitting file status

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.h:49 + /// Called whenever the file status is updated. + virtual void onFileUpdated(PathRef File, const TUStatus &Status){}; }; hokein wrote: > ilya-biryukov wrote: > > Have we thought about

[PATCH] D55124: [CodeComplete] Cleanup access checking in code completion

ilya-biryukov created this revision. ilya-biryukov added a reviewer: ioeric. Also fixes a crash (see the added 'accessibility-crash.cpp' test). Repository: rC Clang https://reviews.llvm.org/D55124 Files: include/clang/Sema/Sema.h lib/Sema/CodeCompleteConsumer.cpp lib/Sema/SemaAccess.cp

[PATCH] D55054: [clang] Fill RealPathName for virtual files.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM and a few nits Comment at: include/clang/Basic/FileManager.h:108 + + // Only for use in tests to see if deferred opens are happening, arther than + // re

[PATCH] D54999: [clangd] Populate include graph during static indexing action.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Thank, LGTM. A few NITs. Comment at: clangd/index/IndexAction.h:31 +std::function RefsCallback, +std::function IncludeGraphCallback = nullptr); --

[PATCH] D54630: Move detection of libc++ include dirs to Driver on MacOS

ilya-biryukov added a comment. @arphaman, did you have a chance to run the tests? There's not rush, just wanted to know whether we have any data at all at how Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54630/new/ https://reviews.llvm.org/D54630 ___

[PATCH] D55062: [clangd] Partition include graph on auto-index.

ilya-biryukov added inline comments. Comment at: clangd/index/Background.cpp:70 +// We keep only the node "Path" and its edges. +IncludeGraph getSubGraph(const URI &Uri, const IncludeGraph &FullGraph) { + IncludeGraph IG; Naming: technically the variable name sh

[PATCH] D55124: [CodeComplete] Cleanup access checking in code completion

ilya-biryukov updated this revision to Diff 176150. ilya-biryukov added a comment. - Add missed field init - Also fix access checks for member access with qualifiers Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55124/new/ https://reviews.llvm.org/D55124 Files:

[PATCH] D55124: [CodeComplete] Cleanup access checking in code completion

ilya-biryukov updated this revision to Diff 176153. ilya-biryukov added a comment. - Fix a comment Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55124/new/ https://reviews.llvm.org/D55124 Files: include/clang/Sema/Sema.h lib/Parse/ParseExprCXX.cpp lib/Sema

[PATCH] D54952: [clangd] DO NOT SUBMIT. Draft interfaces for build system integration.

ilya-biryukov added inline comments. Comment at: clangd/BuildSystem.h:29 +/// Default compilation database used by clangd, based on the build system. +class Integration : public GlobalCompilationDatabase { +public: klimek wrote: > ilya-biryukov wrote: > > klimek

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

ilya-biryukov added a comment. In D54995#1312617 , @lebedev.ri wrote: > Passing-by thought, feel free to ignore: this seems to so far only affect > windows only? > So the fix shouldn't probably pessimize all other arches? (and maybe even > non-clangd)

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

ilya-biryukov added a comment. In D54995#1314912 , @zturner wrote: > Original patch description says this: > > > There are reported cases of non-system files being locked by libclang on > > Windows (and likely by other clients as well) > > What is the nat

[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

ilya-biryukov created this revision. ilya-biryukov added reviewers: hokein, kadircet. Herald added subscribers: arphaman, jkorous, MaskRay, ioeric, mgorny. Memory-mapping files on Windows leads to them being locked and prevents editors from saving changes to those files on disk. This is fine for t

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

ilya-biryukov added a comment. In D54995#1315024 , @zturner wrote: > We can work around it by reading the whole file, but it looks like a bug in > QtCreator to me. We have the file mapped, but maybe when they open the file > to save it, *they* are openi

[PATCH] D55124: [CodeComplete] Cleanup access checking in code completion

ilya-biryukov updated this revision to Diff 176327. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Do not introduce a new local var, reuse existing Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55124/new/ https://reviews.llvm.org

[PATCH] D55124: [CodeComplete] Cleanup access checking in code completion

ilya-biryukov added inline comments. Comment at: lib/Parse/ParseExprCXX.cpp:248 // seen a leading '::' or part of a nested-name-specifier. + ParsedType ObjectTypeForCompletion = ObjectType; ObjectType = nullptr; kadircet wrote: > What about fir

[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

ilya-biryukov planned changes to this revision. ilya-biryukov added a comment. We need to keep mmapping the PCH files. In D55139#1315944 , @malaperle wrote: > Hi Ilya. Does this apply to compile_commands.json too? I've seen that problem > for that file a

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

ilya-biryukov added a comment. It seems there are still cases where memory-mapping should be preferred, even on Windows, specifically: - PCH files produced by libclang: they are huge, loading them in memory is wasteful and they can clearly be used - (maybe?) PCM (precompiled module) files: they

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

ilya-biryukov added a comment. In D54995#1316437 , @yvvan wrote: > Ok, no global option. > Why not placing your VolatileFSProvider in clang so that libclang could you > it too? Would be happy to, will need to figure out what to do with PCH and PCM file

[PATCH] D55191: [clangd] Refine the way of checking a declaration is referenced by the written code.

ilya-biryukov added a comment. The **idea** of using the AST-based approach here was really nice, it was less expensive and seemed to clearly look at the semantic. I wonder if there's a way to keep it on the AST level, without looking at the source locations. What are the cases we're trying to

[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

ilya-biryukov updated this revision to Diff 176381. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. This revision is now accepted and ready to land. - Keep using memory-mapped files for PCHs - Move once Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTIO

[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clangd/FSProvider.cpp:33 + return File; +return std::unique_ptr( +new VolatileFile(std::move(std::move(*File; kadircet wrote: > make_unique? Unfo

[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

ilya-biryukov updated this revision to Diff 176382. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - s/VolatileFSProvider/VolatileFileSystem Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55139/new/ https://reviews.llv

[PATCH] D54630: Move detection of libc++ include dirs to Driver on MacOS

ilya-biryukov added a comment. In D54630#1315168 , @arphaman wrote: > So far everything looks fine, but I have to rerun a couple of things due to > infrastructural issues. I should have the final results next Monday. Thanks for the update. Will be waiti

[PATCH] D55062: [clangd] Partition include graph on auto-index.

ilya-biryukov added a comment. Everything looks good, just a single important request about testing the included files do not have any edges in the resulting graph Comment at: clangd/index/Background.cpp:79 + + // Since the strings in direct includes are references to the key

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

ilya-biryukov added a comment. In D54995#1316476 , @yvvan wrote: > I don't think removing the flag is a good idea since I can easily assume > cases when user wants mmap and is ready to encounter locks. In our case it > can be an IDE option which behavior

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

ilya-biryukov added inline comments. Comment at: include/clang/Frontend/ASTUnit.h:581 + unsigned getPreambleCounter() const { return PreambleCounter; } + NIT: `getPreambleCounterForTests()`? This is clearly an internal detail, would try giving it a name that

[PATCH] D54796: [clangd] C++ API for emitting file status

ilya-biryukov added a comment. Sorry for the delays. Not sure about emitting `Queued` on the main thread, see the corresponding comment. Otherwise looks very good. Comment at: clangd/ClangdServer.h:49 + /// Called whenever the file status is updated. + virtual void onFileUpd

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

ilya-biryukov added inline comments. Comment at: include/clang/Lex/Preprocessor.h:391 } PreambleConditionalStack; + bool PreambleGenerationFailed = false; There's a mechanism to handle preamble with errors, see `PreprocessorOpts::AllowPCHWithCompilerErrors

[PATCH] D55062: [clangd] Partition include graph on auto-index.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: unittests/clangd/BackgroundIndexTests.cpp:204 + EXPECT_EQ(ShardSource->Sources->size(), 2U); // A.cc, A.h + EXPECT_THAT( + ShardSource->

[PATCH] D55260: [CodeComplete] Fix a crash in access checks of inner classes

ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. The crash was introduced in r348135. Repository: rC Clang https://reviews.llvm.org/D55260 Files: lib/Sema/SemaCodeComplete.cpp test/CodeCompletion/accessibility.cpp Index: test/CodeCompletion/accessibility.c

[PATCH] D55260: [CodeComplete] Fix a crash in access checks of inner classes

ilya-biryukov updated this revision to Diff 176593. ilya-biryukov added a comment. - Actually use the computed NamingClass Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55260/new/ https://reviews.llvm.org/D55260 Files: lib/Sema/SemaCodeComplete.cpp test/Code

[PATCH] D55312: [clangd] Fix a typo in TUSchedulerTests

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55312/new/ https://reviews.llvm.org/D55312 __

[PATCH] D54872: [clangd] Add ability to not use -resource-dir at all

ilya-biryukov added a comment. In D54872#1319684 , @malaperle wrote: > It doesn't seem like there is any difference in how -resource-dir and /imsvc > are handled: they are all added as -internal-isystem Thanks for digging this code up. I'm a bit confuse

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

ilya-biryukov added inline comments. Comment at: clangd/Threading.h:125 +// Sets scheduling priority for the calling thread. +void setThreadPriority(ThreadPriority Priority); // Avoid the use of scheduler policies that may starve low-priority threads. Maybe chan

[PATCH] D55322: Mention changes to libc++ include dir lookup in release notes.

ilya-biryukov created this revision. ilya-biryukov added a reviewer: arphaman. Herald added a reviewer: EricWF. The change itself landed as r348365, see the comment for more details. Repository: rC Clang https://reviews.llvm.org/D55322 Files: docs/ReleaseNotes.rst Index: docs/ReleaseNote

[PATCH] D54630: Move detection of libc++ include dirs to Driver on MacOS

ilya-biryukov added a comment. D55322 is a review for release notes Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54630/new/ https://reviews.llvm.org/D54630 ___ cfe-commits mailing l

[PATCH] D55260: [CodeComplete] Fix a crash in access checks of inner classes

ilya-biryukov updated this revision to Diff 176820. ilya-biryukov added a comment. - Make sure we still run ObjC access checks. - Add a test with a different NamingClass and BaseType. - Fix IsSimplyAccessible to work in this case. Repository: rC Clang CHANGES SINCE LAST ACTION https://revie

[PATCH] D55260: [CodeComplete] Fix a crash in access checks of inner classes

ilya-biryukov updated this revision to Diff 176821. ilya-biryukov added a comment. - Reformat Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55260/new/ https://reviews.llvm.org/D55260 Files: lib/Sema/SemaAccess.cpp lib/Sema/SemaCodeComplete.cpp test/CodeCom

[PATCH] D55260: [CodeComplete] Fix a crash in access checks of inner classes

ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. In D55260#1318280 , @kadircet wrote: > I believe also we need another test case where `Cls` and `NamingClass` are > different. Done. And thanks for finding this, I totally mi

[PATCH] D55260: [CodeComplete] Fix a crash in access checks of inner classes

ilya-biryukov updated this revision to Diff 176823. ilya-biryukov added a comment. - Add a newline to the end of the test file Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55260/new/ https://reviews.llvm.org/D55260 Files: lib/Sema/SemaAccess.cpp lib/Sema/Se

[PATCH] D55331: [CodeComplete] Fix assertion failure

ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. ...that fires when running completion inside an argument of UnresolvedMemberExpr (see the added test). The assertion that fires is from Sema::TryObjectArgumentInitialization: assert(FromClassification.isLValue());

[PATCH] D54796: [clangd] C++ API for emitting file status

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, still have a few NITs and suggestions (see the comment about emitting "queued" on waiting for diagnostics and **not** emitting queued if the lock was acquired from the firs

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

ilya-biryukov added a comment. It looks like the integrate went smoothly with the new change, thanks for fixing it! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55085/new/ https://reviews.llvm.org/D55085 ___ cfe-commits mailing list cfe-c

[PATCH] D55250: [clangd] Enhance macro hover to see full definition

ilya-biryukov added inline comments. Comment at: clangd/XRefs.cpp:578 +bool Invalid; +StringRef Buffer = SM.getBufferData(SM.getFileID(StartLoc), &Invalid); +if (!Invalid) { Unfortunately we can't get the buffer here, because for the preamble macros t

<    6   7   8   9   10   11   12   13   14   15   >