[PATCH] D37618: Use CommonOptionsParser in clang-refactor

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Lgtm Repository: rL LLVM https://reviews.llvm.org/D37618 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. I think this is ready to go. @klimek Manuel, are all your concerns addressed? Comment at: tools/clang-refactor/ClangRefactor.cpp:62 + /// \returns true if an error

[PATCH] D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action

ioeric added inline comments. Comment at: tools/clang-refactor/ClangRefactor.cpp:29 +using namespace tooling; +using namespace clang_refactor; +namespace cl = llvm::cl; Sorry for haven't noticed this earlier, but I think `clang::refactor` sounds like a better

[PATCH] D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action

ioeric added a comment. Feel free to land the patch now if comments are addressed. Repository: rL LLVM https://reviews.llvm.org/D36574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action

ioeric added inline comments. Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:43 +class LocalRename : public RefactoringAction { + StringRef getCommand() const override { return "local-rename"; } + Shouldn't these be public?

[PATCH] D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action

ioeric added inline comments. Comment at: tools/clang-refactor/ClangRefactor.cpp:103 +IsSelectionParsed = true; +// FIXME: Support true selection ranges. +StringRef Value = *Selection; arphaman wrote: > ioeric wrote: > > Is the test selection

[PATCH] D37976: [docs][refactor] add refactoring engine design documentation

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Lgtm. Thanks for the documentation! Now we just need to try to keep it up-to-date ;) Comment at: docs/RefactoringEngine.rst:33 +operations (rules). These rules are grouped

[PATCH] D37681: [refactor] Simplify the interface and remove some template magic

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. @klimek Have you got a chance to take a look? Repository: rL LLVM https://reviews.llvm.org/D37681 ___ cfe-commits mailing list

[PATCH] D38402: [clang-refactor] Apply source replacements

ioeric added inline comments. Comment at: test/Refactor/tool-apply-replacements.cpp:6 + +class RenameMe { // CHECK: class { +}; Why is the result `class {`? Comment at: tools/clang-refactor/ClangRefactor.cpp:319 - // FIXME: Consume atomic

[PATCH] D36075: [refactor] Initial support for refactoring action rules

ioeric added a comment. Thanks for the changes! The code is much clearer. I am wondering if the current design could be extended to support tools (or rules) that use AST matchers? Or is the selection expected to be powerful enough to replace AST matchers? We have a few tools in

[PATCH] D36075: [refactor] Initial support for refactoring action rules

ioeric added inline comments. Comment at: include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h:26 +template +detail::SourceSelectionRequirement< +typename selection::detail::EvaluateSelectionChecker< arphaman wrote: > ioeric wrote: > >

[PATCH] D36075: [refactor] Initial support for refactoring action rules

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Thanks for the changes! Lgtm with a few nits. Comment at: include/clang/Tooling/Refactoring/RefactoringActionRules.h:33 +/// +/// - requiredSelection: The refactoring

[PATCH] D37210: [refactor] add a refactoring action rule that returns symbol occurrences

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Lgtm Repository: rL LLVM https://reviews.llvm.org/D37210 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes

ioeric added inline comments. Comment at: include/clang/Tooling/Refactoring/RefactoringResultConsumer.h:39 + /// Handles the source replacements that are produced by a refactoring action. + virtual void handle(AtomicChanges SourceReplacements) = 0; +};

[PATCH] D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. LGTM. @klimek Manuel, would you like to take a look? This addresses your comment https://reviews.llvm.org/D36075#854075 Comment at:

[PATCH] D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action

ioeric added inline comments. Comment at: include/clang/Tooling/Refactoring/RefactoringAction.h:20 + +class RefactoringAction { +public: Please add documentation for the class. I appreciate your RFC that explains all these concepts; it would be nice if you

[PATCH] D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes

ioeric added inline comments. Comment at: include/clang/Tooling/Refactoring/RefactoringResultConsumer.h:39 + /// Handles the source replacements that are produced by a refactoring action. + virtual void handle(AtomicChanges SourceReplacements) = 0; +}; I

[PATCH] D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action

ioeric added inline comments. Comment at: include/clang/Tooling/Refactoring/RefactoringActionRules.h:45 std::unique_ptr createRefactoringRule(Expected (*RefactoringFunction)( typename RequirementTypes::OutputType...), Can we get rid

[PATCH] D39042: [Tooling] Add a factory method for CommonOptionsParser

ioeric created this revision. This returns error instead of exiting the program in case of error. https://reviews.llvm.org/D39042 Files: include/clang/Tooling/CommonOptionsParser.h lib/Tooling/CommonOptionsParser.cpp Index: lib/Tooling/CommonOptionsParser.cpp

[PATCH] D34272: [Tooling] A new framework for executing clang frontend actions.

ioeric added inline comments. Comment at: include/clang/Tooling/CommonOptionsParser.h:90 /// - /// This constructor exits program in case of error. + /// If \p ExitOnError is set (default), This constructor exits program in case + /// of error; otherwise, this sets the

[PATCH] D34272: [Tooling] A new framework for executing clang frontend actions.

ioeric updated this revision to Diff 119447. ioeric added a comment. - Add a factory method for creating CommonOptionsParser. - Minor cleanup in CommonOptionsParser. - Revert CommonOptionsParser changes. - Merge branch 'option' into arcpatch-D34272 https://reviews.llvm.org/D34272 Files:

[PATCH] D34272: [Tooling] A new framework for executing clang frontend actions.

ioeric updated this revision to Diff 119449. ioeric marked 7 inline comments as done. ioeric added a comment. - Address review comments. https://reviews.llvm.org/D34272 Files: include/clang/Tooling/CommonOptionsParser.h include/clang/Tooling/Execution.h

[PATCH] D39043: [clang-rename] Rename alias.

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

[PATCH] D38982: [refactor] Initial outline of implementation of "extract function" refactoring

ioeric added a comment. Code looks good in general. I see there are a bunch of major features missing; it might make sense to print a warning or document the major missing features in the high-level API. Comment at:

[PATCH] D39042: [Tooling] Add a factory method for CommonOptionsParser

ioeric updated this revision to Diff 119465. ioeric marked 2 inline comments as done. ioeric added a comment. - Address review comments. https://reviews.llvm.org/D39042 Files: include/clang/Tooling/CommonOptionsParser.h lib/Tooling/CommonOptionsParser.cpp Index:

[PATCH] D39042: [Tooling] Add a factory method for CommonOptionsParser

ioeric added inline comments. Comment at: include/clang/Tooling/CommonOptionsParser.h:115 + + static llvm::Error init(int , const char **argv, + llvm::cl::OptionCategory , hokein wrote: > We can get rid of the `static` here by calling

[PATCH] D39086: Performance tracing facility for clangd.

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

[PATCH] D39086: Performance tracing facility for clangd.

ioeric added a comment. Looks good in general. A few nits. Comment at: clangd/JSONRPCDispatcher.cpp:230 - // Finally, execute the action for this JSON message. - if (!Dispatcher.call(JSONRef, Out)) -Out.log("JSON dispatch failed!\n"); + { +//

[PATCH] D39042: [Tooling] Add a factory method for CommonOptionsParser

ioeric updated this revision to Diff 119502. ioeric added a comment. - Move unique_ptr result. https://reviews.llvm.org/D39042 Files: include/clang/Tooling/CommonOptionsParser.h lib/Tooling/CommonOptionsParser.cpp Index: lib/Tooling/CommonOptionsParser.cpp

[PATCH] D38985: [refactor] Add support for editor commands that connect IDEs/editors to the refactoring actions

ioeric added inline comments. Comment at: include/clang/Tooling/Refactoring/RefactoringActionRule.h:58 + /// Returns the editor command that's was bound to this rule. + virtual const EditorCommand *getEditorCommand() { return nullptr; } }; arphaman wrote: >

[PATCH] D39120: [rename] Don't overwrite the template argument when renaming a template function.

ioeric added inline comments. Comment at: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp:227 +SourceLocation EndLoc = Expr->hasExplicitTemplateArgs() +? Expr->getLAngleLoc().getLocWithOffset(-1) +:

[PATCH] D39706: [refactor][extract] Initial implementation of variable captures

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Lg (Sorry for losing track of this and the delay!) Repository: rL LLVM https://reviews.llvm.org/D39706 ___ cfe-commits mailing list

[PATCH] D40548: [clangd] Prototyping index support and naive index-based global code completion. WIP

ioeric created this revision. Herald added subscribers: ilya-biryukov, mgorny. o Experimental interfaces to support use multiple index sources (e.g. AST index, global index) for code completion, cross-reference finding etc. o Replace sema code completion for qualified-id with index-based

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

ioeric created this revision. https://reviews.llvm.org/D40563 Files: include/clang/Sema/CodeCompleteConsumer.h lib/Sema/SemaCodeComplete.cpp Index: lib/Sema/SemaCodeComplete.cpp === --- lib/Sema/SemaCodeComplete.cpp +++

[PATCH] D40562: [Sema] Ignore decls in namespaces when global decls are not wanted.

ioeric created this revision. ... in qualified code completion and decl lookup. https://reviews.llvm.org/D40562 Files: lib/Sema/SemaCodeComplete.cpp lib/Sema/SemaLookup.cpp test/CodeCompletion/ignore-global-decls.cpp Index: test/CodeCompletion/ignore-global-decls.cpp

[PATCH] D40548: [clangd] Prototyping index support and naive index-based global code completion. WIP

ioeric added a comment. In https://reviews.llvm.org/D40548#937626, @malaperle wrote: > Hi Eric! As you might know I'm working on persisted indexing. I was wondering > which cases needed the index for code completion? Could you give a small > example? I thought the AST alone would be sufficient

[PATCH] D40406: [clangd] Switch from YAMLParser to JSONExpr

ioeric added inline comments. Comment at: clangd/JSONRPCDispatcher.cpp:221 +Out.log("<-- " + JSONRef + "\n"); +handleAllErrors(Doc.takeError(), [&](const llvm::ErrorInfoBase ) { + Out.log(llvm::Twine("JSON parse error: ") + Err.message() + "\n");

[PATCH] D40399: [clangd] Add missing (but documented!) JSONExpr typed accessors

ioeric added inline comments. Comment at: clangd/JSONExpr.h:368 +// Typed accessors return None/nullptr if the element has the wrong type. +llvm::Optional null(size_t I) const { + return (*this)[I].null(); Why is this needed? `v[x].null()` seems to

[PATCH] D40564: [clangd] Simplify common JSON-parsing patterns in Protocol.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Comment at: clangd/Protocol.cpp:56 +assert(*this && "Must check this is an object before calling parse()"); +if (const json::Expr *E = O->get(Prop)) { +

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

ioeric updated this revision to Diff 124739. ioeric marked 4 inline comments as done. ioeric added a comment. - Address review comments. https://reviews.llvm.org/D40563 Files: include/clang/Sema/CodeCompleteConsumer.h lib/Sema/SemaCodeComplete.cpp Index: lib/Sema/SemaCodeComplete.cpp

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

ioeric added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:4609 + if (SS.isInvalid()) { +CodeCompletionContext CC(CodeCompletionContext::CCC_Name); ilya-biryukov wrote: > ilya-biryukov wrote: > > Why do we alter this code path? > Maybe we

[PATCH] D40562: [Sema] Ignore decls in namespaces when global decls are not wanted.

ioeric updated this revision to Diff 124740. ioeric added a comment. - Clarify comment for includeGlobals() https://reviews.llvm.org/D40562 Files: include/clang/Sema/CodeCompleteConsumer.h lib/Sema/SemaCodeComplete.cpp lib/Sema/SemaLookup.cpp test/CodeCompletion/ignore-global-decls.cpp

[PATCH] D40548: [clangd] Prototyping index support and naive index-based global code completion. WIP

ioeric updated this revision to Diff 124745. ioeric added a comment. Herald added a subscriber: klimek. Merged with origin/master Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40548 Files: clangd/ASTIndex.cpp clangd/ASTIndex.h clangd/CMakeLists.txt

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

ioeric added a comment. In https://reviews.llvm.org/D40563#939555, @arphaman wrote: > Could you please add a test? Any tip on how this should be tested? I couldn't find any existing unit test for either SemaCodeComplete or code completion context (under `clang/unittests`). It might be

[PATCH] D40654: [clangd] Set completion options per-request.

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

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

ioeric added a comment. In https://reviews.llvm.org/D40563#939964, @arphaman wrote: > If nothing uses `getCXXScopeSpecifier` right now we can't really test it with > a clang or c-index-test regression test. A completion unit test could work > here. I don't think we actually have existing

[PATCH] D40399: [clangd] Add missing (but documented!) JSONExpr typed accessors

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Lgtm Thanks for the rename! https://reviews.llvm.org/D40399 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D40562: [Sema] Ignore decls in namespaces when global decls are not wanted.

ioeric added a comment. In https://reviews.llvm.org/D40562#941570, @arphaman wrote: > In https://reviews.llvm.org/D40562#940201, @ilya-biryukov wrote: > > > In https://reviews.llvm.org/D40562#939950, @arphaman wrote: > > > > > This change breaks cached completions for declarations in namespaces

[PATCH] D40780: [clangd] Incorporate fuzzy-match into result rankings.

ioeric added inline comments. Comment at: clangd/ClangdUnit.cpp:377 + : Result(), SymbolScore(score(Result)), FilterScore(FilterScore), +Score(FilterScore * SymbolScore) {} It might worth mentioning how well `FilterScore * SymbolScore` performs. I

[PATCH] D40562: [Sema] Ignore decls in namespaces when global decls are not wanted.

ioeric updated this revision to Diff 126694. ioeric added a comment. - Merged with origin/master Repository: rC Clang https://reviews.llvm.org/D40562 Files: include/clang/Driver/CC1Options.td include/clang/Sema/CodeCompleteConsumer.h include/clang/Sema/CodeCompleteOptions.h

[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.

ioeric updated this revision to Diff 126686. ioeric added a comment. Merged with origin/master and moved index-related files to index/ subdirectory. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40548 Files: clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp

[PATCH] D40562: [Sema] Ignore decls in namespaces when global decls are not wanted.

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL320563: [Sema] Ignore decls in namespaces when global decls are not wanted. (authored by ioeric, committed by ). Changed

[PATCH] D41232: [clangd] Add a FileSymbols container that manages symbols from multiple files.

This revision was automatically updated to reflect the committed changes. Closed by commit rL320701: [clangd] Add a FileSymbols container that manages symbols from multiple files. (authored by ioeric, committed by ). Repository: rL LLVM https://reviews.llvm.org/D41232 Files:

[PATCH] D41232: [clangd] Add a FileSymbols container that manages symbols from multiple files.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, ilya-biryukov, mgorny, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41232 Files: clangd/CMakeLists.txt clangd/index/FileSymbols.cpp clangd/index/FileSymbols.h

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

ioeric added inline comments. Comment at: clangd/index/Index.h:52 +private: + friend class llvm::DenseMapInfo; + warning: class 'DenseMapInfo' was previously declared as a struct [-Wmismatched-tags] Repository: rCTE Clang Tools Extra

[PATCH] D40548: [clangd] Symbol index interfaces and an in-memory index implementation.

ioeric added a comment. In https://reviews.llvm.org/D40548#955289, @malaperle wrote: > In https://reviews.llvm.org/D40548#947081, @ioeric wrote: > > > Hi Marc, the patch is not ready for review yet. I am still cleaning up the > > prototype and will let you know when it's ready for review. > > >

[PATCH] D41232: [clangd] Add a FileSymbols container that manages symbols from multiple files.

ioeric updated this revision to Diff 126944. ioeric added a comment. - fix HEADER_GUARD Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41232 Files: clangd/CMakeLists.txt clangd/index/FileSymbols.cpp clangd/index/FileSymbols.h unittests/clangd/CMakeLists.txt

[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.

ioeric updated this revision to Diff 126916. ioeric marked 7 inline comments as done. ioeric added a comment. - Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40548 Files: clangd/CMakeLists.txt clangd/index/Index.h clangd/index/MemIndex.cpp

[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.

ioeric updated this revision to Diff 126919. ioeric marked 3 inline comments as done. ioeric added a comment. - Address more comments. # I am going to land it now. - Merge remote-tracking branch 'origin/master' into index Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40548

[PATCH] D41345: [clangd] Add more symbol information for code completion.

ioeric created this revision. ioeric added reviewers: hokein, sammccall. Herald added subscribers: cfe-commits, ilya-biryukov, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41345 Files: clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/index/FileIndex.cpp

[PATCH] D41367: [clangd] Support filtering by fixing scopes in fuzzyFind.

ioeric updated this revision to Diff 127409. ioeric added a comment. - minor cleanup. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41367 Files: clangd/index/Index.h clangd/index/MemIndex.cpp unittests/clangd/IndexTests.cpp Index: unittests/clangd/IndexTests.cpp

[PATCH] D41367: [clangd] Support filtering by fixing scopes in fuzzyFind.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, ilya-biryukov, klimek. When scopes are specified, only match symbols from scopes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41367 Files: clangd/index/Index.h

[PATCH] D41281: [clangd] Index-based code completion.

ioeric added a comment. Thanks for the review! Comment at: clangd/CodeComplete.cpp:847 + if (Opts.Index && SCInfo.SSInfo) { +// FIXME: log warning with logger if sema code completion have collected +// results. ilya-biryukov wrote: > NIT:

[PATCH] D41281: [clangd] Index-based code completion.

ioeric updated this revision to Diff 127406. ioeric marked 21 inline comments as done. ioeric added a comment. - Address comments in CodeComplete.cpp - Split index changes into a separate patch. - Merged with patch https://reviews.llvm.org/D41351. - Remove Position.* files. Repository: rCTE

[PATCH] D41367: [clangd] Support filtering by fixing scopes in fuzzyFind.

ioeric updated this revision to Diff 127484. ioeric marked 3 inline comments as done. ioeric added a comment. - Address a few more comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41367 Files: clangd/index/Index.h clangd/index/MemIndex.cpp

[PATCH] D41367: [clangd] Support filtering by fixing scopes in fuzzyFind.

ioeric updated this revision to Diff 127486. ioeric added a comment. - Minor cleanup Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41367 Files: clangd/index/Index.h clangd/index/MemIndex.cpp clangd/index/SymbolCollector.cpp clangd/index/SymbolYAML.cpp

[PATCH] D41367: [clangd] Support filtering by fixing scopes in fuzzyFind.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE321067: [clangd] Support filtering by fixing scopes in fuzzyFind. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D41367?vs=127486=127487#toc Repository:

[PATCH] D41281: [clangd] Index-based code completion.

ioeric updated this revision to Diff 127488. ioeric added a comment. - Merged with origin/master - Merged with https://reviews.llvm.org/D41351 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41281 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h

[PATCH] D41289: [clangd] Build dynamic index and use it for code completion.

ioeric updated this revision to Diff 127493. ioeric added a comment. - Merge with updated https://reviews.llvm.org/D41281 - Fix broken merge Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41289 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h

[PATCH] D41281: [clangd] Index-based code completion.

ioeric updated this revision to Diff 127509. ioeric added a comment. - Fixed a bug when completing scope that starts with '::'. - Diff base on origin/master Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41281 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h

[PATCH] D41281: [clangd] Index-based code completion.

This revision was automatically updated to reflect the committed changes. Closed by commit rL321083: [clangd] Index-based code completion. (authored by ioeric, committed by ). Repository: rL LLVM https://reviews.llvm.org/D41281 Files: clang-tools-extra/trunk/clangd/CodeComplete.cpp

[PATCH] D41289: [clangd] Build dynamic index and use it for code completion.

ioeric updated this revision to Diff 127544. ioeric added a comment. - Merge with https://reviews.llvm.org/D41289. - Merge with origin/master Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41289 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h

[PATCH] D41281: [clangd] Index-based code completion.

ioeric updated this revision to Diff 127536. ioeric marked an inline comment as done. ioeric added a comment. - Move implementations around to make code easier to read. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41281 Files: clangd/CodeComplete.cpp

[PATCH] D41281: [clangd] Index-based code completion.

ioeric updated this revision to Diff 127538. ioeric added a comment. - Add a FIXME for Index in code completion options. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41281 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/index/FileIndex.cpp

[PATCH] D41281: [clangd] Index-based code completion.

ioeric added inline comments. Comment at: clangd/CodeComplete.h:64 + + // Populated internally by clangd, do not set. + /// If `Index` is set, it is used to augment the code completion ilya-biryukov wrote: > sammccall wrote: > > ilya-biryukov wrote: > > >

[PATCH] D41289: [clangd] Build dynamic index and use it for code completion.

ioeric updated this revision to Diff 127554. ioeric marked 4 inline comments as done. ioeric added a comment. - Address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41289 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h

[PATCH] D41289: [clangd] Build dynamic index and use it for code completion.

ioeric added a comment. Thanks for the quick review! Comment at: clangd/ClangdUnit.cpp:617 + new CppFile(FileName, std::move(Command), StorePreamblesInMemory, + std::move(PCHs), std::move(ASTCallback))); } sammccall wrote: > CppFile

[PATCH] D41276: [clangd] Build in-memory index on symbols in files.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, ilya-biryukov, mgorny, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41276 Files: clangd/CMakeLists.txt clangd/index/FileMemIndexManager.cpp

[PATCH] D41281: [clangd] Index-based code completion.

ioeric updated this revision to Diff 127118. ioeric added a comment. - Merge remote-tracking branch 'origin/master' into index-completion - Fix merge with origin/master. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41281 Files: clangd/CMakeLists.txt

[PATCH] D41289: [clangd] Build dynamic index and use it for code completion.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, ilya-biryukov, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41289 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp

[PATCH] D41281: [clangd] Index-based code completion.

ioeric created this revision. ioeric added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, mgorny, klimek. Use symbol index to populate completion results for qualfified IDs e.g. "nx::A^". Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41281 Files:

[PATCH] D41276: [clangd] Build in-memory index on symbols in files.

ioeric added a comment. In https://reviews.llvm.org/D41276#956513, @sammccall wrote: > I don't see `FileSymbols.{h,cpp}` being deleted, but maybe I'm bad at reading > diffs. No, I'm bad at creating diffs... Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41276

[PATCH] D41276: [clangd] Build in-memory index on symbols in files.

ioeric updated this revision to Diff 127098. ioeric marked 2 inline comments as done. ioeric added a comment. - Removed unused files and addressed comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41276 Files: clangd/CMakeLists.txt clangd/index/FileIndex.cpp

[PATCH] D41276: [clangd] Build in-memory index on symbols in files.

ioeric updated this revision to Diff 127086. ioeric added a comment. - Address review comments. Merge FileSymbols and tests into FileIndex. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41276 Files: clangd/CMakeLists.txt clangd/index/FileIndex.cpp

[PATCH] D41276: [clangd] Build in-memory index on symbols in files.

ioeric updated this revision to Diff 127087. ioeric added a comment. - Minor cleanup Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41276 Files: clangd/CMakeLists.txt clangd/index/FileIndex.cpp clangd/index/FileIndex.h unittests/clangd/CMakeLists.txt

[PATCH] D41276: [clangd] Build in-memory index on symbols in files.

This revision was automatically updated to reflect the committed changes. Closed by commit rL320807: [clangd] Build in-memory index on symbols in files. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D41276?vs=127098=127099#toc Repository: rL LLVM

[PATCH] D40548: [clangd] Symbol index interfaces and an in-memory index implementation.

ioeric added inline comments. Comment at: clangd/index/Index.h:134 + virtual bool + fuzzyFind(Context , const FuzzyFindRequest , +std::function Callback) const = 0; malaperle wrote: > ioeric wrote: > > malaperle wrote: > > > I

[PATCH] D41232: [clangd] Add a FileSymbols container that manages symbols from multiple files.

ioeric updated this revision to Diff 126943. ioeric marked 2 inline comments as done. ioeric added a comment. - Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41232 Files: clangd/CMakeLists.txt clangd/index/FileSymbols.cpp

[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.

ioeric updated this revision to Diff 126775. ioeric added a comment. - Remove everything except for SymbolIndex interface and MemIndex - Added unit tests for MemIndex Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40548 Files: clangd/CMakeLists.txt clangd/index/Index.cpp

[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.

ioeric updated this revision to Diff 126917. ioeric marked an inline comment as done. ioeric added a comment. - Fix comment for fuzzyFind Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40548 Files: clangd/CMakeLists.txt clangd/index/Index.h clangd/index/MemIndex.cpp

[PATCH] D40548: [clangd] Symbol index interfaces and an in-memory index implementation.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE320688: [clangd] Symbol index interfaces and an in-memory index implementation. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D40548?vs=126919=126924#toc

[PATCH] D39050: Add index-while-building support to Clang

ioeric added a comment. Thanks a lot for further cleaning up the patch! It is now much easier to review. I really appreciate it! Some more comments on the public APIs and the layering of classes. There are a lot of helper classes in the implementation, so I think it's important to get a clear

[PATCH] D39050: Add index-while-building support to Clang

ioeric added inline comments. Comment at: include/clang/Index/IndexUnitDataConsumer.h:1 +//===--- IndexUnitDataConsumer.h - Abstract index unit data consumer ---===// +// ioeric wrote: > IIUC, this is the index data for a translation unit, as

[PATCH] D41289: [clangd] Build dynamic index and use it for code completion.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE321092: [clangd] Build dynamic index and use it for code completion. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D41289?vs=127554=127555#toc Repository:

[PATCH] D41432: [clangd] Switch xrefs and documenthighlight to annotated-code unit tests. NFC

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm! This is amazing! Comment at: unittests/clangd/Annotations.h:12 +// +//Annotations Example(R"cpp( +// int complete() { x.pri^ } // ^ indicates a

[PATCH] D41367: [clangd] Support filtering by fixing scopes in fuzzyFind.

ioeric updated this revision to Diff 127478. ioeric marked 3 inline comments as done. ioeric added a comment. - Address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41367 Files: clangd/index/Index.h clangd/index/MemIndex.cpp

[PATCH] D41367: [clangd] Support filtering by fixing scopes in fuzzyFind.

ioeric added a comment. Thanks for the review! Comment at: clangd/index/Index.h:127 /// \brief A query string for the fuzzy find. This is matched against symbols' - /// qualfified names. + /// qualified names. If any scope below is provided, \p Query is only matched +

[PATCH] D39050: Add index-while-building support to Clang

ioeric added a comment. >> - I think the implementation of the index output logic (e.g. >> `IndexUnitWriter` and bit format file) can be abstracted away (and split >> into separate patches) so that you can unit-test the action with a >> custom/mock unit writer; this would also make the action

[PATCH] D39706: [refactor][extract] Initial implementation of variable captures

ioeric added inline comments. Comment at: lib/Tooling/Refactoring/Extract/CaptureVariables.cpp:97 + } + llvm_unreachable("invalid kind!"); +} A more informative message would be better. Comment at:

[PATCH] D38425: [clangd] Document highlights for clangd

ioeric added a comment. Drive-by comment: in general, have you considered reusing the existing declarations and occurrences finding functionalities in clang-rename? AFAIK, it deals with templates and macros pretty well. o

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