[PATCH] D139107: [clangd] Allow to build Clangd without decision forest

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc9b325088d14: [clangd] Allow to build Clangd without decision forest (authored by ilya-biryukov). Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D139107: [clangd] Allow to build Clangd without decision forest

ilya-biryukov added a comment. I have sent a follow-up NFC change to fix a typo in the documentation of `--ranking-model=heuristics`. Note that I decided to avoid changing the order of options for `--ranking-model` in the documentation, this didn't seem important. Repository: rG LLVM Github

[PATCH] D139541: Fix parameter name in Sema::addInitCapture to ByRef.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. BTW, feel free to send small changes like these without review. The common jargon for them in LLVM is NFC (non-functional change) Repository: rG LLVM Github Monorepo CH

[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values

ilya-biryukov added inline comments. Comment at: clang/include/clang/AST/ExprCXX.h:4824 + const Expr *getArrayFiller() const { +return const_cast(this)->getArrayFiller(); + } NIT: maybe just use `return ArrayFiller`? The code is much simpler and since func

[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values

ilya-biryukov added a comment. This LG, but not accepting as I believe libc++ is still broken if built with modules, right? I have run `check-cxx` locally and it seems to work, but I suspect that's not using modules by default. I have clicked through the links in phrabricator and the errors see

[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values

ilya-biryukov added a comment. Since the errors only shows up in modular builds, I would look closely for bugs inside `ASTReader`/`ASTWriter`. Also, it seems that `ArrayFiller` is not taken in to account in `computeDependence` and maybe it should be. This assumption is wrong if `ArrayFiller` i

[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values

ilya-biryukov accepted this revision as: ilya-biryukov. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM! Thanks a lot for driving this to conclusion! Please note that there is one small NIT you may want to fix before landing this. UB definitely explains why we

[PATCH] D140044: [CodeComplete] Complete members of dependent `auto` variables

ilya-biryukov added inline comments. Comment at: clang/test/CodeCompletion/member-access.cpp:121 // CHECK-DEP-CHAIN: baseTemplateField : [#T#]baseTemplateField -// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:111:41 %s -o - | FileCheck -check-prefix=CHECK-DEP-CHAIN %s +

[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 clangd/Prot

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

[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 i

[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 emu

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

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:276 +class DeclarationLocationsFinder : public index::IndexDataConsumer { + std::unique_ptr> DeclarationLocations; + const SourceLocation &SearchedLocation; malaperle-ericsson wrote: > malap

[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 http://lists.llvm.org/cgi-bin/mailman/

[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] 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 http://lists.llvm.org/c

[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] 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 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 http://lists

[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 inline comments. Comment at: clangd/GlobalCompilationDatabase.cpp:20 + const SmallVectorImpl &ExtraFlags) { + assert(Command && !Command->CommandLine.empty()); + if (!Command || ExtraFlags.empty()) Maybe pass `Comman

[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`? Th

[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 http://lists.llvm.org/cgi-bin/mailman/

[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 http://list

[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 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: cla

[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] 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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[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] 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 &Ctx, ArrayRef Decls, +std::shared_ptr DataConsumer, +Index

[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] 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 easy

[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 the

[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 t

[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] 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: test/

[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 ___ cfe-

[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 http://lists.llvm.org/cgi-bin/mailman/

[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] D41990: [Frontend] Allow to use PrecompiledPreamble without calling CanReuse

ilya-biryukov created this revision. ilya-biryukov added reviewers: bkramer, sammccall. The new method 'OverridePreamble' allows to override the preamble of any source file without checking if preamble bounds or dependencies were changed. This is used for completion in clangd. Repository: rC

[PATCH] D41991: [clangd] Always use preamble (even stale) for code completion

ilya-biryukov created this revision. ilya-biryukov added reviewers: bkramer, sammccall. Herald added a subscriber: klimek. This improves performance of code completion, because we avoid stating the files from the preamble and never attempt to parse the files without using the preamble if it's prov

[PATCH] D41989: [CodeComplete] Add an option to omit results from the preamble.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM modulo the comment about variable name. Comment at: lib/Sema/SemaLookup.cpp:3502 + bool IncludeDependentBases, +

[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, sorry for the delay. https://reviews.llvm.org/D20124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D41990: [Frontend] Allow to use PrecompiledPreamble without calling CanReuse

ilya-biryukov updated this revision to Diff 129815. ilya-biryukov added a comment. - Simplify comments Repository: rC Clang https://reviews.llvm.org/D41990 Files: include/clang/Frontend/PrecompiledPreamble.h lib/Frontend/PrecompiledPreamble.cpp Index: lib/Frontend/PrecompiledPreamble.cp

[PATCH] D41990: [Frontend] Allow to use PrecompiledPreamble without calling CanReuse

ilya-biryukov updated this revision to Diff 129817. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Remove redundant assertion message Repository: rC Clang https://reviews.llvm.org/D41990 Files: include/clang/Frontend/PrecompiledPreamble.h lib/Frontend/Pr

[PATCH] D41990: [Frontend] Allow to use PrecompiledPreamble without calling CanReuse

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: include/clang/Frontend/PrecompiledPreamble.h:107 /// is accessible. - /// For in-memory preambles, PrecompiledPreamble instance continues to own - /// the MemoryBuffer with the P

[PATCH] D42071: [Sema] Add a callback in VisibleDeclConsumer allowing client to know which DeclContext is going to visit.

ilya-biryukov added inline comments. Comment at: include/clang/Sema/Lookup.h:791 + /// \param Ctx the context which Sema begins to visit. + virtual void BeginVisitContext(DeclContext *Ctx) {}; }; Maybe rename it to `VisitedContext` ? Seems more in-line with `F

[PATCH] D42073: [clangd] Query all visible scopes based on all visible using-namespace declarationsand containing namespace for global qualified code completion.

ilya-biryukov added a comment. Discussed this offline. If we move most to collect visited `DeclContext`s to `CodeCompletionDeclConsumer` (from `CodeComplete.cpp`) and provide a list of `DeclContext`s to `CodeCompletionConsumer`, we will be able to get rid of running lookup manually in clangd.

[PATCH] D42074: [clangd] Collect enum constants in SymbolCollector

ilya-biryukov added inline comments. Comment at: clangd/index/SymbolCollector.cpp:75 + // Skip nameless declarations. + if (ND->getDeclName().isEmpty()) +return true; What are those declarations exactly? Comment at: unittests/clangd/Symbo

[PATCH] D42063: [clangd] Avoid combinatorial explosion in CodeCompleteTests.

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 https://reviews.llvm.org/D42063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D42077: Ensure code complete with !LoadExternal sees all local decls.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM with a few NITs (see the comments) Comment at: include/clang/AST/DeclBase.h:1826 lookups_range lookups() const; - lookups_range noload_lookups() const;

[PATCH] D42164: [clangd] Don't crash on LSP calls for non-added files

ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, ioeric, hokein. Herald added a subscriber: klimek. We will return errors for non-added files for now. Another alternative for clangd would be to read non-added files from disk and provide useful features anyway. There

[PATCH] D42164: [clangd] Don't crash on LSP calls for non-added files

ilya-biryukov updated this revision to Diff 130127. ilya-biryukov added a comment. - Do the check in rename before calling into ClangdServer Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42164 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.

[PATCH] D42164: [clangd] Don't crash on LSP calls for non-added files

ilya-biryukov updated this revision to Diff 130125. ilya-biryukov added a comment. - Renamed crash.test to crash-non-added-files.test Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42164 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h tes

[PATCH] D42164: [clangd] Don't crash on LSP calls for non-added files

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.cpp:153 + + auto Code = Server.getDocument(File); + if (!Code) sammccall wrote: > don't you want to do this before computing replacements? Makes sense. Comment at: clangd

[PATCH] D42071: [Sema] Add visited contexts to CodeCompleteContext

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Look at the comments for a few NITs. Comment at: include/clang/Sema/CodeCompleteConsumer.h:271 + using VisitedContextSet = llvm::SmallPtrSet; + ---

[PATCH] D42173: [clangd] Simplify code handling compile commands

ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, bkramer. Herald added a subscriber: klimek. CppFile can now change compilation arguments during rebuild. This allows simplifying code that manages CppFiles. Repository: rCTE Clang Tools Extra https://reviews.llvm.o

[PATCH] D42174: [clangd] Refactored threading in ClangdServer

ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, bkramer. Herald added a subscriber: klimek. We now provide an abstraction of Scheduler that abstracts threading and resource management in ClangdServer. No changes to behavior are intended with an exception of changed e

[PATCH] D42177: DO NOT SUBMIT. Combined diff of D42174 and D42173.

ilya-biryukov created this revision. Herald added subscribers: cfe-commits, klimek. To simplify review of both changes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42177 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h

[PATCH] D42174: [clangd] Refactored threading in ClangdServer

ilya-biryukov added a comment. Here's also a combined diff from both this and child revision: https://reviews.llvm.org/D42177 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42174 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D41947: Provide default virtual filesystem argument to ClangTool constructor

ilya-biryukov added a comment. LGTM, but could we add a test that checks the VFS is actually used? Repository: rC Clang https://reviews.llvm.org/D41947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D41852: [clang-tidy] Don't generate fix for argument constructed from std::initializer_list.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM modulo a change to the 'initializer_list.h'. Do we really need it for this patch? Comment at: test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.

[PATCH] D41991: [clangd] Always use preamble (even stale) for code completion

ilya-biryukov updated this revision to Diff 130193. ilya-biryukov added a comment. - Rewrote the comment for prepareCompilerInstance - Call CanReuse() and discard its results. We have clients that rely on having the stat() calls for files from preamble Repository: rCTE Clang Tools Extra http

[PATCH] D41991: [clangd] Always use preamble (even stale) for code completion

ilya-biryukov updated this revision to Diff 130194. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Use a better comment suggest by Sam Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41991 Files: clangd/CodeComplete.cpp clangd/Compiler.cpp

[PATCH] D41991: [clangd] Always use preamble (even stale) for code completion

ilya-biryukov added inline comments. Comment at: clangd/CodeComplete.cpp:522 - // Attempt to reuse the PCH from precompiled preamble, if it was built. - if (Preamble) { -auto Bounds = -ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0); -if (!P

[PATCH] D41991: [clangd] Always use preamble (even stale) for code completion

ilya-biryukov updated this revision to Diff 130195. ilya-biryukov added a comment. - Fixed a typo Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41991 Files: clangd/CodeComplete.cpp clangd/Compiler.cpp clangd/Compiler.h Index: clangd/Compiler.h =

[PATCH] D41535: Add -vfsoverlay option for Clang-Tidy

ilya-biryukov requested changes to this revision. ilya-biryukov added a comment. This revision now requires changes to proceed. Sorry for the delay Comment at: clang-tidy/ClangTidy.cpp:93 +vfs::OverlayFileSystem &OverlayFS) { + if (OverlayFile.empty

[PATCH] D42174: [clangd] Refactored threading in ClangdServer

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.h:107 +/// A simple fixed-size thread pool implementation. +class SimpleThreadPool { public: bkramer wrote: > What's so simple about it? Why not `clangd::ThreadPool`? > > Also there's `llvm::T

[PATCH] D41991: [clangd] Always use preamble (even stale) for code completion

ilya-biryukov updated this revision to Diff 130206. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - s/buffer for MainFile/MainFile/ Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41991 Files: clangd/CodeComplete.cpp clangd/Compiler.cpp cla

[PATCH] D42241: [CodeComplete] Fix completion in the middle of idents in macro calls

ilya-biryukov created this revision. ilya-biryukov added reviewers: bkramer, arphaman. This patch removes IdentifierInfo from completion token after remembering the identifier in the preprocessor. Prior to this patch, completion token had the IdentifierInfo set to null when completing at the star

[PATCH] D41947: Provide default virtual filesystem argument to ClangTool constructor

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Looks good. Do you have commit access or do you need someone to land this patch for you? Repository: rC Clang https://reviews.llvm.org/D41947 _

[PATCH] D41852: [clang-tidy] Don't generate fix for argument constructed from std::initializer_list.

ilya-biryukov added inline comments. Comment at: test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h:30 vector(initializer_list<_E> init); + ~vector(); }; hokein wrote: > ilya-biryukov wrote: > > Why do we need to add this destructor in this patch?

[PATCH] D41535: Add -vfsoverlay option for Clang-Tidy

ilya-biryukov requested changes to this revision. ilya-biryukov added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/tool/ClangTidyMain.cpp:345 + if (!Buffer) { +llvm::errs() << diag::err_missing_vfs_overlay_file << OverlayFile; +r

[PATCH] D42074: [clangd] Collect enum constants in SymbolCollector

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. (See the comment about changing the comment, though) Comment at: clangd/index/SymbolCollector.cpp:75 + // Skip nameless declarations. + if (ND->getDeclN

[PATCH] D42073: [clangd] Query all visible scopes based on all visible using-namespace declarationsand containing namespace for global qualified code completion.

ilya-biryukov added inline comments. Comment at: clangd/CodeComplete.cpp:270 + /// namespace scopes which are visible to the qualified-id completion token. + std::vector Scopes; +}; sammccall wrote: > Just to check, if the user types: > "vec" --> None > "::vec"

[PATCH] D41947: Provide default virtual filesystem argument to ClangTool constructor

ilya-biryukov added a comment. In https://reviews.llvm.org/D41947#980305, @vladimir.plyashkun wrote: > In https://reviews.llvm.org/D41947#980298, @ilya-biryukov wrote: > > > Looks good. Do you have commit access or do you need someone to land this > > patch for you? > > > No, i don't have commit

[PATCH] D42173: [clangd] Simplify code handling compile commands

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:35 +tooling::CompileCommand getCompileCommand(GlobalCompilationDatabase &CDB, + PathRef File, PathRef ResourceDir) { sammccall wrote: > This seems

[PATCH] D42173: [clangd] Simplify code handling compile commands

ilya-biryukov updated this revision to Diff 130428. ilya-biryukov marked 7 inline comments as done. ilya-biryukov added a comment. Herald added a subscriber: ioeric. Addressing review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42173 Files: clangd/ClangdServer.cpp

[PATCH] D41594: Support `ivfsoverlay` option in Tooling

ilya-biryukov added a comment. With https://reviews.llvm.org/D41947 in place, do we still need this change? Or can it be "abandoned" now? Repository: rC Clang https://reviews.llvm.org/D41594 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D41947: Provide default virtual filesystem argument to ClangTool constructor

ilya-biryukov added inline comments. Comment at: include/clang/Tooling/Tooling.h:299 /// clang modules. + /// \param BaseFS Base virtual filesystem used for OverlayFileSystem creation ClangTool(const CompilationDatabase &Compilations, NIT: LLVM coding styl

[PATCH] D41852: [clang-tidy] Don't generate fix for argument constructed from std::initializer_list.

ilya-biryukov added inline comments. Comment at: test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h:30 vector(initializer_list<_E> init); + ~vector(); }; hokein wrote: > ilya-biryukov wrote: > > hokein wrote: > > > ilya-biryukov wrote: > > > > Why

[PATCH] D42073: [clangd] Query all visible scopes based on all visible using-namespace declarationsand containing namespace for global qualified code completion.

ilya-biryukov added inline comments. Comment at: clangd/CodeComplete.cpp:270 + /// namespace scopes which are visible to the qualified-id completion token. + std::vector Scopes; +}; sammccall wrote: > ilya-biryukov wrote: > > sammccall wrote: > > > Just to chec

[PATCH] D41535: Add -vfsoverlay option for Clang-Tidy

ilya-biryukov added a comment. Just a few more NITs and we're good to go Comment at: clang-tidy/tool/ClangTidyMain.cpp:347 +llvm::errs() << "Error: virtual filesystem overlay file '" << OverlayFile + << "' not found.\n"; +return nullptr;

[PATCH] D41535: Add -vfsoverlay option for Clang-Tidy

ilya-biryukov added a comment. This looks good, but we should add a test. Should've noticed that before, sorry for the slowing this down a bit more. After the test is there, I'm happy to commit this change for you. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41535 ___

[PATCH] D42073: [clangd] Use accessible scopes to query indexes for global code completion.

ilya-biryukov added inline comments. Comment at: clangd/CodeComplete.cpp:664 + if (SS.isValid()) { // Resolved qualifier. +// FIXME: Disable Sema typo correction dureing code completion. +// The resolved qualifier might not perfectly match the written qualifier.

[PATCH] D41535: Add -vfsoverlay option for Clang-Tidy

ilya-biryukov added a comment. In https://reviews.llvm.org/D41535#983435, @vladimir.plyashkun wrote: > IIUC, it will be little bit difficult to test it, because whole logic placed > in the `ClangTidyMain`. > All existing clang-tidy unit tests use direct calls of `ToolInvocation` > which is do

[PATCH] D42363: [clang-tidy] Don't generate fixes for invalid new expr location in modernize-make-unique.

ilya-biryukov added a comment. Should we fix the endloc bug instead? Or is that too hard? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42363 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[PATCH] D41535: Add -vfsoverlay option for Clang-Tidy

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Thanks for adding the test! LGTM, will submit it shortly. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41535 ___

[PATCH] D42363: [clang-tidy] Don't generate fixes for invalid new expr location in modernize-make-unique.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D42363#983769, @hokein wrote: > Yeah, we should fix the clang bug (root cause). But I think this patch still > makes sense -- as we usually ignore inv

[PATCH] D42173: [clangd] Simplify code handling compile commands

ilya-biryukov updated this revision to Diff 131062. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Unwrap the comment - Remove ParseInputs constructor - Document the CppFile invariant Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42173 Files:

[PATCH] D42173: [clangd] Simplify code handling compile commands

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.h:62 +/// Information required to run clang (e.g., to parse AST or do code +/// completion). +struct ParseInputs { sammccall wrote: > sammccall wrot

[PATCH] D42429: [clangd] Moved caching of compile commands to ClangdServer

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: ioeric, jkorous-apple, klimek. It allows to get rid of CppFile::getLastCommand and simplify the code in the upcoming threading patch. Repository: rCTE Clang Tools Extra https://reviews.ll

[PATCH] D42174: [clangd] Refactored threading in ClangdServer

ilya-biryukov updated this revision to Diff 131105. ilya-biryukov marked 4 inline comments as done. ilya-biryukov added a comment. - Renamed SimpleThreadPool to ThreadPool - Removed PCHs from Scheduler's constructor - Renamed waitFor(AST|Preamble)Action to blocking(AST|Preamble)Read - Updates - Re

[PATCH] D42174: [clangd] Refactored threading in ClangdServer

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:222 + } + // We currently do all the reads of the AST on a running thread to avoid + // inconsistent states coming from subsequent updates. sammccall wrote: > Having trouble with this one

[PATCH] D42174: [clangd] Refactored threading in ClangdServer

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.h:164 +/// Handles running tasks for ClangdServer and managing the resources (e.g., +/// preambles and ASTs) for opened files. sammccall wrote: > This is a nice abstraction, so much better tha

[PATCH] D41454: [clangd] Add ClangdUnit diagnostics tests using annotated code.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Herald added subscribers: hintonda, ioeric, jkorous-apple. LGTM. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41454

[PATCH] D42428: [CodeComplete] only respect LoadExternal hint at namespace/tu scope

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

[PATCH] D39571: [clangd] DidChangeConfiguration Notification

ilya-biryukov added a comment. Hi @simark , thanks for picking up this change. Comment at: clangd/ClangdLSPServer.cpp:302 +// FIXME: This function needs to be properly tested. +void ClangdLSPServer::onChangeConfiguration( Are you planning to to address this F

<    27   28   29   30   31   32   33   >