[PATCH] D31746: Remove ASTUnits for closed documents and cache CompilationDatabase per directory in clangd.

ilya-biryukov updated this revision to Diff 94494. ilya-biryukov added a comment. Moved locking of ClangObjectLock into request handlers. https://reviews.llvm.org/D31746 Files: clangd/ASTManager.cpp clangd/ASTManager.h clangd/ClangDMain.cpp clangd/Protocol.cpp clangd/Protocol.h

[PATCH] D31746: Remove ASTUnits for closed documents and cache CompilationDatabase per directory in clangd.

ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. Regarding the testing. It looks like we have two ways of testing this: 1. Add clangd-specific protocol handlers that output useful stats(i.e. currently opened documents), use those in tests. 2. Add unit tests that

[PATCH] D31746: Remove ASTUnits for closed documents and cache CompilationDatabase per directory in clangd.

ilya-biryukov updated this revision to Diff 94492. ilya-biryukov added a comment. Minor fixes. Fixed variable name issues and comment spelling errors. https://reviews.llvm.org/D31746 Files: clangd/ASTManager.cpp clangd/ASTManager.h clangd/ClangDMain.cpp clangd/Protocol.cpp

[PATCH] D31746: Remove ASTUnits for closed documents and cache CompilationDatabase per directory in clangd.

ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. Addressed the locking comments. Locking inside the request handlers looks much nicer indeed. Comment at: clangd/ASTManager.cpp:203 + // TODO(ibiryukov): at this point DocDatasLock can be unlocked

[PATCH] D35986: [clangd] Add ':' to completion trigger characters.

ilya-biryukov created this revision. Without it we don't get completion requests from VSCode after nested name qualifiers (e.g. after 'std::'). https://reviews.llvm.org/D35986 Files: clangd/ClangdLSPServer.cpp Index: clangd/ClangdLSPServer.cpp

[PATCH] D36095: [clangd] Allow to get vfs::FileSystem used inside codeComplete.

ilya-biryukov created this revision. This is useful for managing lifetime of VFS-based caches. https://reviews.llvm.org/D36095 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h Index: clangd/ClangdServer.h === ---

[PATCH] D36095: [clangd] Allow to get vfs::FileSystem used inside codeComplete.

ilya-biryukov added a comment. In https://reviews.llvm.org/D36095#826275, @bkramer wrote: > lg (it could use a test case though) Will add a test case in one of further commits. Repository: rL LLVM https://reviews.llvm.org/D36095 ___

[PATCH] D36133: [clangd] Rewrote AST and Preamble management.

ilya-biryukov created this revision. The new implementation allows code completion that never waits for AST. https://reviews.llvm.org/D36133 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/ClangdUnitStore.cpp

[PATCH] D35894: [clangd] Code hover for Clangd

ilya-biryukov added a reviewer: ilya-biryukov. ilya-biryukov requested changes to this revision. ilya-biryukov added a comment. I just wanted to take a step back and discuss what we want to get from code hover in the first place. I would expect a typical code hover to be a bit more involved and

[PATCH] D35894: [clangd] Code hover for Clangd

ilya-biryukov added a comment. In https://reviews.llvm.org/D35894#829190, @malaperle wrote: > @Nebiroth I think it's OK to put this on hold until we make the "semantic" > hover and figure out how to have both. From our perspective, this is going > beyond parity of what we had before but it's

[PATCH] D36226: Added a RealFileSystem implementation that does not rely on global CWD.

ilya-biryukov planned changes to this revision. ilya-biryukov added a comment. After a chat with @klimek, working on an implementation that would use `openAt()`. https://reviews.llvm.org/D36226 ___ cfe-commits mailing list

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

ilya-biryukov created this revision. https://reviews.llvm.org/D36261 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/tool/ClangdMain.cpp unittests/clangd/ClangdTests.cpp Index: unittests/clangd/ClangdTests.cpp

[PATCH] D36150: [clangd] LSP extension to switch between source/header file

ilya-biryukov requested changes to this revision. ilya-biryukov added a comment. This revision now requires changes to proceed. This is probably not a final version, but I feel my comments should be helpful anyway. Also, we're missing testcases now. Comment at:

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

ilya-biryukov updated this revision to Diff 109532. ilya-biryukov added a comment. - Removed SchedulingOptions altogether, replaced with AsyncThreadsCount. https://reviews.llvm.org/D36261 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

ilya-biryukov updated this revision to Diff 109534. ilya-biryukov marked 4 inline comments as done. ilya-biryukov added a comment. - Fixed more typos/inconsistences in comments, pointed out by @krasimir. https://reviews.llvm.org/D36261 Files: clangd/ClangdLSPServer.cpp

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

ilya-biryukov added inline comments. Comment at: clangd/tool/ClangdMain.cpp:69 + SchedulingOptions SchedOpts = + !RunSynchronously ? SchedulingOptions::RunOnWorkerThreads(ThreadsCount) +: SchedulingOptions::RunOnCallingThread();

[PATCH] D36226: Added a RealFileSystem implementation that does not rely on global CWD.

ilya-biryukov added a comment. Requires changes in Support library to run correctly: https://reviews.llvm.org/D36265 https://reviews.llvm.org/D36226 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D36397: [clangd] Fixed a data race.

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:714 -void CppFile::cancelRebuilds() { +void CppFile::cancelRebuild() { deferCancelRebuild().get(); } + klimek wrote: > Somewhat orthogonal question - why CppFile? We do support other

[PATCH] D36397: [clangd] Fixed a data race.

ilya-biryukov added a comment. A follow-up after discussion offline: we should make an attempt to simplify the threading code in `ClangdUnit.h` by moving the async worker loop of `CppFile` into the class itself, rather than relying on external scheduling to do the right thing. @klimek, are

[PATCH] D36397: [clangd] Fixed a data race.

ilya-biryukov added a comment. Thanks. I'll make sure to address the awkwardness in further commits. https://reviews.llvm.org/D36397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36150: [clangd] LSP extension to switch between source/header file

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:295 + std::string DEFAULT_SOURCE_EXTENSIONS[] = { ".cpp", ".c", ".cc", ".cxx", +".c++", ".C", ".m", ".mm" }; + std::string DEFAULT_HEADER_EXTENSIONS[] = { ".h", ".hh", ".hpp", ".hxx",

[PATCH] D35200: Don't use mmap on Windows

ilya-biryukov added a comment. In https://reviews.llvm.org/D35200#844132, @yvvan wrote: > The files might also be removed. > I checked that with qtcreator from git. Switching to the different branch > started reparse of some files. > When I frequently switched branches back and forth I got

[PATCH] D35200: Don't use mmap on Windows

ilya-biryukov added a comment. This looks strange. AFAIK, memory mapping files in Windows does no 'locking' by itself. I've just written a small program to check that it's possible to modify the file, memory-mapped using Win32 API. What is specific problem you ran into?

[PATCH] D36828: Updated a usage of createTemporaryFile that does not expect file to be created.

ilya-biryukov created this revision. This fixes a usage of createTemporaryFile in clang repo after a change in llvm repo. https://reviews.llvm.org/D36828 Files: unittests/Tooling/ToolingTest.cpp Index: unittests/Tooling/ToolingTest.cpp

[PATCH] D36828: Updated a usage of createTemporaryFile that does not expect file to be created.

ilya-biryukov added a comment. A change to llvm repository is here: https://reviews.llvm.org/D36827 https://reviews.llvm.org/D36828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36529: Fixed a race condition in PrecompiledPreamble.

ilya-biryukov added inline comments. Comment at: lib/Frontend/PrecompiledPreamble.cpp:470-471 + int FD; + auto EC = llvm::sys::fs::createTemporaryFile(Prefix, Suffix, /*ref*/ FD, + /*ref*/ File); if (EC) klimek

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

ilya-biryukov added a comment. The idea is to allows us changing the way we manage/rebuild PCHs and ASTs. ASTUnit is used for many things and has a fairly complicated and verbose interface and does a lot of mutations all other the place inside it, so making changes to it is not particularly

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

ilya-biryukov added a comment. In https://reviews.llvm.org/D35406#809623, @malaperle wrote: > For synching with what I am doing. I am "only" looking right now at the > modeling of the index and its on-disk storage, not really on the speeding up > of the parsing of the TUs (i.e. the input of

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

ilya-biryukov updated this revision to Diff 106632. ilya-biryukov added a comment. - Replaced TODO with FIXME in a comment. https://reviews.llvm.org/D35406 Files: clangd/ClangdServer.cpp clangd/ClangdUnit.cpp clangd/ClangdUnit.h Index: clangd/ClangdUnit.h

[PATCH] D35405: [index] Added a method indexTopLevelDecls to run indexing on a list of Decls.

ilya-biryukov marked 2 inline comments as done. ilya-biryukov added inline comments. Comment at: lib/Index/IndexingAction.cpp:181 +void index::indexTopLevelDecls(ASTContext , ArrayRef Decls, +std::shared_ptr DataConsumer, +

[PATCH] D35405: [index] Added a method indexTopLevelDecls to run indexing on a list of Decls.

ilya-biryukov updated this revision to Diff 106604. ilya-biryukov added a comment. - Fixed formatting. https://reviews.llvm.org/D35405 Files: include/clang/Index/IndexingAction.h lib/Index/IndexingAction.cpp Index: lib/Index/IndexingAction.cpp

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

ilya-biryukov created this revision. This refactoring does not aim to introduce any significant changes to the behaviour of clangd to keep the change as simple as possible. https://reviews.llvm.org/D35406 Files: clangd/ClangdServer.cpp clangd/ClangdUnit.cpp clangd/ClangdUnit.h Index:

[PATCH] D35405: [index] Added a method indexTopLevelDecls to run indexing on a list of Decls.

ilya-biryukov added a comment. For reference: clangd refactoring https://reviews.llvm.org/D35406 https://reviews.llvm.org/D35405 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35405: [index] Added a method indexTopLevelDecls to run indexing on a list of Decls.

ilya-biryukov created this revision. We need it in clangd for refactoring that replaces ASTUnit with manual AST management. https://reviews.llvm.org/D35405 Files: include/clang/Index/IndexingAction.h lib/Index/IndexingAction.cpp Index: lib/Index/IndexingAction.cpp

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

ilya-biryukov added a comment. Depends on change to cfe (https://reviews.llvm.org/D35405) to compile properly. https://reviews.llvm.org/D35406 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35682: Fixed failing assert in code completion.

ilya-biryukov created this revision. The code was accessing uninstantiated default argument. This resulted in failing assertion at ParamVarDecl::getDefaultArg(). https://reviews.llvm.org/D35682 Files: lib/Sema/SemaCodeComplete.cpp test/CodeCompletion/uninstantiated_params.cpp Index:

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:167 +std::move(VFS)); + CI->getFrontendOpts().DisableFree = false; + return CI; krasimir wrote: > Why `DisableFree`? We rely on CompilerInstance

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

ilya-biryukov updated this revision to Diff 107475. ilya-biryukov added a comment. Added a few comments. https://reviews.llvm.org/D35406 Files: clangd/ClangdServer.cpp clangd/ClangdUnit.cpp clangd/ClangdUnit.h Index: clangd/ClangdUnit.h

[PATCH] D35825: [clangd] Reuse compile commands during reparse

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:248 + VFS->setCurrentWorkingDirectory(Command.Directory); + Also have to call it in `codeComplete`. https://reviews.llvm.org/D35825 ___

[PATCH] D35825: [clangd] Reuse compile commands during reparse

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D35825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

ilya-biryukov added a comment. In https://reviews.llvm.org/D35406#821567, @petarj wrote: > Can you check if this change introduced clang-x86-windows-msvc2015 buildbot > failure > ? It did. Was fixed in r308959

[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

ilya-biryukov added a comment. Here's a patch to fix ClangdTests: https://reviews.llvm.org/D34936 We should probably land it before your patch to avoid ClangdTests failures between those llvm and clang-tools-extra revisions. Repository: rL LLVM https://reviews.llvm.org/D34158

[PATCH] D34936: [clangd] Add -ffreestanding on VFS tests.

ilya-biryukov created this revision. They don't provide proper gcc installations and may fail on implicit include. https://reviews.llvm.org/D34936 Files: unittests/clangd/ClangdTests.cpp Index: unittests/clangd/ClangdTests.cpp

[PATCH] D34936: [clangd] Add -ffreestanding on VFS tests.

ilya-biryukov added a comment. Implicit `stdc-predef.h` include is not yet there, but will appear after https://reviews.llvm.org/D34158 has landed. https://reviews.llvm.org/D34936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34469: Use vfs::FileSystem in ASTUnit when creating CompilerInvocation.

ilya-biryukov added a comment. @bruno, I've added a test to clangd, see D34755 . Repository: rL LLVM https://reviews.llvm.org/D34469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34947: [clangd] Add support for per-file extra flags

ilya-biryukov added inline comments. Comment at: clangd/GlobalCompilationDatabase.cpp:22 +return; + if (Command.CommandLine.empty()) +Command.CommandLine.push_back("clang"); If `Command.CommandLine.empty()` is true, extra flags will be added **before**

[PATCH] D34947: [clangd] Add support for per-file extra flags

ilya-biryukov added a comment. I forgot to submit this last comment yesterday, sorry about that. Comment at: clangd/GlobalCompilationDatabase.h:51 + void addExtraFlagsForFile(PathRef File, std::vector ExtraFlags); + Maybe rename to `setExtraFlagsForFile`?

[PATCH] D34947: [clangd] Add support for per-file extra flags

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D34947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34947: [clangd] Add support for per-file extra flags

ilya-biryukov added inline comments. Comment at: clangd/GlobalCompilationDatabase.cpp:20 + const SmallVectorImpl ) { + assert(Command && !Command->CommandLine.empty()); + if (!Command || ExtraFlags.empty()) Maybe pass `Command` by

[PATCH] D34936: [clangd] Add -ffreestanding on VFS tests.

ilya-biryukov added a comment. In https://reviews.llvm.org/D34936#799589, @mibintc wrote: > Thanks so much! You're welcome. Repository: rL LLVM https://reviews.llvm.org/D34936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34755: [clangd] Added a test, checking that gcc install is searched via VFS.

ilya-biryukov created this revision. https://reviews.llvm.org/D34755 Files: unittests/clangd/ClangdTests.cpp Index: unittests/clangd/ClangdTests.cpp === --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp

[PATCH] D34755: [clangd] Added a test, checking that gcc install is searched via VFS.

ilya-biryukov added a comment. I wonder if it's gonna fail on Windows. Maybe enable it only on Linux? https://reviews.llvm.org/D34755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34755: [clangd] Added a test, checking that gcc install is searched via VFS.

ilya-biryukov added a comment. I wonder if it's gonna fail on Windows. Maybe enable it only on Linux? https://reviews.llvm.org/D34755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34269: [clangd] Add "Go to Declaration" functionality

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D34269 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D36398: [clangd] Check if CompileCommand has changed on forceReparse.

ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. In https://reviews.llvm.org/D36398#834904, @klimek wrote: > Also missing tests :) Done :-) Comment at: clangd/ClangdUnitStore.h:45-48 + struct RecreateResult { +std::shared_ptr

[PATCH] D36398: [clangd] Check if CompileCommand has changed on forceReparse.

ilya-biryukov updated this revision to Diff 110155. ilya-biryukov added a comment. Addressed review comments. - Added a test for forceReparse. - Got rid of a redundant `= nullptr` assignment. https://reviews.llvm.org/D36398 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

ilya-biryukov added a comment. In https://reviews.llvm.org/D36261#834902, @klimek wrote: > High-level question: Why can't we use llvm::ThreadPool? It is not an in-place replacement as it does not allow to prioritize new tasks over old ones (new tasks are usually more important for clangd as

[PATCH] D36133: [clangd] Rewrote AST and Preamble management.

ilya-biryukov updated this revision to Diff 109124. ilya-biryukov added a comment. Addressed review comments. - Moved implementations of template function to header. - Fixed a typo. https://reviews.llvm.org/D36133 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h

[PATCH] D36133: [clangd] Rewrote AST and Preamble management.

ilya-biryukov marked 3 inline comments as done. ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.h:113 + /// queue. The request will be run on a separate thread. + template void addToFront(Func &, Args &&... As); + /// Add a new request to run function

[PATCH] D36133: [clangd] Rewrote AST and Preamble management.

ilya-biryukov updated this revision to Diff 109091. ilya-biryukov added a comment. - Fixed a bug that caused CppFiles to be deleted while used. https://reviews.llvm.org/D36133 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h

[PATCH] D36133: [clangd] Rewrote AST and Preamble management.

ilya-biryukov added a comment. I also want to rename ClangdUnit and ClangdUnitStore accordingly, but will do that in a separate commit so that git-svn correctly detects the renames (i.e. don't want file contents changes). https://reviews.llvm.org/D36133

[PATCH] D36155: Use VFS operations in FileManager::makeAbsolutePath.

ilya-biryukov created this revision. It used to call into llvm::sys::fs::make_absolute. https://reviews.llvm.org/D36155 Files: lib/Basic/FileManager.cpp unittests/Basic/FileManagerTest.cpp Index: unittests/Basic/FileManagerTest.cpp

[PATCH] D36150: [clangd] LSP extension to switch between source/header file

ilya-biryukov requested changes to this revision. ilya-biryukov added a reviewer: ilya-biryukov. ilya-biryukov added a comment. Also, +1 to the comments about file extensions, we have to cover at least `.c`, `.cc` and `.cpp` for source files, `.h` and `.hpp` for header files.

[PATCH] D35894: [clangd] Code hover for Clangd

ilya-biryukov added a comment. > I think all of those would be great. Our objective is to bring basic but > correct features that will put us close to parity with Eclipse CDT, so that > our users can transition. In CDT only the "raw" source is shown, similar to > this patch (except in CDT it

[PATCH] D36226: Added a RealFileSystem implementation that does not rely on global CWD.

ilya-biryukov created this revision. Allows to have multiple instances of RealFileSystem that have different working directories. https://reviews.llvm.org/D36226 Files: include/clang/Basic/VirtualFileSystem.h lib/Basic/VirtualFileSystem.cpp unittests/Basic/VirtualFileSystemTest.cpp

[PATCH] D36226: Added a RealFileSystem implementation that does not rely on global CWD.

ilya-biryukov added a comment. Also tested by temporarily replacing `getRealFileSystem()` body with `return createThreadFriendlyRealFS();` and running `check-all`, all tests passed. To be more specific, there was one crash in clang-format, as there is a piece of code that uses raw pointer from

[PATCH] D36529: Fixed a race condition in PrecompiledPreamble.

ilya-biryukov created this revision. Two PrecompiledPreambles, used in parallel on separate threads, could be writing preamble to the same temporary file. https://reviews.llvm.org/D36529 Files: lib/Frontend/PrecompiledPreamble.cpp Index: lib/Frontend/PrecompiledPreamble.cpp

[PATCH] D36529: Fixed a race condition in PrecompiledPreamble.

ilya-biryukov added a comment. Catched by a test from https://reviews.llvm.org/D36261. https://reviews.llvm.org/D36529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

ilya-biryukov added a comment. Added a test that exercises some multi-threading behaviour. It really finds bugs, at least it found a problem that was fixed by https://reviews.llvm.org/D36397. (Will make sure to submit that other change before this one). As discussed with @klimek, will move to

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

ilya-biryukov updated this revision to Diff 110381. ilya-biryukov added a comment. - Added a stress test for multithreading. https://reviews.llvm.org/D36261 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h

[PATCH] D36397: [clangd] Fixed a data race.

ilya-biryukov added a comment. In https://reviews.llvm.org/D36397#833912, @klimek wrote: > Yea, we'll probably want to add a "smash it hard with parallel" requests kind > of test to catch these things. You're right, there is probably not a better > solution for this specific bug. Added a

[PATCH] D36397: [clangd] Fixed a data race.

ilya-biryukov created this revision. Calling addDocument after removeDocument could have resulted in an invalid program state (AST and Preamble for the valid document could have been incorrectly removed). This commit also includes an improved CppFile::cancelRebuild implementation that allows to

[PATCH] D36398: [clangd] Check if CompileCommand has changed on forceReparse.

ilya-biryukov created this revision. https://reviews.llvm.org/D36398 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnitStore.cpp clangd/ClangdUnitStore.h Index: clangd/ClangdUnitStore.h === ---

[PATCH] D36226: Added a RealFileSystem implementation that does not rely on global CWD.

ilya-biryukov updated this revision to Diff 109524. ilya-biryukov added a comment. Added a very rough prototype of vfs::RealFileSystem not using openat. Not ready for submission yet, wanted to start discussion about Windows implementation. https://reviews.llvm.org/D36226 Files:

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.h:121-122 +public: + /// Returns the number of threads to use when shouldRunsynchronously() is + /// false. Must not be called if shouldRunsynchronously() is true. + unsigned getThreadsCount();

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

ilya-biryukov updated this revision to Diff 109527. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. Addressed review comments. - Fixed typos. - Renamed SchedulingParams to SchedulingOptions. https://reviews.llvm.org/D36261 Files: clangd/ClangdLSPServer.cpp

[PATCH] D36398: [clangd] Check if CompileCommand has changed on forceReparse.

ilya-biryukov updated this revision to Diff 110016. ilya-biryukov added a comment. - Moved assignment `RemoveFile = nullptr` around a bit. - Added a comment to recreateFileIfCompileCommandChanged. https://reviews.llvm.org/D36398 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h

[PATCH] D36398: [clangd] Check if CompileCommand has changed on forceReparse.

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnitStore.cpp:45 + .first; +Result.RemovedFile = nullptr; + } else if (!compileCommandsAreEqual(It->second->getCompileCommand(), klimek wrote: > Just say RemovedFile = nullptr in the

[PATCH] D36397: [clangd] Fixed a data race.

ilya-biryukov added a comment. In https://reviews.llvm.org/D36397#833883, @klimek wrote: > Tests? TSAN does not catch this (as it's a logical error) and it requires a rather bizarre timing of file reparses to trigger. I couldn't come up with an example that would reproduce this.

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.h:121-122 +public: + /// Returns the number of threads to use when shouldRunsynchronously() is + /// false. Must not be called if shouldRunsynchronously() is true. + unsigned getThreadsCount();

[PATCH] D36458: Fix crash when current lexer is nullptr

ilya-biryukov added inline comments. Comment at: lib/Parse/Parser.cpp:519 ConsumeToken(); - - PP.replayPreambleConditionalStack(); + if (!PP.isCurrentLexer(nullptr)) +PP.replayPreambleConditionalStack(); ilya-biryukov wrote: > This certainly fixes the

[PATCH] D36458: Fix crash when current lexer is nullptr

ilya-biryukov added a comment. I've also stumbled upon this crash. Here is an even simpler repro that crashes for me (does not involve includes): #ifndef HEADER_GUARD #define FOO(X) int fhjdfhsdjkfhjkshfjk; FOO(base) struct X { int a; }; int test() { X v; v. }

[PATCH] D36872: Fixed a crash on replaying Preamble's PP conditional stack.

ilya-biryukov created this revision. The crash occurs when the first token after a preamble is a macro expansion. Fixed by moving replayPreambleConditionalStack from Parser into Preprocessor. It is now called right after the predefines file is processed. https://reviews.llvm.org/D36872 Files:

[PATCH] D36458: Fix crash when current lexer is nullptr

ilya-biryukov added a comment. Here's an alternative fix that moves the `replay...` call into `Preprocessor`: https://reviews.llvm.org/D36872. It also fixes the crash and doesn't add invalid compiler errors about non-matching `#ifdefs`. https://reviews.llvm.org/D36458

[PATCH] D36872: Fixed a crash on replaying Preamble's PP conditional stack.

ilya-biryukov updated this revision to Diff 111676. ilya-biryukov added a comment. - Removed redundant 'private' specifier. - Added a test for https://bugs.llvm.org/show_bug.cgi?id=33574 https://reviews.llvm.org/D36872 Files: include/clang/Lex/Preprocessor.h lib/Lex/PPLexerChange.cpp

[PATCH] D36872: Fixed a crash on replaying Preamble's PP conditional stack.

ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. In https://reviews.llvm.org/D36872#845364, @erikjv wrote: > Can you check if the example in https://bugs.llvm.org/show_bug.cgi?id=33574 > works correctly? Works correctly, added an extra test for it.

[PATCH] D33047: [ClangD] Refactor clangd into separate components

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.h:23 + +class ClangdLSPServer { + class LSPDiagnosticsConsumer; klimek wrote: > ilya-biryukov wrote: > > klimek wrote: > > > I'd have expected something that's called LSP server to work on

[PATCH] D33047: [ClangD] Refactor clangd into separate components

ilya-biryukov updated this revision to Diff 98745. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. Addressed new comments https://reviews.llvm.org/D33047 Files: clangd/ASTManager.cpp clangd/ASTManager.h clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp

[PATCH] D33047: [ClangD] Refactor clangd into separate components

ilya-biryukov added inline comments. Comment at: clangd/ClangdMain.cpp:41 // dispatching. - DocumentStore Store; - ASTManager AST(Out, Store, RunSynchronously); - Store.addListener(); + ClangdLSPServer AST(Out, RunSynchronously); JSONRPCDispatcher

[PATCH] D33047: [ClangD] Refactor clangd into separate components

ilya-biryukov updated this revision to Diff 98634. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. Addressed style comments. Moved ClangdScheduler to be the last field of the ClangdServer, so that it's constructed last and destructed first.

[PATCH] D33047: [ClangD] Refactor clangd into separate components

ilya-biryukov updated this revision to Diff 98606. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. Addressed review comments https://reviews.llvm.org/D33047 Files: clangd/ASTManager.cpp clangd/ASTManager.h clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp

[PATCH] D33047: [ClangD] Refactor clangd into separate components

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.h:23 + +class ClangdLSPServer { + class LSPDiagnosticsConsumer; klimek wrote: > I'd have expected something that's called LSP server to work on the LSP > protocol level (that is, have a

[PATCH] D33201: [ClangD] Refactor ProtocolHandlers to decouple them from ClangdLSPServer

ilya-biryukov updated this revision to Diff 99129. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. Addressed review comments https://reviews.llvm.org/D33201 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdMain.cpp

[PATCH] D33201: [clangd] Refactor ProtocolHandlers to decouple them from ClangdLSPServer

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.cpp:20 +template +std::string replacementsToEdits(StringRef Code, const T ) { + // Turn the replacements into the format specified by the Language Server krasimir wrote: > Hm, this is a

[PATCH] D33233: Restored r303067 and fixed failing test.

ilya-biryukov created this revision. Herald added a subscriber: mgorny. This commit restores r303067(reverted by r303094) and fixes the 'formatting.test' failure. The failure is due to destructors of `ClangdLSPServer`'s fields(`FixItsMap` and `FixItsMutex`) being called before destructor of

[PATCH] D33201: [ClangD] Refactor ProtocolHandlers to decouple them from ClangdLSPServer

ilya-biryukov updated this revision to Diff 99009. ilya-biryukov added a comment. Fixed a typo in a comment https://reviews.llvm.org/D33201 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdMain.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h

[PATCH] D34106: [clangd] Use 'std::string' for VFSTag instead of 'int'

ilya-biryukov created this revision. https://reviews.llvm.org/D34106 Files: clangd/ClangdServer.h unittests/clangd/ClangdTests.cpp Index: unittests/clangd/ClangdTests.cpp === --- unittests/clangd/ClangdTests.cpp +++

[PATCH] D34107: [clangd] Allow to override contents of the file during completion.

ilya-biryukov created this revision. https://reviews.llvm.org/D34107 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h unittests/clangd/ClangdTests.cpp Index: unittests/clangd/ClangdTests.cpp === ---

[PATCH] D34287: Moved code hanlding precompiled preamble out of the ASTUnit.

ilya-biryukov updated this revision to Diff 103203. ilya-biryukov added a comment. Removed PossiblyOwnedBuffer, added an extra copy instead. This makes the code much simpler. https://reviews.llvm.org/D34287 Files: include/clang/Frontend/ASTUnit.h

[PATCH] D34287: Moved code hanlding precompiled preamble out of the ASTUnit.

ilya-biryukov updated this revision to Diff 103053. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Added default initializers to PreambleFileHash. - Update file comments to address klimek's questions. https://reviews.llvm.org/D34287 Files:

  1   2   3   4   5   6   7   8   9   10   >