[PATCH] D52789: [clangd] Dex: FALSE iterator, peephole optimizations, fix AND bug

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM and a few NITs. Comment at: clangd/index/dex/Iterator.cpp:376 +} +default: + RealChildren.push_back(std::move(Child)); Maybe

[PATCH] D52872: [clangd] Make binary index format the default, remove dead flag.

ilya-biryukov added inline comments. Comment at: clangd/indexer/IndexerMain.cpp:84 - $ clangd-indexer --executor=all-TUs compile_commands.json > index.yaml + $ clangd-indexer --executor=all-TUs compile_commands.json > clangd-index Maybe we should suggest a

[PATCH] D52796: [clangd] Simplify Dex query tree logic and fix missing-posting-list bug

ilya-biryukov added a comment. Overall LGTM, just a quick question to make sure I get what's going on. Comment at: clangd/index/dex/Dex.cpp:184 +ScopeIterators.push_back(iterator(Token(Token::Kind::Scope, Scope))); + if (Req.AnyScope || /*legacy*/ Req.Scopes.empty())

[PATCH] D52796: [clangd] Simplify Dex query tree logic and fix missing-posting-list bug

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Putting a stamp, assuming is answer is "yes, it's a legacy way to specify all scopes" Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52796 __

[PATCH] D57573: Disable tidy checks with too many hits

ilya-biryukov created this revision. ilya-biryukov added a reviewer: alexfh. Herald added projects: clang, LLVM. Some tidy checks have too many hits in the codebase, making it hard to spot other results from clang-tidy, therefore rendering the tool less useful. Two checks were disabled: - misc-n

[PATCH] D57573: Disable tidy checks with too many hits

ilya-biryukov updated this revision to Diff 184704. ilya-biryukov added a comment. - Also disable 'misc-non-private-member-variables-in-classes' in llvm/.clang-tidy Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57573/new/ https://reviews.llvm.org/

[PATCH] D57532: [Index] Make sure c-index-test finds libc++ on Mac

ilya-biryukov updated this revision to Diff 184712. ilya-biryukov added a comment. - Revert "changes to main" - Compute the path in parseTranslationUnit2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57532/new/ https://reviews.llvm.org/D57532 File

[PATCH] D57532: [Index] Make sure c-index-test finds libc++ on Mac

ilya-biryukov added a comment. @arphaman, thanks for pointing this out. It's definitely nice to change stuff only in one place. There are two issues still pending: - `index-file.cu` still fails, I haven't looked into it closely yet, any idea why that might be happening?; repro is simple, just p

[PATCH] D57573: Disable tidy checks with too many hits

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

[PATCH] D57581: Explicitly add language standard option to test cases that rely on the C++14 default

ilya-biryukov added a comment. The change looks reasonable to me, given that you can change the default language version via a build argument. I don't have the required experience with the lit-tests to give proper approval, though, would wait for someone more relevant to do this. Do have a comm

[PATCH] D57581: Explicitly add language standard option to test cases that rely on the C++14 default

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57581/new/ https://reviews.llvm.org/D57581 ___

[PATCH] D57739: [clangd] Format tweak's replacements.

ilya-biryukov added a comment. Could we move the code to the `Tweak` itself, so that all the callers would get the formatting? I.e. class Tweak { Replacements apply() { // This is now non-virtual. auto Replacements = doApply(); return cleanupAndFormat(Replacements); }

[PATCH] D57739: [clangd] Format tweak's replacements.

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:366 + auto Style = getFormatStyle(Code, File); + if (!Style) +return; ioeric wrote: > hokein wrote: > > not sure the err-handling strategy here -- maybe if this is failed, we > > stil

[PATCH] D57755: [clangd] Some minor fixes.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Many thanks! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57755/new/ https://reviews.llvm.org/D57755

[PATCH] D57739: [clangd] Format tweak's replacements.

ilya-biryukov added a comment. In D57739#1385144 , @hokein wrote: > This seems introduce intrusive changes to the Tweak interface (Tweak will > need to know the `FormatStyle` somehow), I think this might be done in > another patch. It's totally fine, s

[PATCH] D57813: [clangd] Enable clangd on ObjC in VSCode

ilya-biryukov created this revision. ilya-biryukov added a reviewer: hokein. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric. Herald added a project: clang. Thanks to Andreas Ostermeyer for raising this on the mailing list. Repository: rG LLVM Github Monorepo https://re

[PATCH] D57814: [clangd] Update clangd-vscode dependencies

ilya-biryukov created this revision. ilya-biryukov added a reviewer: hokein. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, dschuff. Herald added a project: clang. Also add 'package-lock.json' to the repo, it's a common practice to check in those files into the repository.

[PATCH] D57814: [clangd] Update clangd-vscode dependencies

ilya-biryukov added a comment. Update the description to add some context Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57814/new/ https://reviews.llvm.org/D57814 ___ cfe-commits mailing list cfe-commi

[PATCH] D57739: [clangd] Format tweak's replacements.

ilya-biryukov added inline comments. Comment at: clangd/refactor/Tweak.h:56 +/// The style to format generated changes. +format::FormatStyle Style; }; NIT: Maybe make this a second argument of `apply`? This would convey the idea that `execute()` should

[PATCH] D57739: [clangd] Format tweak's replacements.

ilya-biryukov added inline comments. Comment at: clangd/refactor/Tweak.h:81 + /// EXPECTS: prepare() was called and returned true. + virtual Expected execute(const Selection &Sel) = 0; }; hokein wrote: > I think the current name is slightly better than `doAppl

[PATCH] D57814: [clangd] Update dev dependencies of clangd-vscode

ilya-biryukov added a comment. In D57814#1386902 , @hokein wrote: > This patch updates **dev** dependencies, please correct the commit title to > avoid confusion :) Done, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D57739: [clangd] Format tweak's replacements.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM! Also a few NITs. Comment at: clangd/ClangdServer.cpp:155 Opts.ClangTidyOpts = ClangTidyOptProvider->getOptions(File); + // FIXME: cache this + Opts

[PATCH] D57879: [clangd] Fix an assertion failure in Selection.

ilya-biryukov added a comment. Repro: template struct Foo {}; template class /*cursor here*/U> struct Foo*> {}; I'm not sure how easy is that, but this should probably be fixed in the `RecursiveASTVisitor<>`. There's really no point in calling `TraverseDecl(null)` Repository: r

[PATCH] D57819: [clangd] Reduce number of threads used by BackgroundIndex to number of physical cores.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. NIT about a description: we can't be 100% certain that it's related to hyper-threading, so I'd avoid putting that to the description, maybe simply mention that this avoids

[PATCH] D57532: [Index] Make sure c-index-test finds libc++ on Mac

ilya-biryukov updated this revision to Diff 185772. ilya-biryukov added a comment. - Move the string to CIndexer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57532/new/ https://reviews.llvm.org/D57532 Files: clang/test/Index/record-completion-

[PATCH] D57532: [Index] Make sure c-index-test finds libc++ on Mac

ilya-biryukov updated this revision to Diff 185775. ilya-biryukov added a comment. - Simplify code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57532/new/ https://reviews.llvm.org/D57532 Files: clang/test/Index/record-completion-invocation.c

[PATCH] D57739: [clangd] Format tweak's replacements.

ilya-biryukov added a comment. In D57739#1390252 , @sammccall wrote: > I agree with this concern, and don't think this is an appropriate layer to be > aware of styling or failing on format errors. Can we consider moving the > formatting to clangdserver a

[PATCH] D57739: [clangd] Format tweak's replacements.

ilya-biryukov added a comment. In D57739#1390321 , @sammccall wrote: > It's not about stability or whether the functionality is desired, but > layering. > Unit tests having narrow scope is a good thing - if we want system tests > that check clangdserver

[PATCH] D57532: [Index] Make sure c-index-test finds libc++ on Mac

ilya-biryukov planned changes to this revision. ilya-biryukov added a comment. Still need to figure out why the test fails and fix it before submitting the change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57532/new/ https://reviews.llvm.org/D

[PATCH] D58051: Revamp the "[clangd] Format tweak's replacements"

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:382 +auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents, + FSPr

[PATCH] D58111: [Sema] Fix a crash in access checking for deduction guides

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a project: clang. See the added test for a repro. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D58111 Files: clang/lib/Sema/SemaDeclCXX.cpp clang/test/Sema/crash-deduction-guide-a

[PATCH] D58111: [Sema] Fix a crash in access checking for deduction guides

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

[PATCH] D58189: [clang][Index] Fix usage of IndexImplicitInstantiation

ilya-biryukov added a comment. Only have a few NITs, will dig deeper into the change tomorrow. Added @arphaman too as an owner of the index library. Alex, feel free to reassign if you're the wrong person to take a look at this Comment at: unittests/Index/IndexTests.cpp:31 +str

[PATCH] D58134: [Analysis] -Wunreachable-code shouldn't fire on the increment of a foreach loop

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58134/new/ https://reviews.llvm.org/D58134 ___

[PATCH] D58189: [clang][Index] Fix usage of IndexImplicitInstantiation

ilya-biryukov added inline comments. Comment at: unittests/Index/IndexTests.cpp:30 +struct Position { + size_t Line; NIT: put all of the decls of a file into an anonymous namespace Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D58189: [clang][Index] Fix usage of IndexImplicitInstantiation

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58189/new/ https://reviews.llvm.org/D58189 ___

[PATCH] D58340: [clang][Index] Visit UsingDecls and generate USRs for them

ilya-biryukov added inline comments. Comment at: lib/Index/IndexDecl.cpp:588 D->getLexicalDeclContext()); +IndexCtx.handleDecl(D); for (const auto *I : D->shadows()) Any reason to add this to the middle of the fu

[PATCH] D58190: [clangd] Add tests for template specializations

ilya-biryukov added inline comments. Comment at: unittests/clangd/XRefsTests.cpp:376 +} + )cpp", }; While here, could wee add tests for a few more cases? They are all represented by different classes in a hierarchy in clang, so each could potenti

[PATCH] D58190: [clangd] Add tests for template specializations

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM with a typo-nit :-) Comment at: unittests/clangd/XRefsTests.cpp:378 + + R"cpp(// partial tmeplate specialization +template s

[PATCH] D58340: [clang][Index] Visit UsingDecls and generate USRs for them

ilya-biryukov added inline comments. Comment at: lib/Index/IndexDecl.cpp:586 +IndexCtx.handleDecl(D); IndexCtx.indexNestedNameSpecifierLoc(D->getQualifierLoc(), Parent, NIT: maybe put it at the very first line of the function, before the `DC` and `Par

[PATCH] D58340: [clang][Index] Visit UsingDecls and generate USRs for them

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, but we need to make sure the variable is used in configs without assertions, otherwise we'll break buildbots. Repository: rC Clang CHANGES SINCE LAST ACTION https://r

[PATCH] D58341: [clangd] Index UsingDecls

ilya-biryukov added a comment. Could we also add a test for code completion to make sure we return the new decls there? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58341/new/ https://reviews.llvm.org/D58341

[PATCH] D58341: [clangd] Index UsingDecls

ilya-biryukov added a comment. For context: @hokein mentioned problems in the clangd's code completion if we would index these symbols. This patch in itself does not hurt much, users of the indexing API can decide how to deal with `UsingDecl` on their own, clangd is just one of the clients. >

[PATCH] D58341: [clangd] Index UsingDecls

ilya-biryukov added a comment. In D58341#1401295 , @hokein wrote: > std::strcmp is a fair case here. Sema seems not returning using-decls as part > of code completion results, it this an intended behavior? Yeah, I think it is. There's an explicit code p

[PATCH] D58294: [clangd] Enable indexing of template type parameters

ilya-biryukov added a comment. It feels that storing template parameters in the index is a waste, one can only access them through the scope they are introduced in (there are out-of-line definitions of a function, but I think we treat every redeclaration of template headers separately). Can we

[PATCH] D58293: [clang][Index] Enable indexing of Template Type Parameters behind a flag

ilya-biryukov added inline comments. Comment at: lib/Index/IndexingContext.cpp:51 + bool IndexingContext::handleDecl(const Decl *D, SymbolRoleSet Roles, Do we call `handleDecl` for template parameters now too?

[PATCH] D58294: [clangd] Enable indexing of template type parameters

ilya-biryukov added inline comments. Comment at: clangd/XRefs.cpp:42 // Only a single declaration is allowed. - if (isa(D)) // except cases above + if (isa(D) || isa(D)) // except cases above return D; Should probably also handle `TemplateTemplateParmDe

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

ilya-biryukov added a comment. This LGTM wrt to layering, i.e. it's nice we don't need to change the code of diagnostics. It's been a while since I've looked at the ASTUnit code, though, would be good if someone else took an extra look at it. Added a few nits too. Comment at

[PATCH] D58294: [clangd] Enable indexing of template type parameters

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 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58294/new/ https://reviews.llvm.org/D58294 __

[PATCH] D55250: [clangd] Enhance macro hover to see full definition

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.cpp:823 + (*H)->contents.value = + llvm::formatv("```C++\n{0}\n```", (*H)->contents.value); +} The contents of the hover is not strictly C++ code, so

[PATCH] D58293: [clang][Index] Enable indexing of Template Type Parameters behind a flag

ilya-biryukov added inline comments. Comment at: include/clang/Index/IndexingAction.h:49 bool IndexParametersInDeclarations = false; + bool IndexTemplateParmDecls = false; }; NIT: maybe rename to `IndexTemplateParameters`? The `ParmDecl` is a somewhat weird

[PATCH] D58341: [clangd] Index UsingDecls

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM from my side, with a few NITs. But let's wait for an LGTM from Haojian too, to make sure his concerns are addressed. Comment at: unittests/clangd/SymbolCo

[PATCH] D58387: [clangd] Add an option in the code to not display number of fixes

ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, ioeric. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Only to the APIs, which are used by our embedders. We do not plan to add a user-facing option for this. Repository

[PATCH] D58293: [clang][Index] Enable indexing of Template Type Parameters behind a flag

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, but see the comment about avoiding code duplication Comment at: lib/Index/IndexDecl.cpp:678 IndexCtx.indexTypeSourceInfo(TTP->getDefaultArgum

[PATCH] D58440: [clangd] Store index in '.clangd/index' instead of '.clangd-index'

ilya-biryukov created this revision. ilya-biryukov added reviewers: kadircet, sammccall. Herald added subscribers: arphaman, jkorous, MaskRay, ioeric. Herald added a project: clang. To take up the .clangd folder for other potential uses in the future. Repository: rG LLVM Github Monorepo https

[PATCH] D58447: [clangd] Fix a crash in Selection

ilya-biryukov created this revision. ilya-biryukov added reviewers: kadircet, sammccall. Herald added subscribers: arphaman, jkorous, MaskRay, ioeric. Herald added a project: clang. The assertion checking that the range of the node is a token range does not hold in case of "split" tokens, e.g. bet

[PATCH] D58447: [clangd] Fix a crash in Selection

ilya-biryukov updated this revision to Diff 187569. ilya-biryukov added a comment. - Fix a syntax error in the test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58447/new/ https://reviews.llvm.org/D58447 Files: clang-tools-extra/clangd/Selectio

[PATCH] D58440: [clangd] Store index in '.clangd/index' instead of '.clangd-index'

ilya-biryukov updated this revision to Diff 187622. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Remove trailing slash from the path Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58440/new/ https://reviews.llvm.o

[PATCH] D58440: [clangd] Store index in '.clangd/index' instead of '.clangd-index'

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp:75 llvm::SmallString<128> CDBDirectory(Directory); -llvm::sys::path::append(CDBDirectory, ".clangd-index/"); +llvm::sys::path::append(CDBDirectory, ".clangd", "

[PATCH] D58440: [clangd] Store index in '.clangd/index' instead of '.clangd-index'

ilya-biryukov updated this revision to Diff 187627. ilya-biryukov added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. - Update .gitignore Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58440/new/ https://reviews.l

[PATCH] D58495: [clangd] Only report explicitly typed symbols during code navigation

ilya-biryukov added a reviewer: hokein. 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. Haojian is the author, so adding him as a reviewer. LGTM from my side Co

[PATCH] D58446: [CodeComplete] Collect visited contexts when scope specifier is invalid.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58446/new/ https://reviews.llvm.org/D58446 ___

[PATCH] D58492: [clangd] Add thread priority lowering for MacOS as well

ilya-biryukov added a reviewer: gribozavr. ilya-biryukov added a subscriber: gribozavr. ilya-biryukov added a comment. Not an expert on MacOS threading APIs, only checked this builds on Mac. @gribozavr could you confirm using these APIs is the right thing to do on Mac? Comment

[PATCH] D58492: [clangd] Add thread priority lowering for MacOS as well

ilya-biryukov added inline comments. Comment at: clangd/Threading.cpp:125 +#elif defined(__APPLE__) + // https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/getpriority.2.html + setpriority(PRIO_DARWIN_THREAD, 0,

[PATCH] D55250: [clangd] Enhance macro hover to see full definition

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.cpp:816 +// If the client supports Markdown, convert from plaintext here. +if (*H && HoverSupportsMarkdown) { + (*H)->contents.kind = MarkupKind::Markdown; ---

[PATCH] D55250: [clangd] Enhance macro hover to see full definition

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Thanks! Comment at: clangd/ClangdLSPServer.cpp:816 +// If the client supports Markdown, convert from plaintext here. +if (*H && Ho

[PATCH] D58525: [clangd] Don't attach FixIt to the source code in macro.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Thanks! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58525/new/ https://reviews.llvm.org/D58525 _

[PATCH] D58541: [CodeComplete] Propagate preferred type for function arguments in more cases

ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. Herald added a subscriber: jdoerfert. Herald added a project: clang. See the added test for some new cases. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D58541 Files: clang/include/clang/Parse/Pa

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

ilya-biryukov created this revision. ilya-biryukov added reviewers: malaperle, sammccall, ioeric. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, mgorny. Herald added a project: clang. That can render to markdown or plain text. Used for findHover requests. Repository: rG LLVM G

[PATCH] D55250: [clangd] Enhance macro hover to see full definition

ilya-biryukov added a comment. More concretely, here's the proposed API for the intermediate representation of formatted string: D58547 Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55250/new/ https://reviews.llvm.

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

ilya-biryukov added a comment. This is a follow-up for a discussion from D55250 . Still missing test, wanted to get some input on the API first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58547/new/ https://revi

[PATCH] D58607: Moved clangd docs to a separate directory in preparation to restructure them into multiple files

ilya-biryukov added inline comments. Comment at: clang-tools-extra/docs/clangd.rst:1 - -Clangd - +:orphan: Having this does page not seem useful in the long-run. Do we plan to remove it later? Or maybe there a way to create a redirect i

[PATCH] D58602: Removed an unhelpful comment in index.rst

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/D58602/new/ https://reviews.llvm.org/D58602 _

[PATCH] D58603: Updated the documentation build instructions for the current CMake build system

ilya-biryukov added inline comments. Comment at: clang-tools-extra/docs/README.txt:5 -Sphinx and doxygen documentation is generated by executing make. +You can generate documentation in HTML format from files in +clang-tools-extra/docs by building the docs-clang-tools-html targ

[PATCH] D58601: Fixed grammar in index.rst

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/D58601/new/ https://reviews.llvm.org/D58601 _

[PATCH] D58603: Updated the documentation build instructions for the current CMake build system

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/docs/README.txt:6 +You can generate documentation in HTML format from files in +clang-tools-extra/docs by building the docs-clang-tools-html target. --

[PATCH] D58607: Moved clangd docs to a separate directory in preparation to restructure them into multiple files

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/docs/clangd.rst:1 - -Clangd - +:orphan: gribozavr wrote: > ilya-biryukov wrote: >

[PATCH] D58603: Updated the documentation build instructions for the current CMake build system

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/D58603/new/ https://reviews.llvm.org/D58603 _

[PATCH] D58611: Fixed typos in tests: s/CHEKC/CHECK/

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/D58611/new/ https://reviews.llvm.org/D58611 _

[PATCH] D58612: Use std::atomic<> for static counters.

ilya-biryukov added a comment. In D58612#1408831 , @riccibruno wrote: > Hmm. These are not the only static variables which are used for statistics > (eg: in `DeclBase.cpp`). Would it make sense instead to keep all of the > statistics in the AST context (

[PATCH] D58612: Use std::atomic<> for static counters.

ilya-biryukov added a comment. In D58612#1408839 , @riccibruno wrote: > I don't know how hard it would be to do this, but I would like to argue that > this should be done even if it require some refactoring. These static > variables used for stats are ki

[PATCH] D58612: Use std::atomic<> for static counters.

ilya-biryukov added a comment. Awesome, I was also surprised it's so easy to convert them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58612/new/ https://reviews.llvm.org/D58612 ___ cfe-commits maili

[PATCH] D58612: Use std::atomic<> for static counters.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58612/new/ https://reviews.llvm.org/D58612 ___ cfe-commits mailing list cfe-comm

[PATCH] D58541: [CodeComplete] Propagate preferred type for function arguments in more cases

ilya-biryukov updated this revision to Diff 188225. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. - Check in the middle and at the end of identifiers. - Add a test for the second parameter and a class member function Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D58541: [CodeComplete] Propagate preferred type for function arguments in more cases

ilya-biryukov added inline comments. Comment at: clang/include/clang/Sema/Sema.h:294 + /// function_ref, clients should make sure all calls to get() with the same + /// location happen while function_ref is alive. + void enterFunctionArgument(SourceLocation Tok, --

[PATCH] D58541: [CodeComplete] Propagate preferred type for function arguments in more cases

ilya-biryukov updated this revision to Diff 188226. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Fix a typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58541/new/ https://reviews.llvm.org/D58541 Files: clang

[PATCH] D58541: [CodeComplete] Propagate preferred type for function arguments in more cases

ilya-biryukov added inline comments. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:494 default: assert(false && "unhnalded unary op"); return QualType(); xbolva00 wrote: > Typo? Fixed it. Thanks. Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D58556: [LibTooling] Add "smart" retrieval of AST-node source to FixIt library

ilya-biryukov added a comment. Thanks, the APIs totally make sense. And seem to fit into the other functions we have in `FixIt.h`, although the name of the file is somewhat misleading. Mostly comments about naming and one comment about the necessity of using matchers from my side. ===

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

ilya-biryukov added a comment. In https://reviews.llvm.org/D37101#853509, @rwols wrote: > After digging into VSCode, I now think that presenting snippets is the wrong > way forward, and I believe the right way is to implement the > `signatureHelpProvider` protocol for method/function parameters

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:321 + +// Fill in the label, detail, documentation and insertText fields of the +// CompletionItem. rwols wrote: > ilya-biryukov wrote: > > Maybe split writes into `Item.label` and oth

[PATCH] D37150: [clangd] Command line arg to specify compile_commands.json path

ilya-biryukov added inline comments. Comment at: clangd/GlobalCompilationDatabase.cpp:29 + +static cl::opt CompileCommands("compileCommands", + cl::desc("Start with absolute path to compile_commands.json")); Please place this fl

[PATCH] D37150: [clangd] Command line arg to specify compile_commands.json path

ilya-biryukov added a comment. BTW, if you're only looking for providing extra compilation flags for some files, you could use an extension to LSP that is already in clangd source tree. (See tests from https://reviews.llvm.org/D34947 for more details). But your change is also useful, as it allow

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

ilya-biryukov added a comment. This looks like a useful change even without prior changes to VSCode. Maybe add a command-line flag to clangd(`--enable-snippets`) and commit that? When snippets are disabled, we could simply do `insertText = /**/` after `ProcessChunks`, we will deprecate and remo

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

ilya-biryukov added a comment. Also wanted to stress once again that we need some tests for this change. https://reviews.llvm.org/D37101 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:418 +// for functions and methods, the return type. +assert(item.detail.empty() && "Unexpected extraneous CK_ResultType"); +Item.detail = Chunk.Text; Typo: should be `I

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:337 +// Fill in the kind field of the CompletionItem. +Item.kind = getKind(Result.CursorKind); + ilya-biryukov wrote: > Could we also set `Item.filterText` to completion item name? > S

[PATCH] D37382: Fixed a crash in code completion.

ilya-biryukov created this revision. The crash occured when FunctionDecl was parsed with an initializer. https://reviews.llvm.org/D37382 Files: lib/Parse/ParseDecl.cpp test/CodeCompletion/crash-func-init.cpp Index: test/CodeCompletion/crash-func-init.cpp ==

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:337 +// Fill in the kind field of the CompletionItem. +Item.kind = getKind(Result.CursorKind); + ilya-biryukov wrote: > ilya-biryukov wrote: > > Could we also set `Item.filterText` to

[PATCH] D37282: clangd: Tolerate additional headers

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Looks good and ready to land. Thanks for this change. Do you have commit rights to llvm repo? https://reviews.llvm.org/D37282 ___ c

[PATCH] D37282: clangd: Tolerate additional headers

ilya-biryukov added inline comments. Comment at: clangd/JSONRPCDispatcher.cpp:167 + // It's another header, ignore it. continue; +} else { puremourning wrote: > ilya-biryukov wrote: > > Maybe log the skipped header? > I think this would just be no

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