[PATCH] D61495: [clangd] Use more relaxed fuzzy-matching rules for local variables and class members.

ilya-biryukov added a comment. Another fair question is: how much should we care about client-side and server-side filtering consistencty in the first place? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61495/new/ https://reviews.llvm.org/D61495

[PATCH] D61537: [clangd] Boost code completion results that were named in the last few lines.

ilya-biryukov added a comment. Nice! Looking at the numbers, the improvements look worthwhile. The stopwords don't seem to be in the patch anymore, but they do seem like a good idea. Specifically, we might want to remove all keywords - the intuition is that they don't provide much value, becaus

[PATCH] D61537: [clangd] Boost code completion results that were named in the last few lines.

ilya-biryukov added a comment. Nah, it's ok. LGTM Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61537/new/ https://reviews.llvm.org/D61537 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:/

[PATCH] D61589: [CodeComplete] Add a trailing semicolons to some pattern completions

ilya-biryukov created this revision. ilya-biryukov added a reviewer: gribozavr. Herald added a project: clang. Where semicolon is required in any case. Here's a list of completions that now have a semicolon: - namespace = ; - using namespace ; - using ::; - continue; - break; - goto ; - return;

[PATCH] D61589: [CodeComplete] Add a trailing semicolons to some pattern completions

ilya-biryukov updated this revision to Diff 198254. ilya-biryukov added a comment. - Add tests for 'continue;' and 'break;' completions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61589/new/ https://reviews.llvm.org/D61589 Files: clang/lib/Sem

[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

ilya-biryukov added a comment. Sorry for the delay, I'll make sure to take a close look at the change tomorrow. As mentioned in offline discussions, doing this through `clang-format` seems like the right approach. At the same the markers we put into the files to drive formatting seem fragile, g

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

ilya-biryukov updated this revision to Diff 198298. ilya-biryukov marked 7 inline comments as done. ilya-biryukov added a comment. - Use isWhitespace from CharInfo. - Add a comment for merging text blocks together. - Remove assertion that inline code block does not contain newlines. - Simplify how

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/FormattedString.cpp:71 + // start and end the code block with more. + unsigned MaxBackticks = 0; + for (llvm::StringRef Left = Input;;) { sammccall wrote: > ``` > backticks = "```" > whi

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

ilya-biryukov updated this revision to Diff 198300. ilya-biryukov added a comment. - Remove hover-related bits, they will go into a separate revision Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58547/new/ https://reviews.llvm.org/D58547 Files:

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D61601 Files: clang-tools-extra/clangd/ClangdLSPServer.c

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

ilya-biryukov added a comment. In D61601#1492979 , @kadircet wrote: > I think it makes more sense to expose semantical information in > HoverInfo(like Name, Scope, Definition, etc), rather than formatted strings, > and let this be serialized by the users

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

ilya-biryukov updated this revision to Diff 198410. ilya-biryukov added a comment. - Move range and kind assignment out of the loop. - Log error on failed deserialization of markup kind. - s/Plaintext/PlainText Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D61497: [clangd] Introduce a structured hover response

ilya-biryukov added a comment. Was passing by, just a small clarifying question... Comment at: clang-tools-extra/clangd/XRefs.h:73 + llvm::Optional> Parameters; + llvm::Optional> TemplateParameters; + What does `Type` mean for non-type and template template p

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

ilya-biryukov marked 3 inline comments as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:853 + Hover R; + switch (HoverContentFormat) { + case MarkupKin

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Protocol.cpp:707 + else +return false; + return true; kadircet wrote: > kadircet wrote: > > Maybe also vlog/elog the unknown kind? So that we can easily catch new > > additions to sp

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

ilya-biryukov updated this revision to Diff 198415. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Forgotten change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61601/new/ https://reviews.llvm.org/D61601 Files:

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

ilya-biryukov planned changes to this revision. ilya-biryukov added a comment. Planning to rebase this on top of D61497 and land afterwards in order to minimize merge conflicts. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a subscriber: mgorny. Herald added a project: clang. ilya-biryukov added a parent revision: D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library. An tooling-focused alternative to the

[PATCH] D61497: [clangd] Introduce a structured hover response

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/XRefs.h:57 +// template class. +Type T; +std::string Name; Same about type template parameters. Probably just holds `class` or `typename`? Repository: rG LLVM Github Monorep

[PATCH] D61497: [clangd] Introduce a structured hover response

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/XRefs.h:73 + llvm::Optional> Parameters; + llvm::Optional> TemplateParameters; + kadircet wrote: > ilya-biryukov wrote: > > What does `Type` mean for non-type and template template parame

[PATCH] D61497: [clangd] Introduce a structured hover response

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/XRefs.h:53 +struct HoverInfo { + using Type = std::string; + struct Param { NIT: maybe go with `std::string Type` at a use-site instead? The scope of the name is large enough to make sing

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

ilya-biryukov updated this revision to Diff 198463. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Update a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58547/new/ https://reviews.llvm.org/D58547 Files:

[PATCH] D61642: [clang-tidy] Do not show incorrect fix in modernize-make-unique

ilya-biryukov created this revision. ilya-biryukov added a reviewer: aaron.ballman. Herald added a subscriber: xazax.hun. Herald added a project: clang. The case when initialize_list hides behind an implicit case was not handled before. Repository: rG LLVM Github Monorepo https://reviews.llvm

[PATCH] D61642: [clang-tidy] Do not show incorrect fix in modernize-make-unique

ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. BTW, for a common use-case we can do the same trick that's being done for aggregate init: new X({1,2,3}, 123, {a}); into make_unique(X({1,2,3}, 123, {a})); I can try fixing this, but would want to land this fir

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

ilya-biryukov added inline comments. Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:130 + OS << llvm::formatv( + "['{0}'_{1}, '{2}'_{3}) => ['{4}'_{5}, '{6}'_{7})\n", + PrintToken(File.SpelledTokens[M.BeginSpelled]), M.BeginSpelled, sa

[PATCH] D61642: [clang-tidy] Do not show incorrect fix in modernize-make-unique

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:302 if (const auto *ImplicitCE = dyn_cast(Arg->IgnoreImplicit())) { if (ImplicitCE->isStdInitListInitialization()) aaron.b

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:40 +/// node. +void traverse(Node *N, llvm::function_ref Visit); +void traverse(const Node *N, llvm::function_ref Visit); sammccall wrote: > I've been burned with adding th

[PATCH] D61642: [clang-tidy] Do not show incorrect fix in modernize-make-unique

ilya-biryukov updated this revision to Diff 198497. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. - Remove redundant check. - Actually check the code stays the same in tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Corpus.h:23 +/// token buffers, source manager, etc. +class Corpus { +public: sammccall wrote: > I think plain SyntaxArena might be a better name here :-/ > Corpus refers to texts

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 198609. ilya-biryukov added a comment. - s/corpus/arena - Remove an accidental cmake change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61637/new/ https://reviews.llvm.org/D61637 Files: clang/include

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 198606. ilya-biryukov marked 5 inline comments as done. ilya-biryukov added a comment. - Make traverse() internal to its only use-site. - s/Corpus/Arena. - Address some other comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/index/Background.cpp:482 FileDigest Digest; +bool IsMainFile; }; NIT: maybe initialize with `=false` to avoid potential UB. `Digest` seems uninitialized too, could also `={}` fo

[PATCH] D53866: [Preamble] Stop circular inclusion of main file when building preamble

ilya-biryukov added a comment. Sorry for losing this. Neat change, minimal and focused, thanks! Just wanted to clarify why we need the change in `SourceManager.cpp`, will LGTM as soon as we resolve this Comment at: lib/Basic/SourceManager.cpp:1594 SourceFileName = ll

[PATCH] D61681: [clangd] A code tweak to expand a macro

ilya-biryukov created this revision. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, mgorny. Herald added a project: clang. ilya-biryukov added a parent revision: D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library. Repository: rG LLVM Github Monorepo https

[PATCH] D61681: [clangd] A code tweak to expand a macro

ilya-biryukov added a comment. This actually works, but still far from landing. Notable problems: - this adds a dependency on `TokenBuffer`, so we need to land it first. - this change is too big, planning to split into multiple changes: (1) collecting tokens when building the AST for clangd, (2)

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

ilya-biryukov added a comment. Again, sorry for the delay. This looks good, just a few NITs from me before I stamp it Comment at: lib/Frontend/PrecompiledPreamble.cpp:457 + llvm::StringMap OverridenFileBuffers; for (const auto &RB : PreprocessorOpts.RemappedFileBuffers)

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 198662. ilya-biryukov added a comment. Herald added subscribers: jsji, kbarton, nemanjai. - Propagate whether the token came from a token stream through CachingLex. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added a comment. I've managed to get what I needed by returning a flag from `Lex`. @rsmith, could you take a look? Is there a better way to do the equivalent? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/ https://reviews.ll

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 198663. ilya-biryukov added a comment. - Get rid of an accidental change in how LexAfterModuleImport is handled Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/ https://reviews.llvm.org/D59885 Fi

[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

ilya-biryukov added a comment. In D60605#1495268 , @yvvan wrote: > @ilya-biryukov > What do you think about D53072 ? It can be > polished and combined with this change removing some code from here (which I > assume is a

[PATCH] D53072: [clang-format] Create a new tool for IDEs based on clang-format

ilya-biryukov added a comment. In D53072#1478575 , @yvvan wrote: > @sammccall > > Having a separate tool is nice because it allows the client to make it > plugable. clang-format sometimes changes options quite significantly and it > can be nice if you ha

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ilya-biryukov added a comment. Sorry for the delay. I personally like the `RewriteRule::Action` best. Allows to use a rather common term, while still avoiding any possible confusion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61335/new/ https:/

[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:101 FileToRefs.erase(Path); - else -FileToRefs[Path] = std::move(Refs); + else { +R

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

ilya-biryukov added a comment. We should definitely land this. @Dmitry.Kozhevnikov, you don't have commit access, right? Should we land these two revisions for you? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D50993/new/ https://reviews.llvm.org/

[PATCH] D61724: [clangd] Use AsyncTaskRunner in BackgroundIndex instead of std::thread

ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. Herald added subscribers: jfb, arphaman, jkorous, MaskRay. Herald added a project: clang. To unify the way we create threads in clangd. This should simplify landing D50993 . Repository

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

ilya-biryukov added a comment. D61724 refactors `BackgroundIndexer` to use `AsyncTaskRunner`. So by the time this lands, it should cover all places where threads are created in clangd. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://

[PATCH] D61724: [clangd] Use AsyncTaskRunner in BackgroundIndex instead of std::thread

ilya-biryukov updated this revision to Diff 198791. ilya-biryukov marked 4 inline comments as done. ilya-biryukov added a comment. - Count from 0. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61724/new/ https://reviews.llvm.org/D61724 Files: cl

[PATCH] D61724: [clangd] Use AsyncTaskRunner in BackgroundIndex instead of std::thread

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/index/Background.cpp:152 + for (unsigned I = 1; I <= ThreadPoolSize; ++I) { +ThreadPool.runAsync("background-worker-" + llvm::Twine(I), +[this] { run(); }); kad

[PATCH] D53866: [Preamble] Stop circular inclusion of main file when building preamble

ilya-biryukov added inline comments. Comment at: lib/Basic/SourceManager.cpp:1594 SourceFileName = llvm::sys::path::filename(SourceFile->getName()); -if (*SourceFileName == llvm::sys::path::filename(MainFile->getName())) { +if (MainFile && *SourceFileNam

[PATCH] D61734: [clangd] Bump index version and get rid of wrong assertion

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61734/new/ https://reviews.llvm.org/D61734 _

[PATCH] D53866: [Preamble] Stop circular inclusion of main file when building preamble

ilya-biryukov accepted this revision. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. See the nit about naming of an error, though Comment at: include/clang/Basic/DiagnosticLexKinds.td:429

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added a comment. Here are some numbers from running `clang -cc1 -Eonly` on `SemaExpr.cpp`, it includes a whole ton of headers, including a rather large `TreeTransform.h`. This was run on my machine, so keep in mind it's rather noisy. Minimal times should be somewhat representative

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

ilya-biryukov updated this revision to Diff 198857. ilya-biryukov added a comment. - Move the constuction logic to a separate class, split into multiple methods. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59887/new/ https://reviews.llvm.org/D598

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

ilya-biryukov added inline comments. Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:130 + OS << llvm::formatv( + "['{0}'_{1}, '{2}'_{3}) => ['{4}'_{5}, '{6}'_{7})\n", + PrintToken(File.SpelledTokens[M.BeginSpelled]), M.BeginSpelled, il

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

ilya-biryukov updated this revision to Diff 198862. ilya-biryukov marked 6 inline comments as done. ilya-biryukov added a comment. - Move filling the gaps at the end of files to a separate function Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59887

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

ilya-biryukov updated this revision to Diff 198867. ilya-biryukov marked 5 inline comments as done. ilya-biryukov added a comment. - Check invariants on FileRange construction, unify access to all fields Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

ilya-biryukov marked 2 inline comments as done. ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:62 + SourceLocation location() const { return Location; } + SourceLocation endLocation() const { +return Location.getLocWithOffset(Le

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

ilya-biryukov marked 3 inline comments as done. ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:78 + /// For debugging purposes. + std::string str() const; + sammccall wrote: > ilya-biryukov wrote: > > sammccall wrot

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

ilya-biryukov updated this revision to Diff 198868. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Use bsearch instead of upper_bound Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59887/new/ https://reviews.llvm.or

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

ilya-biryukov added a comment. I'd still put it into LLVM to avoid platform-specific code in clangd. Maybe `std::abort()` in the added `...Async` function if threads are disabled? It's a bit unusual, but would allow keeping this function where it belongs. Repository: rCTE Clang Tools Extra

[PATCH] D45719: [clang-Format] Fix indentation of member call after block

ilya-biryukov added a comment. Landed the patch as r342363. Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52076: [CodeComplete] Add completions for filenames in #include directives.

ilya-biryukov added a comment. Sorry for the late response. Please see the comments about adding closing quotes and slashes. It does not seem to work in the clients that auto-add closing quotes or when completing inside existing includes. Comment at: lib/Lex/Lexer.cpp:1931 +

[PATCH] D52098: [Index] Add an option to collect macros from preprocesor.

ilya-biryukov added a comment. Overall LG, see the major comment about running without the preprocessor. Comment at: include/clang/Index/IndexingAction.h:44 bool IndexImplicitInstantiation = false; + bool IndexMacrosInPreprocessor = false; }; Maybe add a c

[PATCH] D52083: [clangd] Store OR iterator children in heap

ilya-biryukov added a comment. > Seems like this optimization is not worth (yet?). As soon as we get more > proximity paths (and hence more OR iterator children) that might make sense. WDYT about storing **all** the elements with the minimal doc-id outside the heap? I.e. we can pop **all** elem

[PATCH] D52076: [CodeComplete] Add completions for filenames in #include directives.

ilya-biryukov added inline comments. Comment at: lib/Lex/Lexer.cpp:2086 + StringRef PartialPath(PathStart, CompletionPoint - PathStart); + auto Slash = PartialPath.rfind('/'); + StringRef Dir = This will probably break for backslash includes. We should probabl

[PATCH] D52098: [Index] Add an option to collect macros from preprocesor.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: include/clang/Index/IndexingAction.h:59 /// Recursively indexes \p Decls. -/// Note that this does not index macros. -void indexTopLevelDecls(

[PATCH] D52079: [Sema] Do not load macros from preamble when LoadExternal is false.

ilya-biryukov added a comment. Maybe add a test? There is `test/Index/complete-pch-skip.cpp` that checks similar things for AST completions, adding macros there should be trivial. Repository: rC Clang https://reviews.llvm.org/D52079 ___ cfe-com

[PATCH] D52076: [CodeComplete] Add completions for filenames in #include directives.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Sema/SemaCodeComplete.cpp:8057 + if (!(Filename.endswith(".h") || Filename.endswith(".hh") || +Filename.endswith("

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

ilya-biryukov added a comment. Maybe commit only an option to enable function arg snippets for now (found myself wanting this option today :-))? The fixes would also be nice, but since they never work... Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51214 _

[PATCH] D52079: [Sema] Do not load macros from preamble when LoadExternal is false.

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

[PATCH] D51214: [clangd] Add option to enable/disable function argument snippets.

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

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

ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, ioeric. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, mgorny. Provides facilities to model the C++ conversion rules without the AST. The introduced representation can be stored in the index and used to

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

ilya-biryukov added a comment. The implementation might look a bit scary, please feel free to ask for comments/clarifications! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52273 ___ cfe-commits mailing list cfe-commits@lists.llvm

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

ilya-biryukov added inline comments. Comment at: clangd/ExpectedTypes.h:119 + explicit SType(SHA1Array Data); + SHA1Array Data; +}; I assume this will be controversial. Happy to discuss/change. We are currently building this representation based on USRs for typ

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

ilya-biryukov created this revision. ilya-biryukov added reviewers: ioeric, sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. And add a hidden option to control whether the types are collected. For experiments, will be removed when expected types implementation is stabiliz

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

ilya-biryukov planned changes to this revision. ilya-biryukov added a comment. Need to store the types in RIFF format too Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52274 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D52275: [Index] Expose USR generation for types

ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, ioeric. Herald added a subscriber: kadircet. Used in clangd. Repository: rC Clang https://reviews.llvm.org/D52275 Files: include/clang/Index/USRGeneration.h lib/Index/USRGeneration.cpp Index: lib/Index/USRGe

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

ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, ioeric. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. ilya-biryukov added a dependency: D52274: [clangd] Collect and store expected types in the index. ilya-biryukov planned changes to this revision. i

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

ilya-biryukov planned changes to this revision. ilya-biryukov added a comment. Ranking-related code should be moved to `Quality.cpp` Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52276 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D52261: [Sema] Generate completion fix-its for C code as well

ilya-biryukov added a comment. It's the responsibility of the caller to provide a corrected expression, this keeps the completion function simpler. I suggest we stick to this contract. We do something in `getLangOpts().CPlusPlus` case to create a corrected expression, let's just implement a simi

[PATCH] D52047: [clangd] Add building benchmark and memory consumption tracking

ilya-biryukov added a comment. Also not sure about the trick: - Would be surprising to see the "ms" instead of "mbytes" - Time tends to vary between runs, so using Google Benchmark's capabilities to run multiple times and report aggregated times seems useful. Not so much for the memory usage: i

[PATCH] D52261: [Sema] Generate completion fix-its for C code as well

ilya-biryukov added a comment. In https://reviews.llvm.org/D52261#1240143, @yvvan wrote: > I tried that first but did not I find a way just to copy an expression (we > basically need the same expr for such case). Do you know how to properly > generate a copy of expression or some other way to g

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

ilya-biryukov created this revision. ilya-biryukov added reviewers: ioeric, sammccall, simark. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52311 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clan

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

ilya-biryukov planned changes to this revision. ilya-biryukov added a comment. Posted to make sure it's visible that I've started doing this. Still need to update the tests and check for the capability from the client (and fallback to SymbolInformation if client does not support the new implemen

[PATCH] D52261: [Sema] Generate completion fix-its for C code as well

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

[PATCH] D52420: [clangd] Fix crash if pending computations were active on exit

ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, ioeric. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Make sure JSONRPCDispatcher outlives the worker threads, they access its fields to remove the stored cancellations when Context dies. Repository:

[PATCH] D52422: [clangd] Handle template args for disabled function arg snippets

ilya-biryukov created this revision. ilya-biryukov added reviewers: kadircet, ioeric, sammccall. Herald added subscribers: arphaman, jkorous, MaskRay. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52422 Files: clangd/CodeComplete.cpp unittests/clangd/CodeCompleteTests.cpp I

[PATCH] D50171: [python] [tests] Update test_code_completion

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D50171#1236792, @mgorny wrote: > @ilya-biryukov, gentle ping. I'd like to patch this for 7.0.0 in Gentoo. Do > you think my patch would be good enough

[PATCH] D52420: [clangd] Fix crash if pending computations were active on exit

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.cpp:483 + // Destroy ClangdServer to ensure all worker threads finish. + Server.reset(); ioeric wrote: > This woudn't work if `run()` is called multiple times. Maybe create a > `Server`

[PATCH] D52422: [clangd] Handle template args for disabled function arg snippets

ilya-biryukov added inline comments. Comment at: clangd/CodeComplete.cpp:1699 + if (Opts.EnableSnippets) { +log("Suffix: {0}", SnippetSuffix); LSP.textEdit->newText += SnippetSuffix; Sorry, leftover from debug printing. Will remove Repository: rCTE

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

ilya-biryukov added a comment. > This seems very clever, but extremely complicated - you've implemented much > of C++'s conversion logic, it's not clear to me which parts are actually > necessary to completion quality. Clearly the model that supports C++ conversions is something that **will**

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

ilya-biryukov added inline comments. Comment at: clangd/ExpectedTypes.h:68 + +/// Represents a type of partially applied conversion. Should be treated as an +/// opaque value and can only be used to check whether the types are converible sammccall wrote: > this r

[PATCH] D52419: [clangd] Cache FS stat() calls when building preamble.

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:119 +/// Collect and cache all file status from the underlying file system. +class CollectStatCacheVFS : public vfs::FileSystem { ioeric wrote: > Would it make sense to add a `clang::vfs::Pr

[PATCH] D52422: [clangd] Handle template args for disabled function arg snippets

ilya-biryukov updated this revision to Diff 167044. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Remove debug formatting - Update snippet for no-arg case with the suggested changes Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52422 Files:

[PATCH] D52422: [clangd] Handle template args for disabled function arg snippets

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clangd/CodeComplete.cpp:507 + if (Snippet->front() == '<') +return EmptyArgs ? "<$0>()" : "<$1>($0)"; + if (Snippet->front() == '(') kadircet wrote:

[PATCH] D52419: [clangd] Cache FS stat() calls when building preamble.

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:339 +llvm::ErrorOr status(const Twine &Path) override { + auto I = StatCache.find(Path.str()); + return (I != StatCache.end()) ? I->getValue() : FS->status(Path); ioeric wrote: >

[PATCH] D52547: Tell whether file/folder for include completions.

ilya-biryukov added a comment. A drive-by comment. Would it be cleaner to pass this information from clang? Relying on completion label seems shaky. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52547 ___ cfe-commits mailing list

[PATCH] D52620: Added Support for StatOnly Files in VFS.

ilya-biryukov added a comment. > I would call it a requirement instead of an assumption. The replay must be > exactly the same (even the file stats and reads). If Clang reads the file in > replay which was only stat()ed during compilation, it indicates > non-determinism or something wrong (in c

[PATCH] D52710: [clangd] Add "check-clangd" target

ilya-biryukov added a comment. This is awesome! Can't wait for it to land! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52710 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D52647: [CodeComplete] Re-fix accessibilty of protected members from base class.

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

<    9   10   11   12   13   14   15   16   17   18   >