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

ilya-biryukov added a comment. Really excited about this one, this should give us decent latency improvements when using the static index. My main suggestion would be to put almost all of the speculating code into `CodeComplete.cpp`. We could merely return the request that should be used for sp

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

ilya-biryukov added a comment. - New mode ./bin/global-symbol-builder -merge-on-the-fly -executor=all-TUs > /dev/null 40079.41s user 1874.40s system 5340% cpu 13:05.56 total - Old mode Still running, expect it to be more than 40minutes... Will update when I have the numbers. Repository:

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

ilya-biryukov added inline comments. Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:61 +/// rely on MR use-case to work properly. +llvm::cl::init(false)); + ioeric wrote: > AFAIK, this flag would basically depend on the executor bein

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

ilya-biryukov added a comment. The numbers are finally there New mode ./bin/global-symbol-builder -merge-on-the-fly -executor=all-TUs > /dev/null 40079.41s user 1874.40s system 5340% cpu 13:05.56 total Old mode ./bin/global-symbol-builder -merge-on-the-fly=false -executor=all-TUs > /dev

[PATCH] D51102: [clangd] Move function argument snippet disable mechanism from LSP rendering to internal clangd reprensentation.

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/D51102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

ilya-biryukov added a comment. One major limitation of the current workaround is that IIRC clang can actually call code completion callbacks, including this method, multiple times (e.g. when completing inside a macro definition that gets expanded later in the file or during parser recovery). Th

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

ilya-biryukov added inline comments. Comment at: clangd/index/Index.h:46 + +inline bool operator==(const SymbolLocation::Position &L, + const SymbolLocation::Position &R) { hokein wrote: > ilya-biryukov wrote: > > NIT: having friend decls in

[PATCH] D50993: [clangd] Increase stack size of the new threads on macOS

ilya-biryukov accepted this revision. ilya-biryukov added a comment. LGTM. Many thanks! See the NIT about avoiding the helper class too. Comment at: clangd/Threading.cpp:88 + llvm::llvm_execute_on_thread_async( + Callable{Name.str(), std::move(Action), std::move(CleanupTa

[PATCH] D51154: [clangd] Log memory usage of DexIndex and MemIndex

ilya-biryukov added a comment. Some drive-by NITS. Comment at: clang-tools-extra/clangd/index/MemIndex.cpp:109 +std::lock_guard Lock(Mutex); + +Bytes += Index.getMemorySize(); Why not simply `return Index.getMemorySize()` under a lock? ===

[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

ilya-biryukov added inline comments. Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:61 +/// rely on MR use-case to work properly. +llvm::cl::init(false)); + ioeric wrote: > ilya-biryukov wrote: > > ioeric wrote: > > > AFAIK, this fla

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

ilya-biryukov updated this revision to Diff 145691. ilya-biryukov added a comment. - Fixed infinite loop with comments that contain doxygen commands Repository: rC Clang https://reviews.llvm.org/D46000 Files: include/clang/AST/CommentLexer.h include/clang/AST/RawCommentList.h lib/AST/C

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

ilya-biryukov added inline comments. Comment at: include/clang-c/Index.h:5237 +/** + * \brief FixIts that *must* be applied before inserting the text for the + * corresponding completion item. Completion items with non-empty fixits will This seems too large for a

[PATCH] D45999: [clangd] Retrieve minimally formatted comment text in completion.

ilya-biryukov updated this revision to Diff 145883. ilya-biryukov marked 4 inline comments as done. ilya-biryukov added a comment. - Renames and other comments - Don't include brief comments in signature help either, comments there are also handled by the code completion code now. Repository:

[PATCH] D45999: [clangd] Retrieve minimally formatted comment text in completion.

ilya-biryukov added inline comments. Comment at: clangd/CodeCompletionStrings.h:24 +/// Gets a raw documentation comment of \p Result. +/// Returns empty string when no comment is available. sammccall wrote: > What does raw mean - range of the file? indentation

[PATCH] D46002: [clangd] Parse all comments in Sema and completion.

ilya-biryukov updated this revision to Diff 145890. ilya-biryukov added a comment. - Moved tests to clang Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46002 Files: clangd/ClangdUnit.cpp clangd/CodeComplete.cpp Index: clangd/CodeComplete.cpp =

[PATCH] D46639: [CodeComplete] Provide completion in decls even for incomplete types

ilya-biryukov created this revision. ilya-biryukov added reviewers: bkramer, aaron.ballman, sammccall. This change fixes lack of completions in the following case ('^'designates completion points) : void f(^); struct Incomplete; Incomplete g(^); Repository: rC Clang https://reviews.llv

[PATCH] D42966: Fix USR generation in the presence of #line directives or linemarkes

ilya-biryukov added a comment. In https://reviews.llvm.org/D42966#1085303, @mikhail.ramalho wrote: > Hi, > > > Where do virtual files come from in the first place? > > From the linemarker: I tried dumping locations in presence of line markers. They have non-null `FileEntry` and a reasonable off

[PATCH] D46676: [clangd] Remove LSP command-based #include insertion.

ilya-biryukov added a comment. This LG, but we should first roll out the `additionalEdits` change. Aren't the dependencies reversed in the dependency stack, i.e. this change should be the last one? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46676

[PATCH] D46670: [clangd] Move helpers that convert Replacements to TextEdits to SourceCode.h

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, just a few non-blocking NITs with questions. Comment at: clangd/SourceCode.cpp:180 + // Turn the replacements into the format specified by the Language S

[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.

ilya-biryukov added inline comments. Comment at: clangd/index/SymbolCollector.cpp:95 + auto FileName = SM.getFilename(findNameLoc(ND)); + if (FileName.endswith(".proto.h") || FileName.endswith(".pb.h")) { +auto Name = ND->getName(); NIT: reduce nesting by i

[PATCH] D46496: [Tooling] Pull #include manipulation code from clangFormat into libToolingCore.

ilya-biryukov added a comment. This change is big enough to be somewhat hard to grasp. Maybe we could split it into two, to make it easier for review? I.e. - Extraction of IncludeStyle into a separate subclass. - Moving stuff around without other refactorings. Comment at: incl

[PATCH] D46676: [clangd] Remove LSP command-based #include insertion.

ilya-biryukov added inline comments. Comment at: unittests/clangd/ClangdTests.cpp:941 -TEST_F(ClangdVFSTest, InsertIncludes) { - MockFSProvider FS; Do we test the same thing somewhere else (e.g. code completion) in one of the dependent changes? Maybe it's wo

[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.

ilya-biryukov added a comment. In https://reviews.llvm.org/D46751#1095894, @malaperle wrote: > Can there be an option for this? This seems very library specific and could > break other code bases. Ideally, there would be a generic mechanism for this > kind of filtering, i.e. specify a pattern o

[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.

ilya-biryukov added a subscriber: sammccall. ilya-biryukov added a comment. So, the first line of the file generated by proto compiler seems to be something like this: // Generated by the protocol buffer compiler. DO NOT EDIT! If we check the symbol comes from a file with this comment, there

[PATCH] D46676: [clangd] Remove LSP command-based #include insertion.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LG when all the dependencies are done Comment at: unittests/clangd/ClangdTests.cpp:941 -TEST_F(ClangdVFSTest, InsertIncludes) { - MockFSProvider FS;

[PATCH] D46684: [Frontend] Don't skip function body when the return type is dependent on the template parameter.

ilya-biryukov added inline comments. Comment at: lib/Sema/SemaDecl.cpp:12645 +if (FD->isConstexpr() || FD->getReturnType()->isUndeducedType() || +FD->getReturnType()->isInstantiationDependentType()) return false; rsmith wrote: > This is a lot b

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

ilya-biryukov updated this revision to Diff 146326. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Simplify test code with SourceManagerForFile. Repository: rC Clang https://reviews.llvm.org/D46000 Files: include/clang/AST/CommentLexer.h include/clang/AS

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

ilya-biryukov added inline comments. Comment at: include/clang/AST/RawCommentList.h:138 + /// the overload with ASTContext in the rest of the code. + std::string getFormattedText(const SourceManager &SourceMgr, + DiagnosticsEngine &Diags) const; --

[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.

ilya-biryukov added a comment. > Here :) > > http://www.sidefx.com/docs/hdk/_s_o_p___bone_capture_lines_8proto_8h_source.html Didn't take along to find an example. Thanks for digging this up. That looks like a good enough reason to not apply proto-specific filtering based solely on the filena

[PATCH] D46758: [clang-format] Move #include related style to libToolingCore

ilya-biryukov added inline comments. Comment at: lib/Format/Format.cpp:461 + +template <> struct MappingTraits { + static void mapping(IO &IO, IncludeStyle::IncludeCategory &Category) { Should we move this to `tooling::Core` too? If we need to specialize `Mappin

[PATCH] D46795: [clangd] Don't query index when completing inside classes

ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, ioeric. Herald added subscribers: jkorous, MaskRay, klimek. We used to query the index when completing after class qualifiers, e.g. 'ClassName::^'. We should not do that for the same reasons we don't query the index for

[PATCH] D46795: [clangd] Don't query index when completing inside classes

ilya-biryukov added inline comments. Comment at: unittests/clangd/CodeCompleteTests.cpp:829 +TEST(CompletionTest, NoIndexCompletionsInsideClasses) { + // clang-format off + auto Completions = completions(R"cpp( sammccall wrote: > Can we avoid disabling clang-fo

[PATCH] D46795: [clangd] Don't query index when completing inside classes

ilya-biryukov updated this revision to Diff 146571. ilya-biryukov marked 5 inline comments as done. ilya-biryukov added a comment. - Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46795 Files: clangd/CodeComplete.cpp unittests/clangd/CodeCompleteTest

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

ilya-biryukov added a comment. In https://reviews.llvm.org/D41537#1097601, @yvvan wrote: > I hope this review will be over some day :) Sorry for keeping it that long. FWIW, I think we keep jumping back and forth between the clang and libclang changes here. I should've suggested splitting the c

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

ilya-biryukov added inline comments. Comment at: tools/libclang/CIndexCodeCompletion.cpp:309 + /// before that result for the corresponding completion item. + std::vector> FixItsVector; }; yvvan wrote: > ilya-biryukov wrote: > > Storing `vector>` here makes lo

[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only

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/D45815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D46758: [clang-format] Move #include related style to libToolingCore

ilya-biryukov added a comment. LG. Just a quick comment about possibly unrelated change. Comment at: lib/Format/Format.cpp:2277 auto Replace = -Includes.insert(trimInclude(IncludeName), IncludeName.startswith("<")); +Includes.insert(IncludeName.trim("\"<>")

[PATCH] D46758: [clang-format] Move #include related style to libToolingCore

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/D46758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D46496: [Tooling] Pull #include manipulation code from clangFormat into libToolingCore.

ilya-biryukov added a comment. This LG, presuming there are no semantic changes here, just moving the code around. Also see the nits about `IncludeStyle` that seems to be in the wrong change... Comment at: include/clang/Tooling/Core/HeaderIncludes.h:112 +/// priorities for hea

[PATCH] D46496: [Tooling] Pull #include manipulation code from clangFormat into libToolingCore.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Format/Format.cpp:1702 // cases where the first #include is unlikely to be the main header. - IncludeCategoryManager Categories(Style,

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

ilya-biryukov updated this revision to Diff 146757. ilya-biryukov added a comment. - Removed the overload that accepts ASTContext Repository: rC Clang https://reviews.llvm.org/D46000 Files: include/clang/AST/CommentLexer.h include/clang/AST/RawCommentList.h lib/AST/CommentLexer.cpp l

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

ilya-biryukov added a comment. Maybe move the tests back to this revision? There are tests for code completion that don't depend on libindex or libclang and use clang -cc1 directly, i.e. `tools/clang/test/CodeComplete`. This would require adding a flag to clang and patching up `PrintingCodeCompl

[PATCH] D46001: [CodeComplete] Expose helpers to get RawComment of completion result.

ilya-biryukov updated this revision to Diff 146812. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Rebase onto head and remove \brief from the comments, it's gone upstream now - Rename CurrentArg to ArgIndex Repository: rC Clang https://reviews.llvm.org/D4600

[PATCH] D46524: [clangd] Extract scoring/ranking logic, and shave yaks.

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/D46524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45999: [clangd] Retrieve minimally formatted comment text in completion.

ilya-biryukov updated this revision to Diff 146827. ilya-biryukov marked 6 inline comments as done. ilya-biryukov added a comment. - Fix code after a change in deps (getFormattedText now needs a SourceManager instead of an ASTContext) - Address review comments Repository: rCTE Clang Tools Ext

[PATCH] D45999: [clangd] Retrieve minimally formatted comment text in completion.

ilya-biryukov added inline comments. Comment at: clangd/CodeCompletionStrings.h:29 + +/// Gets a raw documentation comment of the current active parameter +/// of \p Result. sammccall wrote: > sammccall wrote: > > ilya-biryukov wrote: > > > ilya-biryukov wrote: >

[PATCH] D46002: [clangd] Parse all comments in Sema and completion.

ilya-biryukov updated this revision to Diff 146831. ilya-biryukov added a comment. - Added tests that all comments are being parsed. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46002 Files: clangd/ClangdUnit.cpp clangd/CodeComplete.cpp unittests/clangd/CodeCompleteTest

[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.cpp:274 WorkspaceEdit WE; WE.changes = {{Params.textDocument.uri.uri(), Edits}}; reply(WE); NIT: not related to this change, but maybe use `std::move(Edits)` to avo

[PATCH] D46751: [clangd] Filter out private proto symbols in SymbolCollector.

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/D46751 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45999: [clangd] Retrieve minimally formatted comment text in completion.

ilya-biryukov updated this revision to Diff 147060. ilya-biryukov added a comment. Rebase onto head, fix merge conflicts Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45999 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/CodeCompletionStrings.cpp clangd/Cod

[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

ilya-biryukov created this revision. ilya-biryukov added reviewers: ioeric, sammccall. Herald added subscribers: mgrang, jkorous, MaskRay, klimek. This should, arguably, give better ranking. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46943 Files: clangd/AST.cpp clangd/A

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

ilya-biryukov added a comment. Sorry, a few more things and we're good to go. Comment at: include/clang/Driver/CC1Options.td:449 +def code_completion_include_fixits : Flag<["-"], "code-completion-include-fixits">, + HelpText<"Include code completion results which require havi

[PATCH] D44954: [clangd] Add "member" symbols to the index

ilya-biryukov added a comment. A few questions regarding class members. To pinpoint some interesting cases and agree on how we want those to behave in the long run. How do we handle template specializations? What will the qualified names of those instantiations be? I.e. how do I query for `push

[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

ilya-biryukov added inline comments. Comment at: clangd/Quality.cpp:24 + if (SemaCCResult.Declaration) +AllDeclsInMainFile = allDeclsInMainFile(SemaCCResult.Declaration); if (SemaCCResult.Availability == CXAvailability_Deprecated) sammccall wrote: > ioeri

[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

ilya-biryukov updated this revision to Diff 147301. ilya-biryukov added a comment. - Move tests to QualityTests.cpp - Fix findDecl() assertion on redecls of the same thing - Fix file comment of TestTU.cpp Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46943 Files: clangd/AST.

[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

ilya-biryukov added inline comments. Comment at: clangd/AST.h:32 +/// Returns true iff all redecls of \p D are in the main file. +bool allDeclsInMainFile(const Decl *D); sammccall wrote: > Do you expect this to be reused? > If so, unit test? Otherwise this seem

[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

ilya-biryukov added inline comments. Comment at: clangd/Quality.cpp:24 + if (SemaCCResult.Declaration) +AllDeclsInMainFile = allDeclsInMainFile(SemaCCResult.Declaration); if (SemaCCResult.Availability == CXAvailability_Deprecated) ioeric wrote: > ioeric w

[PATCH] D44954: [clangd] Add "member" symbols to the index

ilya-biryukov added a comment. In https://reviews.llvm.org/D44954#1103664, @malaperle wrote: > It's probably better to consider this in a future patch. Maybe something like > the first suggestion: vector::push_back and match both. Otherwise, I would > think it might be a bit too verbose to have

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: jkorous, MaskRay, ioeric, javed.absar, klimek. After this commit, clangd will only keep the last 3 accessed ASTs in memory. Preambles for each of the opened files are still kept in memory to m

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

ilya-biryukov added a comment. Another alternative that I've considered was evicting the ASTs from memory after a certain period of time, e.g. after 30 seconds of inactivity for some file. This might be simpler and also cover the use-case of speeding up multiple code navigation requests (findDe

[PATCH] D47065: [clangd] Enable parsing of non-doxygen comments in global-symbol-builder

ilya-biryukov created this revision. ilya-biryukov added reviewers: ioeric, sammccall. Herald added subscribers: jkorous, MaskRay, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47065 Files: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp Index: clangd/global

[PATCH] D47066: [clangd] Remove ignored Preamble::CanReuse call from completion

ilya-biryukov created this revision. ilya-biryukov added reviewers: ioeric, sammccall. Herald added subscribers: jkorous, MaskRay, klimek. Now that the clients who relied on stats for files from preamble are gone. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47066 Files: cl

[PATCH] D47068: Move #include manipulation code to new lib/Tooling/Inclusions.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM with a minor naming suggestion. Comment at: include/clang/Format/Format.h:20 #include "clang/Tooling/Core/Replacement.h" +#include "clang/Tooling/Inclusio

[PATCH] D47183: [clangd] Support multiple sema code completion callbacks.

ilya-biryukov added inline comments. Comment at: clangd/CodeComplete.cpp:457 Result.StartsNestedNameSpecifier = false; + // FIXME: the same result can be added multiple times as the callback can + // be called more than once. We may want to deduplicate identical

[PATCH] D47183: [clangd] Support multiple sema code completion callbacks.

ilya-biryukov added inline comments. Comment at: clangd/CodeComplete.cpp:457 Result.StartsNestedNameSpecifier = false; + // FIXME: the same result can be added multiple times as the callback can + // be called more than once. We may want to deduplicate identical

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

ilya-biryukov added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:4109 + if (CodeCompleter->includeFixIts()) { +const SourceRange OpRange(OpLoc, OpLoc.getLocWithOffset(IsArrow ? 2 : 1)); +CompletionSucceded = yvvan wrote: > ilya-biryukov wro

[PATCH] D47065: [clangd] Enable parsing of non-doxygen comments in global-symbol-builder

ilya-biryukov added a comment. In https://reviews.llvm.org/D47065#1104730, @ioeric wrote: > Should we also change the behavior in dynamic index? Dynamic index already uses whatever AST provides (which is `ParseAllComments = true` since https://reviews.llvm.org/D46002) Repository: rCTE Clan

[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

ilya-biryukov updated this revision to Diff 148006. ilya-biryukov added a comment. Herald added a subscriber: mgorny. - Move helpers tha switch header and source into separate files. - Also uprank items coming from the matching headers Repository: rCTE Clang Tools Extra https://reviews.llvm.o

[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

ilya-biryukov added a comment. I've added an initial version of testing for the matching header and wanted to get feedback before proceeding further with tests and other changes. A few things that bug me so far: - We need to match headers of items from the index, not only from the Sema results

[PATCH] D47223: [clangd] Handle enumerators in named, unscoped enums similarly to scoped enums

ilya-biryukov added a subscriber: sammccall. ilya-biryukov added a comment. I'm not sure if we have tests for that, but I remember that we kept the enumerators in the outer scope so that completion could find them.. Am I right that this patch will change the behavior and we won't get enumerators

[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

ilya-biryukov added a comment. In https://reviews.llvm.org/D46943#1109199, @ioeric wrote: > > - Symbols store their paths as URIs ⇒ we need to parse them in order to > > apply heuristics. We could avoid that by writing a version of > > header-matching that also works on URIs, but that would mea

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

ilya-biryukov created this revision. ilya-biryukov added reviewers: ioeric, sammccall. Herald added subscribers: jkorous, MaskRay, javed.absar, klimek. This is more efficient and avoids data races when reading files that come from the preamble. The staleness can occur when reading a file from disk

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

ilya-biryukov added a comment. In https://reviews.llvm.org/D47272#1109909, @malaperle wrote: > > We do not to rely on symbols from the main file anyway, since any info hat > > those provide can always be taken from the AST. > > I'll be adding those soon for workspace symbols... And also for docu

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

ilya-biryukov added a comment. @malaperle, a slight offtopic. I was wondering which completion options do you use for clangd? Defaults? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47272 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

ilya-biryukov updated this revision to Diff 148262. ilya-biryukov added a comment. - Added forgotten renames Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47272 Files: clangd/ClangdServer.cpp clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/TUScheduler.cpp clangd/TUS

[PATCH] D47274: [clangd] Serve comments for headers decls from dynamic index only

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: jkorous, MaskRay, ioeric, klimek. To fix a crash in code completion that occurrs when reading doc comments from files that were updated after the preamble was computed. In that case, the files

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

ilya-biryukov updated this revision to Diff 148355. ilya-biryukov added a comment. - Reimplemented LRU cache with shared_ptr and weak_ptr. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47063 Files: clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/TUScheduler.cpp clangd

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.h:132 -/// Manages resources, required by clangd. Allows to rebuild file with new -/// contents, and provides AST and Preamble for it. -class CppFile { +/// A helper class that handles building preambles and AST

[PATCH] D47256: [clangd] Fix code completion in MACROs with stringification.

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/D47256 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

ilya-biryukov added a comment. After an offline chat: we decided to boost sema results that have **any** decls in the main file. Even though it introduces some unwanted inconsistencies in some cases (e.g. see the comment), most of us agree that's a very simple implementation that does boost use

[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

ilya-biryukov updated this revision to Diff 148381. ilya-biryukov added a comment. - Simplify the change by boosting any Decls that have a redecl in the current file Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46943 Files: clangd/Quality.cpp clangd/Quality.h unittests

[PATCH] D47223: [clangd] Handle enumerators in named, unscoped enums similarly to scoped enums

ilya-biryukov added a comment. > But I feel it's a bit odd that completion and workspace symbols would be > inconsistent. It does not seem that odd to me. Completion is something that should follow the language rules more closely, i.e. we don't want to deviate from sema completions too much. W

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

ilya-biryukov added a comment. I have only a few nits left, otherwise looks good. Thanks for making this change! Really excited for it to get in finally! Comment at: include/clang/Driver/CC1Options.td:449 +def code_completion_with_fixits : Flag<["-"], "code-completion-with-fix

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

ilya-biryukov added a comment. In https://reviews.llvm.org/D47272#1110958, @ioeric wrote: > Would it be possible to add some integration tests for file index plus > preamble? Sure, will do Comment at: clangd/index/FileIndex.cpp:37 + std::vector TopLevelDecls( + AST.ge

[PATCH] D47274: [clangd] Serve comments for headers decls from dynamic index only

ilya-biryukov updated this revision to Diff 148413. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Invert flag, remove the default. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47274 Files: clangd/CodeComplete.cpp clangd/CodeCompletionStr

[PATCH] D47274: [clangd] Serve comments for headers decls from dynamic index only

ilya-biryukov added inline comments. Comment at: clangd/CodeCompletionStrings.h:46 const CodeCompleteConsumer::OverloadCandidate &Result, - unsigned ArgIndex); + unsigned ArgIndex, bool NoCommentsFromHeaders = tr

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

ilya-biryukov added a comment. Added the test. Comment at: clangd/index/FileIndex.cpp:37 + std::vector TopLevelDecls( + AST.getTranslationUnitDecl()->decls().begin(), + AST.getTranslationUnitDecl()->decls().end()); ioeric wrote: > ilya-biryukov wrote

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

ilya-biryukov updated this revision to Diff 148421. ilya-biryukov added a comment. - Added a test for preamble rebuilds. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47272 Files: clangd/ClangdServer.cpp clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/TUScheduler.cpp

[PATCH] D47331: [clangd] Remove accessors for top-level decls from preamble

ilya-biryukov created this revision. ilya-biryukov added reviewers: ioeric, sammccall. Herald added subscribers: jkorous, MaskRay, mehdi_amini, klimek. They cause lots of deserialization and are not actually used. The ParsedAST accessor that previously returned those was renamed from getTopLevelDe

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Thanks for addressing the NITs. LGTM! https://reviews.llvm.org/D41537 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

ilya-biryukov updated this revision to Diff 148599. ilya-biryukov added a comment. - Rebase, fix merge conflicts - Simpler implemenataion of the Cache - s/IdleASTs/ASTCache Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47063 Files: clangd/ClangdUnit.cpp clangd/ClangdUnit.h

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

ilya-biryukov added a comment. I have addressed the comments regarding the cache implementation. ASTBuilder ones are still pending, but I would appreciate the feedback on how `TUScheduler.cpp` looks like. Comment at: clangd/ClangdUnit.h:132 -/// Manages resources, required

[PATCH] D47331: [clangd] Remove accessors for top-level decls from preamble

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.h:81 ASTContext &getASTContext(); const ASTContext &getASTContext() const; ioeric wrote: > IIUC, `ASTContext` in a `ParsedAST` may not contain information from > preambles and thus may gi

[PATCH] D47331: [clangd] Remove accessors for top-level decls from preamble

ilya-biryukov updated this revision to Diff 148602. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. - Rename the field in addition to the getter - Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47331 Files: clangd/ClangdU

[PATCH] D47331: [clangd] Remove accessors for top-level decls from preamble

ilya-biryukov updated this revision to Diff 148605. ilya-biryukov added a comment. - Rewrote findDecl, it will deserialize decls if needed now Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47331 Files: clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/XRefs.cpp unittest

[PATCH] D47223: [clangd] Handle enumerators in named, unscoped enums similarly to scoped enums

ilya-biryukov added a comment. So to keep completion working, maybe we can give add enumerators with full qualification and have a flag that determines whether the last component of the qualifier can be ignored? This would give a simple implementation path for now, and will add a semantic signa

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

ilya-biryukov updated this revision to Diff 148820. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47063 Files: clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/TUSch

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

ilya-biryukov added inline comments. Comment at: clangd/TUScheduler.cpp:71 + + /// Update the function used to compute the value. + void update(std::function()> ComputeF); sammccall wrote: > ilya-biryukov wrote: > > sammccall wrote: > > > I think I understand t

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

ilya-biryukov updated this revision to Diff 148822. ilya-biryukov added a comment. - Remove ASTBuilder, make everything a helper function Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47063 Files: clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/TUScheduler.cpp clangd/

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

ilya-biryukov updated this revision to Diff 148824. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - s/Params/Policy Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47063 Files: clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/TUScheduler.cp

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