[PATCH] D51297: [docs] Create a guide for Vim users on how to set up Clangd

ioeric added inline comments. Comment at: clang-tools-extra/docs/clangd.rst:114 + +There are multiple editors and plugins that are known to work with Clangd, such +as :program:`VSCode` and ``vscode-clangd`` extension, :program:`Emacs` and Thanks! I would be

[PATCH] D51297: [docs] Create a guide for Vim users on how to set up Clangd

ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/docs/clangd.rst:114 + +There are multiple editors and plugins that are known to work with Clangd, such +as :program:`VSCode` and

[PATCH] D51297: [docs] Create a guide for Vim users on how to set up Clangd

ioeric added inline comments. Comment at: clang-tools-extra/docs/clangd.rst:111 +Editor Integration +== While we are here, would you mind including some other editors/plugins that are known to work with clangd (just so that people don't think

[PATCH] D51987: [clangd] Rename global-symbol-builder to clangd-symbol-builder.

ioeric accepted this revision. ioeric added a comment. In https://reviews.llvm.org/D51987#1232021, @sammccall wrote: > In https://reviews.llvm.org/D51987#1231993, @ioeric wrote: > > > In https://reviews.llvm.org/D51987#1231990, @ilya-biryukov wrote: > > > > > > You beat it to me, but I thought

[PATCH] D51987: [clangd] Rename global-symbol-builder to clangd-symbol-builder.

ioeric added a comment. In https://reviews.llvm.org/D51987#1231990, @ilya-biryukov wrote: > > You beat it to me, but I thought we didn't use indexer as it could be > > confused with the actual indexing? > > Sorry. Not a big deal, can revert back if needed. > Mostly like indexer since it's

[PATCH] D51987: [clangd] Rename global-symbol-builder to clangd-symbol-builder.

ioeric added a comment. In https://reviews.llvm.org/D51987#1231971, @ilya-biryukov wrote: > `clangd-indexer` looks nice and short. @ioeric, WDYT? You beat it to me, but I thought we didn't use `indexer` as it could be confused with the actual `indexing`? Repository: rCTE Clang Tools Extra

[PATCH] D51987: [clangd] Rename global-symbol-builder to clangd-symbol-builder.

ioeric added a subscriber: ilya-biryukov. ioeric added a comment. I mean `clangd-symbol-builder` Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51987 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51987: [clangd] Rename global-symbol-builder to clangd-symbol-builder.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm +1 to `clang-symbol-builder` Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51987 ___ cfe-commits mailing list

[PATCH] D51977: [clangd] Clarify and hide -index flag.

ioeric created this revision. ioeric added reviewers: sammccall, ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay. The wording implies global index support, which is confusing. As most users shouldn't care about this flag, also make it hidden to avoid

[PATCH] D51860: [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/Index.h:440 /// return more than this, e.g. if it doesn't know which candidates are best. - size_t MaxCandidateCount = std::numeric_limits::max(); + uint32_t MaxCandidateCount =

[PATCH] D51860: [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/index/Index.h:440 /// return more than this, e.g. if it doesn't know which candidates are best. - size_t MaxCandidateCount =

[PATCH] D51864: [Tooling] Restore working dir in ClangTool.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lg Repository: rC Clang https://reviews.llvm.org/D51864 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51860: [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/Index.cpp:186 + O.map("ProximityPaths", Request.ProximityPaths); + if (OK) +Request.MaxCandidateCount = MaxCandidateCount; I think we should not set `Request.MaxCandidateCount` if

[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd

ioeric added a comment. In https://reviews.llvm.org/D51297#1228441, @kbobyrev wrote: > In https://reviews.llvm.org/D51297#1225546, @ilya-biryukov wrote: > > > I would stamp this from my side, but concerns whether we should recommend > > YCM's LSP-based completer instead are probably still

[PATCH] D51539: [clangd] Add symbol slab size to index memory consumption estimates

ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/index/Index.h:512 llvm::function_ref) const override; + void lookup(const LookupRequest &,

[PATCH] D51747: [clangd] Implement deprecation diagnostics with lower severity.

ioeric added a comment. In https://reviews.llvm.org/D51747#1227089, @ilya-biryukov wrote: > Not sure if it's fine to hijack our own diagnostic-specific flags in to clang > command args. > > Cons that I see: > > 1. There is no way for the users to turn them off if they find them > non-useful.

[PATCH] D51539: [clangd] Add symbol slab size to index memory consumption estimates

ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:144 +size_t FileIndex::estimateMemoryUsage() const { + return FSymbols.estimateMemoryUsage(); +} sammccall wrote: > ioeric wrote: > > This can be a bit tricky. Generally, the

[PATCH] D51747: [clangd] Implement deprecation diagnostics with lower severity.

ioeric added inline comments. Comment at: clangd/Diagnostics.cpp:299 +D.Severity = +D.Category == "Deprecations" ? DiagnosticsEngine::Note : DiagLevel; return D; kadircet wrote: > Couldn't find a better way to check for this one. Category types

[PATCH] D51539: [clangd] Add symbol slab size to index memory consumption estimates

ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:144 +size_t FileIndex::estimateMemoryUsage() const { + return FSymbols.estimateMemoryUsage(); +} This can be a bit tricky. Generally, the size of a `FileIndex` is the size of

[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE341576: [clangd] Add Deprecated field to Symbol and CodeCompletion. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D51724?vs=164259=164260#toc Repository:

[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

ioeric updated this revision to Diff 164259. ioeric added a comment. - merged with origin/master Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51724 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/Protocol.cpp clangd/Protocol.h clangd/Quality.cpp

[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

ioeric updated this revision to Diff 164258. ioeric marked 5 inline comments as done. ioeric added a comment. - addressed review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51724 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/Protocol.cpp

[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

ioeric updated this revision to Diff 164205. ioeric marked an inline comment as done. ioeric added a comment. - Pack flags in Symbol. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51724 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/Protocol.cpp

[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

ioeric added inline comments. Comment at: clangd/Protocol.cpp:520 Result["additionalTextEdits"] = json::Array(CI.additionalTextEdits); + if (CI.deprecated) +Result["deprecated"] = CI.deprecated; sammccall wrote: > do we actually want this in JSON? >

[PATCH] D51688: [clangd] Set SymbolID for sema macros so that they can be merged with index macros.

This revision was automatically updated to reflect the committed changes. Closed by commit rL341534: [clangd] Set SymbolID for sema macros so that they can be merged with index… (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D51688: [clangd] Set SymbolID for sema macros so that they can be merged with index macros.

ioeric updated this revision to Diff 164177. ioeric marked 2 inline comments as done. ioeric added a comment. - improve comment; remove dead code. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51688 Files: clangd/AST.cpp clangd/AST.h clangd/CodeComplete.cpp

[PATCH] D51724: [clangd] Add "Deprecated" field to Symbol and CodeCompletion.

ioeric created this revision. ioeric added reviewers: sammccall, kadircet. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ilya-biryukov. Also set "deprecated" field in LSP CompletionItem. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51724 Files:

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/URI.cpp:200 + return make_string_error("Couldn't convert " + AbsolutePath + + " to any given scheme.");

[PATCH] D51688: [clangd] Set SymbolID for sema macros so that they can be merged with index macros.

ioeric updated this revision to Diff 164055. ioeric added a comment. - Set name for the test symbol. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51688 Files: clangd/AST.cpp clangd/AST.h clangd/CodeComplete.cpp clangd/index/SymbolCollector.cpp

[PATCH] D51688: [clangd] Set SymbolID for sema macros so that they can be merged with index macros.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51688 Files: clangd/AST.cpp clangd/AST.h clangd/CodeComplete.cpp

[PATCH] D51675: [Sema] Store MacroInfo in CodeCompletionResult for macro results.

This revision was automatically updated to reflect the committed changes. Closed by commit rL341476: [Sema] Store MacroInfo in CodeCompletionResult for macro results. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D51675: [Sema] Store MacroInfo in CodeCompletionResult for macro results.

ioeric added inline comments. Comment at: include/clang/Sema/CodeCompleteConsumer.h:847 + /// If the result is RK_Macro, this can store the information about the macro + /// definition. sammccall wrote: > Let's not make this maybe/optional: `If the result is

[PATCH] D51675: [Sema] Store MacroInfo in CodeCompletionResult for macro results.

ioeric updated this revision to Diff 164044. ioeric added a comment. - update comment about missing MacroInfo. Repository: rC Clang https://reviews.llvm.org/D51675 Files: include/clang/Sema/CodeCompleteConsumer.h lib/Sema/SemaCodeComplete.cpp tools/libclang/CXCursor.cpp Index:

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

ioeric requested changes to this revision. ioeric added a comment. This revision now requires changes to proceed. This should be the last round! ;) Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:45 +std::vector> +createPathIterators(llvm::ArrayRef ProximityPaths,

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:59 LookupTable[Sym->ID] = Sym; -SymbolQuality[Sym] = quality(*Sym); +SymbolAndScores[I] = {Sym, static_cast(quality(*Sym))}; } ioeric wrote: > ioeric wrote:

[PATCH] D51675: [Sema] Store MacroInfo in CodeCompletionResult for macro results.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added a subscriber: cfe-commits. This provides information about the macro definition. For example, it can be used to compute macro USRs. Repository: rC Clang https://reviews.llvm.org/D51675 Files:

[PATCH] D51641: [VFS] Cache the current working directory for the real FS.

This revision was automatically updated to reflect the committed changes. Closed by commit rC341455: [VFS] Cache the current working directory for the real FS. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D51641?vs=163986=163994#toc Repository: rC

[PATCH] D51641: [VFS] Cache the current working directory for the real FS.

ioeric updated this revision to Diff 163986. ioeric marked an inline comment as done. ioeric added a comment. - s/Mutex/CWDMutex/ Repository: rC Clang https://reviews.llvm.org/D51641 Files: lib/Basic/VirtualFileSystem.cpp Index: lib/Basic/VirtualFileSystem.cpp

[PATCH] D51641: [VFS] Cache the current working directory for the real FS.

ioeric added inline comments. Comment at: lib/Basic/VirtualFileSystem.cpp:275 return EC; - return Dir.str().str(); + CWDCache = Dir.str(); + return CWDCache; sammccall wrote: > Doesn't this need to be guarded by a lock? I know the current version is >

[PATCH] D51641: [VFS] Cache the current working directory for the real FS.

ioeric updated this revision to Diff 163858. ioeric marked an inline comment as done. ioeric added a comment. - Guard CWDCache with mutex. Repository: rC Clang https://reviews.llvm.org/D51641 Files: lib/Basic/VirtualFileSystem.cpp Index: lib/Basic/VirtualFileSystem.cpp

[PATCH] D51641: [VFS] Cache the current working directory for the real FS.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D51641 Files: lib/Basic/VirtualFileSystem.cpp Index: lib/Basic/VirtualFileSystem.cpp

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

ioeric abandoned this revision. ioeric added a comment. Dropping this in favor of https://reviews.llvm.org/D51638 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51638: [clangd] Load static index asynchronously, add tracing.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lg. Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51585: [clangd] Define a compact binary serialization fomat for symbol slab/index.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51585 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51585: [clangd] Define a compact binary serialization fomat for symbol slab/index.

ioeric added a comment. Super cool! Just a few nits. Comment at: clangd/RIFF.cpp:58 + if (RIFF->ID != fourCC("RIFF")) +return makeError("Extra content after RIFF chunk"); + if (RIFF->Data.size() < 4) The error message seems wrong?

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

ioeric added a comment. There are a few changes/refactorings which I would suggest splitting from this patch, as they would require more thoughts and seem unrelated in this patch. Please see the inline comments :) Comment at: clang-tools-extra/clangd/Quality.h:130 +///

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

ioeric requested changes to this revision. ioeric added a comment. This revision now requires changes to proceed. (and forgot to request changes ;) https://reviews.llvm.org/D51481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51605: [clangd] SymbolOccurrences -> Refs and cleanup

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. nice! Comment at: clangd/index/Index.cpp:153 + // We can reuse the arena, as it only has unique strings and we need them all. + // Reallocate the ref lists on the arena

[PATCH] D51598: [clangd] Avoid crashes in override completions

ioeric added inline comments. Comment at: unittests/clangd/CodeCompleteTests.cpp:1770 + // Check the completions call does not crash. + completions(R"cpp( +struct Base { ilya-biryukov wrote: > Was wondering if testing for crashes LG this way, or adding

[PATCH] D51422: [clangd] Factor out the data-swapping functionality from MemIndex/DexIndex.

ioeric accepted this revision. ioeric added inline comments. Comment at: clangd/index/FileIndex.h:47 + // The shared_ptr keeps the symbols alive. + std::shared_ptr buildMemIndex(); kbobyrev wrote: > sammccall wrote: > > ioeric wrote: > > > Maybe avoid

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

ioeric added inline comments. Comment at: clang-tools-extra/;:1 +//===--- Token.cpp - Symbol Search primitive *- C++ -*-===// +// The LLVM Compiler Infrastructure Unintended change? Comment at:

[PATCH] D51291: [clangd] Support multiple #include headers in one symbol.

This revision was automatically updated to reflect the committed changes. Closed by commit rL341304: [clangd] Support multiple #include headers in one symbol. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D51291

[PATCH] D51291: [clangd] Support multiple #include headers in one symbol.

ioeric updated this revision to Diff 163678. ioeric added a comment. - Document limitation for overload bundling. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51291 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/index/Index.cpp clangd/index/Index.h

[PATCH] D51291: [clangd] Support multiple #include headers in one symbol.

ioeric updated this revision to Diff 163677. ioeric added a comment. - Rebase Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51291 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/index/Index.cpp clangd/index/Index.h clangd/index/Merge.cpp

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:36 +Result.push_back(PathToken); + } return Result; nit: no need for braces. [llvm-style] Comment at:

[PATCH] D51422: [clangd] Factor out the data-swapping functionality from MemIndex/DexIndex.

ioeric added inline comments. Comment at: clangd/index/MemIndex.h:30 + /// Builds an index from a slab. The shared_ptr manages the slab's lifetime. + static std::shared_ptr build(SymbolSlab Slab); sammccall wrote: > ioeric wrote: > > (It's a bit unfortunate

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:137 + BoostingIterators.push_back( + createBoost(create(It->second), P.second + 10)); + } Could you comment on `P.second + 10` here? It sounds

[PATCH] D51422: [clangd] Factor out the data-swapping functionality from MemIndex/DexIndex.

ioeric added inline comments. Comment at: clangd/index/dex/DexIndex.h:42 + // All symbols must outlive this index. + template DexIndex(Range &) { +for (auto & : Symbols) Why is this constructor needed? I think this could restrict the flexibility of

[PATCH] D51528: [NFC] Cleanup Dex

ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:33 std::vector Result = generateIdentifierTrigrams(Sym.Name); -

[PATCH] D51504: [clangd] Flatten out Symbol::Details. It was ill-conceived, sorry.

ioeric accepted this revision. ioeric added a comment. lg Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51422: [clangd] Factor out the data-swapping functionality from MemIndex/DexIndex.

ioeric added inline comments. Comment at: clangd/index/FileIndex.h:47 + // The shared_ptr keeps the symbols alive. + std::shared_ptr buildMemIndex(); Maybe avoid hardcoding the index name, so that we could potentially switch to use a different index

[PATCH] D51291: [clangd] Support multiple #include headers in one symbol.

ioeric updated this revision to Diff 163351. ioeric added a comment. - Return all possible #includes in CodeComplete. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51291 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/index/Index.cpp clangd/index/Index.h

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

ioeric added a comment. In https://reviews.llvm.org/D51475#1219291, @sammccall wrote: > I don't have a strong opinion on async vs sync - startup time is important > and we shouldn't block simple AST-based functionality on the index, but this > introduces some slightly confusing UX for that

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

ioeric added inline comments. Comment at: clangd/tool/ClangdMain.cpp:86 + return nullptr; +Index = AsyncLoad.get(); +return Index.get(); ilya-biryukov wrote: > ioeric wrote: > > ilya-biryukov wrote: > > > ioeric wrote: > > > > ilya-biryukov wrote: >

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

ioeric added a comment. In https://reviews.llvm.org/D51475#1219197, @ilya-biryukov wrote: > In https://reviews.llvm.org/D51475#1219184, @ioeric wrote: > > > The index loading can be slow. When using LLVM YAML index, I need to wait > > for >10s before clangd starts giving me anything useful. We

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

ioeric added a comment. In https://reviews.llvm.org/D51475#1219133, @ilya-biryukov wrote: > Any reason to not just wait for the index to load? Is this a UX concern or a > problem when experimenting? The index loading can be slow. When using LLVM YAML index, I need to wait for >10s before

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

ioeric updated this revision to Diff 163321. ioeric added a comment. - Fix data race. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51475 Files: clangd/tool/ClangdMain.cpp Index: clangd/tool/ClangdMain.cpp ===

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:45 + void build(std::shared_ptr> Symbols, + llvm::ArrayRef URISchemes); sammccall wrote: > ioeric wrote: > > URI schemes are property of `Symbols`. We

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

ioeric added a comment. Some high-level comments :) Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:45 + void build(std::shared_ptr> Symbols, + llvm::ArrayRef URISchemes); URI schemes are property of `Symbols`. We shouldn't need to

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

ioeric updated this revision to Diff 163276. ioeric added a comment. - revert unintended change Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51475 Files: clangd/tool/ClangdMain.cpp Index: clangd/tool/ClangdMain.cpp

[PATCH] D51475: [clangd] Load YAML static index asyncrhonously.

ioeric created this revision. ioeric added reviewers: sammccall, kbobyrev. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov, mgorny. Loading static index can be slow (>10s for LLVM index). This allows the clangd server to be initialized before index is

[PATCH] D51407: [Tooling] Do not restore working dir in ClangTool

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lg Comment at: lib/Tooling/Tooling.cpp:202 -std::string getAbsolutePath(StringRef File) { +static llvm::Expected getAbsolutePath(vfs::FileSystem , +

[PATCH] D51292: [docs] Update clang-rename documentation

ioeric added inline comments. Comment at: clang-tools-extra/docs/clang-rename.rst:140 + +:program:`clangd `_ uses +:program:`clang-rename` infrastructure to handle renaming requests. Currently, nit: `clangd *shares* the

[PATCH] D51291: [clangd] Support multiple #include headers in one symbol.

ioeric updated this revision to Diff 163074. ioeric added a comment. - minor cleanup Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51291 Files: clangd/CodeComplete.cpp clangd/index/Index.cpp clangd/index/Index.h clangd/index/Merge.cpp

[PATCH] D51407: [Tooling] Do not restore working dir in ClangTool

ioeric added inline comments. Comment at: lib/Tooling/Tooling.cpp:419 - } else { -llvm::report_fatal_error("Cannot detect current path: " + - Twine(CWD.getError().message())); ilya-biryukov wrote: > ioeric wrote: > > Should we

[PATCH] D51407: [Tooling] Do not restore working dir in ClangTool

ioeric added inline comments. Comment at: lib/Tooling/Tooling.cpp:419 - } else { -llvm::report_fatal_error("Cannot detect current path: " + - Twine(CWD.getError().message())); Should we still check if `CWD` is correctly set?

[PATCH] D51291: [clangd] Support multiple #include headers in one symbol.

ioeric added a comment. Thanks for the review! I reduced the scope of the patch. PTAL Comment at: clangd/CodeComplete.cpp:1396 + if (IndexResult && !IndexResult->IncludeHeaders.empty()) { +for (const auto : IndexResult->IncludeHeaders) +

[PATCH] D51291: [clangd] *Prototype* Support multiple #include headers in one symbol.

ioeric updated this revision to Diff 162897. ioeric marked 3 inline comments as done. ioeric retitled this revision from "[clangd] Support multiple #include headers in one symbol." to "[clangd] *Prototype* Support multiple #include headers in one symbol.". ioeric edited the summary of this

[PATCH] D51352: [clangd] Switch to Dex by default for the static index

ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:32 +// FIXME(kbobyrev): This option should be removed as Dex is now the default +// static index.

[PATCH] D51352: [clangd] Switch to Dex by default for the static index

ioeric added inline comments. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:32 static llvm::cl::opt -UseDex("use-dex-index", I think we should stick to the same option and just flip the default. Introducing yet another option (that is going to

[PATCH] D51349: [clangd] Use buffered llvm::errs() in the clangd binary.

This revision was automatically updated to reflect the committed changes. ioeric marked an inline comment as done. Closed by commit rCTE340822: [clangd] Use buffered llvm::errs() in the clangd binary. (authored by ioeric, committed by ). Changed prior to commit:

[PATCH] D51349: [clangd] Use buffered llvm::errs() in the clangd binary.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Unbuffered stream can cause significant (non-deterministic) latency for the logger. Repository: rCTE Clang Tools Extra

[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd

ioeric added a comment. In https://reviews.llvm.org/D51297#1214297, @ilya-biryukov wrote: > Do we want to keep the docs for different editors separate or do we want to > put them all into a single page in case we add more editors? > I would vouch for keeping them on a single page, but that's

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

ioeric added inline comments. Comment at: lib/Basic/SourceManager.cpp:1626 +if (FileContentCache->OrigEntry == SourceFile) { + if (Callback(FileID::get(I))) +return true; Should we check `FileID::get(I)` is valid? Comment at:

[PATCH] D51291: [clangd] *Prototype* Support multiple #include headers in one symbol.

ioeric updated this revision to Diff 162650. ioeric added a comment. - minor cleanup Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51291 Files: clangd/CodeComplete.cpp clangd/Quality.cpp clangd/Quality.h clangd/index/Index.cpp clangd/index/Index.h

[PATCH] D51291: [clangd] *Prototype* Support multiple #include headers in one symbol.

ioeric added a comment. @sammccall The code still needs cleanup but should be useful for providing high-level feedback, which I would like to get before moving further. Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51291

[PATCH] D51291: [clangd] *Prototype* Support multiple #include headers in one symbol.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Currently, a symbol can have only one #include header attached, which might not work well if the symbol can be imported via different

[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE340604: [clangd] Speculative code completion index request before Sema is run. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D50962?vs=162356=162358#toc

[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.

ioeric updated this revision to Diff 162356. ioeric added a comment. - add doc Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50962 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/TUScheduler.h

[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.

ioeric updated this revision to Diff 162355. ioeric added a comment. - moved SpecReq into CodeComplete.cpp Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50962 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/CodeComplete.cpp clangd/CodeComplete.h

[PATCH] D50385: [clangd] Collect symbol occurrences from AST.

ioeric added inline comments. Comment at: clangd/index/SymbolCollector.cpp:241 -SymbolCollector::SymbolCollector(Options Opts) : Opts(std::move(Opts)) {} +class SymbolCollector::CollectOccurrence { +public: I don't see a strong reason for the separation of

[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.

ioeric updated this revision to Diff 162351. ioeric added a comment. - fix doc Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50962 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/TUScheduler.h

[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.

ioeric updated this revision to Diff 162348. ioeric marked 5 inline comments as done. ioeric added a comment. - Merge remote-tracking branch 'origin/master' into speculate-index - address comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50962 Files:

[PATCH] D51159: [FileManager] Do not call 'real_path' in getFile().

This revision was automatically updated to reflect the committed changes. Closed by commit rC340598: [FileManager] Do not call real_path in getFile(). (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D51159?vs=162337=162344#toc Repository: rC Clang

[PATCH] D50385: [clangd] Collect symbol occurrences from AST.

ioeric added inline comments. Comment at: clangd/index/SymbolCollector.h:74 +// If not null, SymbolCollector will collect symbols. +const CollectSymbolOptions *SymOpts; +// If not null, SymbolCollector will collect symbol occurrences. Use

[PATCH] D51159: [FileManager] Do not call 'real_path' in getFile().

ioeric added inline comments. Comment at: lib/Basic/FileManager.cpp:319 - SmallString<128> RealPathName; - if (!FS->getRealPath(InterndFileName, RealPathName)) -UFE.RealPathName = RealPathName.str(); + if (UFE.File) { +if (auto Path = UFE.File->getName()) {

[PATCH] D51159: [FileManager] Do not call 'real_path' in getFile().

ioeric updated this revision to Diff 162337. ioeric added a comment. - Compute absolute paths even when file is not opened. Repository: rC Clang https://reviews.llvm.org/D51159 Files: lib/Basic/FileManager.cpp Index: lib/Basic/FileManager.cpp

[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.

ioeric updated this revision to Diff 162335. ioeric added a comment. - fix typos.. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50962 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/TUScheduler.h

[PATCH] D51159: [FileManager] Do not call 'real_path' in getFile().

ioeric added inline comments. Comment at: lib/Basic/FileManager.cpp:319 - SmallString<128> RealPathName; - if (!FS->getRealPath(InterndFileName, RealPathName)) -UFE.RealPathName = RealPathName.str(); + if (UFE.File) { +if (auto Path = UFE.File->getName()) {

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

ioeric added inline comments. Herald added a subscriber: kadircet. Comment at: clangd/SourceCode.cpp:209 + llvm::SmallString<128> RealPath; + if (SourceMgr.getFileManager().getVirtualFileSystem()->getRealPath( + Path, RealPath)) { With the recent

<    1   2   3   4   5   6   7   8   9   10   >