[PATCH] D51287: [clangd] Use TRUE iterator instead of complete posting list

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

[PATCH] D51279: [clangd] Implement findOccurrences interface in dynamic index.

ilya-biryukov added a comment. The memory usage looks good. Some NITs and a major consideration wrt to the implementation of merging occurences from dynamic and static index. Comment at: clangd/index/FileIndex.cpp:48 + if (TopLevelDecls) { // index main AST, set occurrence

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

ilya-biryukov added inline comments. Comment at: clang-tools-extra/docs/clang-rename.rst:28 +:program:`clang-rename` infrastructure to handle renaming requests. Because of +much better editor integration and support, it is advised to use +:program:`clangd-rename` as part of :prog

[PATCH] D51279: [clangd] Implement findOccurrences interface in dynamic index.

ilya-biryukov added a comment. Just noticed I'm not on the reviewers list, sorry for chiming in without invitation. Was really excited about the change :-) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51279 ___ cfe-commits mailin

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

ilya-biryukov added inline comments. Comment at: clangd/tool/ClangdMain.cpp:202 +"Like changing an arrow(->) member access to dot(.) member access."), +llvm::cl::init(clangd::CodeCompleteOptions().IncludeFixIts)); + sammccall wrote: > ilya-biryukov wr

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

ilya-biryukov added a comment. 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 not a strong opinion. Wonder what other people think? https://revi

[PATCH] D51298: [Tooling] Allow to filter files used by AllTUsExecutor

ilya-biryukov created this revision. ilya-biryukov added a reviewer: ioeric. This allows to run clang tools in parallel on a subset of files in the repository. Repository: rC Clang https://reviews.llvm.org/D51298 Files: include/clang/Tooling/AllTUsExecution.h lib/Tooling/AllTUsExecution.

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

ilya-biryukov added a comment. In https://reviews.llvm.org/D51297#1214313, @kbobyrev wrote: > 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 mor

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a subscriber: jfb. This greatly reduces the time to read 'compile_commands.json'. For Chromium on my machine it's now 5 secs vs 30 secs before the change. Repository: rC Clang https://reviews.llvm.org

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

ilya-biryukov added inline comments. Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:440 Result.emplace_back(std::move(Command)); - if (Result.back().Type == types::TY_INVALID) -Result.pop_back(); We can't look at 'Type' at this p

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

ilya-biryukov updated this revision to Diff 162699. ilya-biryukov added a comment. - Add a comment - Use std::call_once to compute the lazy value Repository: rC Clang https://reviews.llvm.org/D51314 Files: lib/Tooling/InterpolatingCompilationDatabase.cpp Index: lib/Tooling/InterpolatingCo

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

ilya-biryukov updated this revision to Diff 162701. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Remove computeTraits, put the body inside a lambda Repository: rC Clang https://reviews.llvm.org/D51314 Files: lib/Tooling/InterpolatingCompilationDatabase.c

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

ilya-biryukov added inline comments. Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:201 + + CommandTraits computeTraits() const { +CommandTraits Result; jfb wrote: > It's not clear to me that this entire function is safe to call from multiple

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

ilya-biryukov updated this revision to Diff 162702. ilya-biryukov added a comment. - Remove #include , it is not used anymore Repository: rC Clang https://reviews.llvm.org/D51314 Files: lib/Tooling/InterpolatingCompilationDatabase.cpp Index: lib/Tooling/InterpolatingCompilationDatabase.cp

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

ilya-biryukov added inline comments. Comment at: clang-tools-extra/docs/clang-rename.rst:28 +:program:`clang-rename` infrastructure to handle renaming requests. Because of +much better editor integration and support, it is advised to use +:program:`clangd-rename` as part of :prog

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

ilya-biryukov added inline comments. Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:124 +// A CompileCommand that can be applied to another file. Any instance of this +// object is invalid after std::move() from it. struct TransferableCommand { jfb

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

ilya-biryukov updated this revision to Diff 162817. ilya-biryukov added a comment. Herald added a subscriber: mgrang. - Remove mutexes, recompute every time instead - Delay creation of TransferableCommand to avoid calling getAllCommands() on JSONCompilationDatabase Repository: rC Clang https

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

ilya-biryukov marked 4 inline comments as done. ilya-biryukov added inline comments. Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:133 +assert(TraitsComputed && "calling transferTo on moved-from object"); +const CommandTraits &T = getTraits(); +CompileC

[PATCH] D51298: [Tooling] Allow to filter files used by AllTUsExecutor

ilya-biryukov planned changes to this revision. ilya-biryukov added a comment. As discussed offline, instead of changing the interface of AllTUsScheduler, we could add concurrency to StandloneToolExecutor Repository: rC Clang https://reviews.llvm.org/D51298 ___

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

ilya-biryukov updated this revision to Diff 162849. ilya-biryukov marked 6 inline comments as done. ilya-biryukov added a comment. - Update the comments - Rename the new class to FileIndex - Restore an accidentally lost comment - Store file types in a parallel array instead of recomputing on each

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

ilya-biryukov updated this revision to Diff 162851. ilya-biryukov added a comment. - Reformat the code - Minor spelling fix Repository: rC Clang https://reviews.llvm.org/D51314 Files: lib/Tooling/InterpolatingCompilationDatabase.cpp Index: lib/Tooling/InterpolatingCompilationDatabase.cpp

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

ilya-biryukov updated this revision to Diff 162855. ilya-biryukov added a comment. - Lowercase everything stored in the index. Repository: rC Clang https://reviews.llvm.org/D51314 Files: lib/Tooling/InterpolatingCompilationDatabase.cpp Index: lib/Tooling/InterpolatingCompilationDatabase.c

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

ilya-biryukov updated this revision to Diff 162861. ilya-biryukov added a comment. - Handle TransferableCommands with TY_INVALID type (never transfer -x flag for those) - Add a test with invalid extensions, seen a crash while experimenting - Update the test wrt to the new behavior. Repository:

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

ilya-biryukov updated this revision to Diff 162863. ilya-biryukov added a comment. - Sort Paths, they are different from OriginalPaths, i.e. lowercased. Repository: rC Clang https://reviews.llvm.org/D51314 Files: lib/Tooling/InterpolatingCompilationDatabase.cpp unittests/Tooling/Compilat

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

ilya-biryukov updated this revision to Diff 162868. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. - Cleanups Repository: rC Clang https://reviews.llvm.org/D51314 Files: lib/Tooling/InterpolatingCompilationDatabase.cpp unittests/Tooling/CompilationDatabase

[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

ilya-biryukov added a comment. Thanks for the change. Having something like this makes total sense. Mutating existing in-memory nodes looks shaky and requires making `Status` mutable, which also looks undesirable. Maybe we could consider adding a new kind of `InMemoryNode` for hard links that w

[PATCH] D50438: [clangd] Sort GoToDefinition results.

ilya-biryukov added inline comments. Herald added a subscriber: kadircet. Comment at: clangd/XRefs.cpp:105 +// Sort results. Declarations being referenced explicitly come first. +std::sort(Result.begin(), Result.end(), + [](const DeclInfo &L, const DeclInfo &

[PATCH] D50438: [clangd] Sort GoToDefinition results.

ilya-biryukov added a comment. Sorry for the delays with this review Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50438 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D51311: (WIP) Type hierarchy

ilya-biryukov added a comment. Thanks for looking into this. Would be cool to get this supported after the proposal is finalized. Comment at: clangd/Protocol.h:891 + + std::vector Parents; + What is the proposal to add children (i.e. overriden methods) to the

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

ilya-biryukov created this revision. ilya-biryukov added reviewers: ioeric, sammccall. Resolve all relative paths before running the tool instead. This fixes the usage of ClangTool in AllTUsExecutor. The executor will try running multiple ClangTool instances in parallel with compile commands that

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

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

[PATCH] D51311: (WIP) Type hierarchy

ilya-biryukov added a subscriber: sammccall. ilya-biryukov added inline comments. Comment at: clangd/XRefs.cpp:669 + const CXXMethodDecl *Candidate) { + // FIXME: How do I determine if Method overrides Candidate? + kadircet wrote: > ilya

[PATCH] D51380: Fix incorrect usage of std::error_category

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

[PATCH] D51221: [clangd] Some nitpicking around the new split (preamble/main) dynamic index

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clangd/ClangdServer.cpp:122 + // - symbols declared both in the main file and the preamble + // (Note that symbols *only* in the main file a

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

ilya-biryukov updated this revision to Diff 163071. ilya-biryukov added a comment. - Use vfs for file accesses - Report errors when getAbsolutePath fails, skip those files Repository: rC Clang https://reviews.llvm.org/D51407 Files: lib/Tooling/Tooling.cpp Index: lib/Tooling/Tooling.cpp =

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

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

[PATCH] D51380: Fix incorrect usage of std::error_category

ilya-biryukov added a comment. Thanks for fixing this. Repository: rC Clang https://reviews.llvm.org/D51380 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51279: [clangd] Implement findOccurrences interface in dynamic index.

ilya-biryukov added inline comments. Comment at: clangd/index/FileIndex.cpp:48 + if (TopLevelDecls) { // index main AST, set occurrence flag. +CollectorOpts.OccurrenceFilter = SymbolOccurrenceKind::Declaration | hokein wrote: > ilya-biryukov wrote: > > The

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

ilya-biryukov added a comment. Leaving some comments, but also suggest getting the final LGTM from the owner of the doc (@ioeric?) Comment at: clang-tools-extra/docs/clang-rename.rst:141 +:program:`clangd `_ uses +:program:`clang-renam

[PATCH] D51436: [CodeComplete] Report location of opening parens for signature help

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, ioeric. Used in clangd. Repository: rC Clang https://reviews.llvm.org/D51436 Files: include/clang/Sema/CodeCompleteConsumer.h include/clang/Sema/Sema.h lib/Frontend/ASTUn

[PATCH] D51437: [clangd] Report position of opening paren in singature help

ilya-biryukov updated this revision to Diff 163116. ilya-biryukov added a comment. - Format the code Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51437 Files: clangd/CodeComplete.cpp clangd/Protocol.h unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeC

[PATCH] D51437: [clangd] Report position of opening paren in singature help

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric. ilya-biryukov added a dependency: D51436: [CodeComplete] Report location of opening parens for signature help. ilya-biryukov updated this revision

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

ilya-biryukov updated this revision to Diff 163122. ilya-biryukov added a comment. - Expose getAbsolutePath in the public API Repository: rC Clang https://reviews.llvm.org/D51407 Files: include/clang/Tooling/Tooling.h lib/Tooling/Tooling.cpp Index: lib/Tooling/Tooling.cpp ==

[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

ilya-biryukov added a comment. Could we try removing `Status` from the base class and move it into `InMemoryFile` and `InMemoryDir`? It shouldn't be too much work and would give safer interfaces for our new hierarchy. Comment at: lib/Basic/VirtualFileSystem.cpp:470 -enum In

[PATCH] D51437: [clangd] Report position of opening paren in singature help

ilya-biryukov updated this revision to Diff 163286. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Clarify the docs, add example. - Parse each test separately. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51437 Files: clangd/CodeComplete.cp

[PATCH] D51437: [clangd] Report position of opening paren in singature help

ilya-biryukov added inline comments. Comment at: unittests/clangd/CodeCompleteTests.cpp:914 +TEST(SignatureHelpTest, OpeningParen) { + Annotations Code(R"cpp( sammccall wrote: > Hmm, I think this test would be easier to follow if tests 1-5 were written > sepa

[PATCH] D51438: [clangd] Run SignatureHelp using an up-to-date preamble, waiting if needed.

ilya-biryukov added a comment. The implementation is really simple. LG! To get proper operation order in tests, we can wait in the diagnostics callback that runs on the worker thread (IIRC, we do that in some of the other tests too). Comment at: clangd/TUScheduler.cpp:188 +

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

ilya-biryukov added inline comments. Comment at: clangd/tool/ClangdMain.cpp:86 + return nullptr; +Index = AsyncLoad.get(); +return Index.get(); I believe is a data race (multiple threads may run this line concurrently). You would want some synchroniz

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

ilya-biryukov added a comment. Any reason to not just wait for the index to load? Is this a UX concern or a problem when experimenting? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51475 ___ cfe-commits mailing list cfe-commits@l

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

ilya-biryukov added a comment. 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 could potentially > speed up loading (e.g. replacing yaml), but the

[PATCH] D51438: [clangd] Run SignatureHelp using an up-to-date preamble, waiting if needed.

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

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

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

[PATCH] D50438: [clangd] Sort GoToDefinition results.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LG, thanks. And a question of what are the things we can accidentally misdetect as explicit Comment at: clangd/XRefs.cpp:105 +// Sort results. Declarations

[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

ilya-biryukov added inline comments. Comment at: include/clang/Basic/VirtualFileSystem.h:359 + /// Add a HardLink from one file to another. + /// Makes the UniqueID of To file same as that of From file. If CopyBuffer is ilya-biryukov wrote: > Maybe use the co

[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

ilya-biryukov added a comment. Thanks, the interface and implementation look good! Last drop of nits and suggestions for code simplification. Comment at: include/clang/Basic/VirtualFileSystem.h:359 + public: + /// Add a HardLink to a File. + /// The To path must be an existi

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

ilya-biryukov created this revision. ilya-biryukov added reviewers: kadircet, ioeric, hokein, sammccall. Herald added subscribers: arphaman, jkorous, MaskRay. NamedDecl::getName cannot be called on non-identifier names. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51598 Files

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

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

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

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

[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

ilya-biryukov added inline comments. Comment at: lib/Basic/VirtualFileSystem.cpp:478 +public: + InMemoryNode(const std::string& FileName, InMemoryNodeKind Kind) + : Kind(Kind), FileName(llvm::sys::path::filename(FileName.data())) {} NIT: accept a parameter

[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Thanks! LGTM with a last drop of NITs. Do you already have commit access or do you want me to submit this change for you? Comment at: include/clang/Basic/Virtu

[PATCH] D51221: [clangd] Some nitpicking around the new split (preamble/main) dynamic index

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:152 WorkScheduler(Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory, -DynamicIdx ? *DynamicIdx : noopParsingCallbacks(), +DynamicIdx ? DynamicIdx->makeUpdateC

[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

ilya-biryukov added a comment. In https://reviews.llvm.org/D51359#1222823, @usaxena95 wrote: > I don't have commit access. Can you please submit this ? Will do. Repository: rC Clang https://reviews.llvm.org/D51359 ___ cfe-commits mailing list

[PATCH] D50462: Try building complete AST after a fatal error was emitted if further diagnostics are expected

ilya-biryukov added a comment. In https://reviews.llvm.org/D50462#1214120, @Dmitry.Kozhevnikov wrote: > When lookin through the list of the fatal errors, I think there are different > categories: > > 1. "Scary" ones - "module was relocated", "invalid vfs overlay file", we > probably shouldn't t

[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

ilya-biryukov added a comment. Submitted. Thanks for the change! Repository: rL LLVM https://reviews.llvm.org/D51359 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50438: [clangd] Sort GoToDefinition results.

ilya-biryukov added a comment. @hokein, would it be fine if I submit this change for you? It would be nice to get this fix in before you get back from vacation. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50438 ___ cfe-commits ma

[PATCH] D50814: [clangd] transfer the fixits with the notes instead of adding them to the main diagnostic if request by the client

ilya-biryukov added a comment. Sorry for the delayed response. It seems we absolutely need this if mirroring libclang is an absolute requirement. We seem to have multiple implementation options: Which classes to use for representing diagnostics? We can: 1. Reuse existing hierarchy for diagnost

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

ilya-biryukov added inline comments. Comment at: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp:287 +StaticIdx.reset(Placeholder = new SwapIndex(llvm::make_unique())); +runAsync([Placeholder] { + if (auto Idx = loadIndex(YamlSymbolFile)) Wouldn'

[PATCH] D50438: [clangd] Sort GoToDefinition results.

ilya-biryukov added a comment. In https://reviews.llvm.org/D50438#1224289, @hokein wrote:> > Thanks, and sorry for the delay here, I was planning to submit it after > https://reviews.llvm.org/D50958 is submitted (probably today?)-- because it > requires some rebasing changes. Thanks for the u

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

ilya-biryukov added a comment. I would stamp this from my side, but concerns whether we should recommend YCM's LSP-based completer instead are probably still there. @sammccall, WDYT? https://reviews.llvm.org/D51297 ___ cfe-commits mailing list cfe-

[PATCH] D51674: [clangd] Fix async index loading (from r341376).

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/D51674 ___ 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 inline comments. Comment at: test/Index/complete-block-property-assignment.m:71 +// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO1 %s +// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35) ---

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

ilya-biryukov added inline comments. Comment at: test/Index/complete-block-property-assignment.m:71 +// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO1 %s +// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35) ---

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

ilya-biryukov added a comment. Not sure if it's fine to hijack our own diagnostic-specific flags in to clang command args. Const that I see: 1. There is no way for the users to turn them off if they find them non-useful. If we add a way, it would be more config parameters which overlap with ot

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

ilya-biryukov added inline comments. Comment at: test/Index/complete-block-property-assignment.m:71 +// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO1 %s +// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35) ---

[PATCH] D51782: [CodeComplete] Clearly distinguish signature help and code completion.

ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, kadircet. Code completion in clang is actually a mix of two features: - Code completion is a familiar feature. Results are exposed via the CodeCompleteConsumer::ProcessCodeCompleteResults callback. - Signature help fi

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

ilya-biryukov added inline comments. Comment at: test/Index/complete-block-property-assignment.m:71 +// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO1 %s +// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35) ---

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

ilya-biryukov added inline comments. Comment at: test/Index/complete-block-property-assignment.m:71 +// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO1 %s +// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35) ---

[PATCH] D51628: [clangd] Implement a Proof-of-Concept tool for symbol index exploration

ilya-biryukov added a comment. Definitely like the idea of the tool. The main complication seems to be parsing of user input at this point. I suggest we explore an option proposed before, that is reusing the LLVM command-line parser (see inline comment too). If that turns out to be much work, w

[PATCH] D51802: [clangd] Make advanceTo() faster on Posting Lists

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/dex/Iterator.cpp:41 assert(!reachedEnd() && "DOCUMENT iterator can't advance() at the end."); +if (peek

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

ilya-biryukov added a comment. > Currently the problem is, there are again some tests out there that rely on > CodeCompeleteOrdinaryName to be called even when getting overloads at an > unknown > parameter type CodeCompleteExpression works just fine there. Unknown parameter type should not bl

[PATCH] D51628: [clangd] Implement a Proof-of-Concept tool for symbol index exploration

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/dexplorer/Dexplorer.cpp:39 + +// FIXME(kbobyrev): Make this an actual REPL: probably use LLVM Command Line +// library for parsing flags and arguments. kbobyrev wrote: > ilya-biryukov wrote

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

ilya-biryukov added inline comments. Comment at: include/clang/Parse/Parser.h:217 + /// Gets set to true after calling ProduceSignaturehelp, it is for a + /// workaround to make sure ProduceSignatureHelp is only called at the deepest s/ProduceSignaturehelp/P

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

ilya-biryukov added a comment. ilya-biryukov added inline comments. Comment at: lib/Sema/CodeCompleteConsumer.cpp:622 +case CodeCompletionString::CK_Optional: + break; This change is really sneaky and unrelated to the rest of the patch. It should defi

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

ilya-biryukov added inline comments. Comment at: lib/Parse/ParseExpr.cpp:1663 if (Tok.isNot(tok::r_paren)) { - if (ParseExpressionList(ArgExprs, CommaLocs, [&] { -QualType PreferredType = Actions.ProduceCallSignatureHelp( -get

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

ilya-biryukov added inline comments. Comment at: lib/Sema/CodeCompleteConsumer.cpp:622 +case CodeCompletionString::CK_Optional: + break; ilya-biryukov wrote: > This change is really sneaky and unrelated to the rest of the patch. It > should definitely

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

ilya-biryukov added inline comments. Comment at: lib/Parse/ParseExprCXX.cpp:2827 if (Tok.isNot(tok::r_paren)) { + ParsedType TypeRep = + Actions.ActOnTypeName(getCurScope(), DeclaratorInfo).get(); ilya-biryukov wrote: > ActOnTypeName is called

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

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Thanks! LGTM Comment at: lib/Parse/ParseExprCXX.cpp:1687 if (Tok.isNot(tok::r_paren)) { - if (ParseExpressionList(Exprs, CommaLocs, [&] { -

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

ilya-biryukov added a comment. > I also agree with you regarding options. A pattern I've observed (in some > infamous large codebase ;) is that warnings for deprecated symbols are > disabled in the compile command as they can introduce too much noise during > build, although it would make sense

[PATCH] D51039: [clangd] Add unittests for D51038

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

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

ilya-biryukov added a comment. > Most of the value of adding an option is: if someone complains, we can tell > them to go away :-) One possible corollary is: we shouldn't add the option > until someone complains. I'm not 100% sure about that, though - not totally > opposed to an option here. A

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

ilya-biryukov created this revision. ilya-biryukov added reviewers: ioeric, steveire. And add an option to disable this behavior. The option is only used in AllTUsExecutor to avoid races when running concurrently on multiple threads. This fixes PR38869 introduced by r340937. Repository: rC Cl

[PATCH] D51865: [clang-tidy] Added a test -export-fixes with relative paths.

ilya-biryukov created this revision. ilya-biryukov added reviewers: ioeric, steveire. Herald added a subscriber: xazax.hun. A test for https://reviews.llvm.org/D51864. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51865 Files: test/clang-tidy/export-relpath.cpp Index: test

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

ilya-biryukov added a comment. Also can't explain why `const` and `volatile` have a different priority now. The `P::` and `Q::` seem to be completely different completion items from `P` and `Q` (wildly different priorities suggest they're not the same), can't explain what caused the first ones n

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

ilya-biryukov 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 = std::numeric_limi

[PATCH] D51917: [CodeCompletion] Enable signature help when initializing class/struct/union members.

ilya-biryukov added inline comments. Comment at: include/clang/Sema/Sema.h:10798 +ValueDecl *tryGetMember(CXXRecordDecl *ClassDecl, CXXScopeSpec &SS, +ParsedType TemplateTypeTy, The name is very generic, but the helper is only applicable

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

ilya-biryukov 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 = std::numeric_limi

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

ilya-biryukov added a comment. LG from my side, @sammccall is not a big fan of options so please wait for his approval too Comment at: clangd/tool/ClangdMain.cpp:174 +llvm::cl::desc( +"Enables suggestion of completion items that needs additional changes. " +

[PATCH] D51628: [clangd] Implement a Proof-of-Concept tool for symbol index exploration

ilya-biryukov added a comment. Dexplorer is a long name. How about shortening it to `dexp`? Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:56 + +template +void reportTime(StringRef Name, Function F) { Why do we want `TimeUnit`? It adds

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

ilya-biryukov added inline comments. Comment at: clangd/tool/ClangdMain.cpp:197 +static llvm::cl::opt IncludeFixIts( +"include-fixits", sammccall wrote: > ilya-biryukov wrote: > > sammccall wrote: > > > ilya-biryukov wrote: > > > > I wonder if we should mak

[PATCH] D51917: [CodeCompletion] Enable signature help when initializing class/struct/union members.

ilya-biryukov added inline comments. Comment at: lib/Parse/ParseDeclCXX.cpp:3472 +ParseExpressionList(ArgExprs, CommaLocs, [&] { + if (CalledSignatureHelp) +return; Let's always call signature help and code completion here to be consi

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