[PATCH] D60116: [clang-format] Regroup #includes into blocks for Google style

ioeric created this revision. ioeric added reviewers: sammccall, klimek. Herald added a project: clang. Herald added a subscriber: cfe-commits. Regrouping #includes in blocks separated by blank lines when sorting C++ #include headers was implemented recently, and it has been preferred in Google's

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

ioeric updated this revision to Diff 193268. ioeric added a comment. - Rebase Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59811/new/ https://reviews.llvm.org/D59811 Files: clangd/ClangdServer.cpp clangd/CodeComplete.h clangd/TUScheduler.cpp

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jdoerfert, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. o Lex the code to get the identifiers and put them into a "symbol" index. o Adds a new completion

[PATCH] D60116: [clang-format] Regroup #includes into blocks for Google style

ioeric updated this revision to Diff 193446. ioeric added a comment. - Improved test. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60116/new/ https://reviews.llvm.org/D60116 Files: lib/Format/Format.cpp unittests/Format/CleanupTest.cpp unittests/Format/So

[PATCH] D60116: [clang-format] Regroup #includes into blocks for Google style

This revision was automatically updated to reflect the committed changes. Closed by commit rC357567: [clang-format] Regroup #includes into blocks for Google style (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D60116?vs=193446&id=193452#toc Repository:

[PATCH] D60199: [clang-format] Do not emit replacements while regrouping if Cpp includes are OK

ioeric added inline comments. Comment at: unittests/Format/SortIncludesTest.cpp:28 std::string sort(StringRef Code, std::vector Ranges, + llvm::Optional ExpectedNumRanges = llvm::None, StringRef FileName = "input.cc") { As

[PATCH] D60257: [Lookup] Invisible decls should not be ambiguous when renaming.

ioeric created this revision. ioeric added a reviewer: hokein. Herald added a project: clang. Herald added a subscriber: cfe-commits. For example, a renamed type in a header file can conflict with declaration in a random file that includes the header, but we should not consider the decl ambiguous

[PATCH] D60263: [clang-format] Preserve include blocks in ObjC Google style

ioeric added inline comments. Comment at: lib/Format/Format.cpp:787 GoogleStyle.IncludeStyle.IncludeIsMainRegex = "([-_](test|unittest))?$"; GoogleStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup; GoogleStyle.IndentCaseLabels = true;

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

ioeric added a comment. In D59376#1454834 , @ymandel wrote: > In D59376#1454768 , @ilya-biryukov > wrote: > > > Per @ioeric's suggestion: why not move the helper into > > `Tooling/Refactoring/ExtendedRange.h`? > >

[PATCH] D60316: [clangd] Include insertion: require header guards, drop other heuristics, treat .def like .inc.

ioeric added inline comments. Comment at: clangd/index/SymbolCollector.cpp:157 + return Canonical.str(); +else if (Canonical != Filename) + return toURI(SM, Canonical, Opts); nit: no need for `else`? Comment at: unittests/clangd/F

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

ioeric updated this revision to Diff 194129. ioeric marked 3 inline comments as done. ioeric added a comment. - split out the compile command change. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59811/new/ https://reviews.llvm.org/D59811 Files:

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

ioeric added inline comments. Comment at: clangd/TUScheduler.h:204 + Callback Action, + bool AllowFallback = false); ilya-biryukov wrote: > sammccall wrote: > > I think this isn't orthogonal to `PreambleConsistency`.

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

ioeric planned changes to this revision. ioeric added a comment. Oops, forgot to update tests... Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59811/new/ https://reviews.llvm.org/D59811 ___ cfe-commits

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

ioeric updated this revision to Diff 194132. ioeric added a comment. - Fix unit test Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59811/new/ https://reviews.llvm.org/D59811 Files: clangd/ClangdServer.cpp clangd/CodeComplete.h clangd/TUSchedu

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

ioeric added inline comments. Comment at: clangd/TUScheduler.cpp:881 + // asynchronous mode, as TU update should finish before this is run. + if (!It->second->Worker->isFirstPreambleBuilt() && + Consistency == StaleOrAbsent && PreambleTasks) { sammccall wr

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

ioeric updated this revision to Diff 194141. ioeric marked 11 inline comments as done. ioeric added a comment. - address review comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59811/new/ https://reviews.llvm.org/D59811 Files: clangd/Clangd

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

This revision was automatically updated to reflect the committed changes. Closed by commit rL357916: [clangd] Add fallback mode for code completion when compile command or preamble… (authored by ioeric, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Reposit

[PATCH] D60409: [clangd] Add -header-insertion=never flag to disable include insertion in code completion

ioeric added a comment. lg! Comment at: clangd/CodeComplete.cpp:1176 // This is available after Sema has run. - llvm::Optional Inserter; // Available during runWithSema. + llvm::Optional Inserter; // Optional during runWithSema. llvm::Optional FileProximity; // Initia

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

ioeric updated this revision to Diff 194311. ioeric marked 9 inline comments as done. ioeric added a comment. - address review comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60126/new/ https://reviews.llvm.org/D60126 Files: clangd/ClangdS

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

ioeric updated this revision to Diff 194315. ioeric added a comment. - minor cleanup Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60126/new/ https://reviews.llvm.org/D60126 Files: clangd/ClangdServer.cpp clangd/ClangdUnit.cpp clangd/CodeComp

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

ioeric added a comment. Thanks for the review! Comment at: clangd/CodeComplete.cpp:1320 +llvm::IntrusiveRefCntPtr VFS) && { +auto CompletionFilter = speculateCompletionFilter(Content, Pos); +if (!CompletionFilter) { sammccall wrote:

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Comment at: clangd/CodeComplete.cpp:1542 + assert(Offset <= Content.size()); + StringRef Suffix = Content.take_front(Offset); + CompletionPrefix Result; -

[PATCH] D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Feel free to land this. I'll rebase D60126 on this when it's in. Comment at: clangd/CodeComplete.cpp:580 + // Case 3: sema saw and resolve

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

ioeric updated this revision to Diff 194647. ioeric marked 14 inline comments as done. ioeric added a comment. - address review comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60126/new/ https://reviews.llvm.org/D60126 Files: clangd/Clangd

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

ioeric added inline comments. Comment at: clangd/CodeComplete.cpp:1360 getQueryScopes(Recorder->CCContext, *Recorder->CCSema, Opts); if (!QueryScopes.empty()) ScopeProximity.emplace(QueryScopes); sammccall wrote: > add this to the non-sema ca

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

ioeric updated this revision to Diff 194652. ioeric marked 2 inline comments as done. ioeric added a comment. - address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60126/new/ https://reviews.llvm.org/D60126 Files: clangd/ClangdServer.c

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE358159: [clangd] Use identifiers in file as completion candidates when build is not… (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D60126?vs=194652&id=194653#t

[PATCH] D59639: [clangd] Print template arguments helper

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang-tools-extra/clangd/AST.cpp:139 + // location information. + printTemplateArgumentList(OS, Cls->getTemplateArgs().asArray(), Policy); +} -

[PATCH] D60607: [clangd] Wait for compile command in ASTWorker instead of ClangdServer

ioeric created this revision. ioeric added reviewers: sammccall, ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, javed.absar. Herald added a project: clang. This makes addDocument non-blocking and would also allow code completion (in fallback mode) to r

[PATCH] D60316: [clangd] Include insertion: require header guards, drop other heuristics, treat .def like .inc.

ioeric requested changes to this revision. ioeric added a comment. This revision now requires changes to proceed. in case you missed this patch :) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60316/new/ https://reviews.llvm.org/D60316 __

[PATCH] D60257: [Lookup] Invisible decls should not be ambiguous when renaming.

ioeric added inline comments. Comment at: unittests/Tooling/LookupTest.cpp:215 + + // Potentially ambiguous symbols that are not visible at reference location + // are not considered ambiguous. hokein wrote: > The test seems hard to understand what it actually

[PATCH] D60257: [Lookup] Invisible decls should not be ambiguous when renaming.

ioeric updated this revision to Diff 194841. ioeric marked 2 inline comments as done. ioeric added a comment. - Add test comment. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60257/new/ https://reviews.llvm.org/D60257 Files: include/clang/Tooling/Core/Lookup.

[PATCH] D60607: [clangd] Wait for compile command in ASTWorker instead of ClangdServer

ioeric added inline comments. Comment at: clangd/TUScheduler.cpp:338 + Barrier(Barrier), Done(false) { + FileInputs.CompileCommand = CDB.getFallbackCommand(FileName); +} ilya-biryukov wrote: > The command is filled with a fallback by `ClangdServer`, right?

[PATCH] D60607: [clangd] Wait for compile command in ASTWorker instead of ClangdServer

ioeric updated this revision to Diff 194850. ioeric marked 7 inline comments as done. ioeric added a comment. - address review comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60607/new/ https://reviews.llvm.org/D60607 Files: clangd/ClangdS

[PATCH] D60607: [clangd] Wait for compile command in ASTWorker instead of ClangdServer

ioeric updated this revision to Diff 194851. ioeric added a comment. - Add missing comment to TUScheduler.h Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60607/new/ https://reviews.llvm.org/D60607 Files: clangd/ClangdServer.cpp clangd/ClangdSer

[PATCH] D60607: [clangd] Wait for compile command in ASTWorker instead of ClangdServer

ioeric updated this revision to Diff 194891. ioeric marked 5 inline comments as done. ioeric added a comment. - address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60607/new/ https://reviews.llvm.org/D60607 Files: clangd/ClangdServer.c

[PATCH] D60607: [clangd] Wait for compile command in ASTWorker instead of ClangdServer

ioeric added inline comments. Comment at: clangd/TUScheduler.cpp:338 + Barrier(Barrier), Done(false) { + FileInputs.CompileCommand = CDB.getFallbackCommand(FileName); +} ilya-biryukov wrote: > ioeric wrote: > > ilya-biryukov wrote: > > > The command is fill

[PATCH] D60607: [clangd] Wait for compile command in ASTWorker instead of ClangdServer

ioeric marked an inline comment as done. ioeric added inline comments. Comment at: clangd/TUScheduler.cpp:552 +const tooling::CompileCommand &ASTWorker::getCurrentCompileCommand() const { + return FileInputs.CompileCommand; +} ilya-biryukov wrote: > ioeric wrote

[PATCH] D60607: [clangd] Wait for compile command in ASTWorker instead of ClangdServer

ioeric marked an inline comment as done. ioeric added inline comments. Comment at: clangd/TUScheduler.cpp:552 +const tooling::CompileCommand &ASTWorker::getCurrentCompileCommand() const { + return FileInputs.CompileCommand; +} ilya-biryukov wrote: > ioeric wrote

[PATCH] D60607: [clangd] Wait for compile command in ASTWorker instead of ClangdServer

ioeric marked an inline comment as done. ioeric added inline comments. Comment at: clangd/TUScheduler.cpp:552 +const tooling::CompileCommand &ASTWorker::getCurrentCompileCommand() const { + return FileInputs.CompileCommand; +} ioeric wrote: > ilya-biryukov wrote

[PATCH] D60257: [Lookup] Invisible decls should not be ambiguous when renaming.

This revision was automatically updated to reflect the committed changes. Closed by commit rC358378: [Lookup] Invisible decls should not be ambiguous when renaming. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D60257?vs=194841&id=195103#toc Repository:

[PATCH] D60607: [clangd] Wait for compile command in ASTWorker instead of ClangdServer

ioeric updated this revision to Diff 195111. ioeric added a comment. - Only return compile command (instead of FileInputs) from AST worker. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60607/new/ https://reviews.llvm.org/D60607 Files: clangd/Cla

[PATCH] D60687: [clangd] Check file path of declaring header when deciding whether to insert include.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Previously, we would use include spelling of the declaring header to check whether the inserted header is th

[PATCH] D60607: [clangd] Wait for compile command in ASTWorker instead of ClangdServer

ioeric added inline comments. Comment at: unittests/clangd/ClangdTests.cpp:1148 + EXPECT_EQ(Res.Context, CodeCompletionContext::CCC_Recovery); + // Identifier-based fallback completion doesn't know about "symbol" scope. + EXPECT_THAT(Res.Completions, ilya-biry

[PATCH] D60607: [clangd] Wait for compile command in ASTWorker instead of ClangdServer

ioeric updated this revision to Diff 195137. ioeric marked 9 inline comments as done. ioeric added a comment. - address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60607/new/ https://reviews.llvm.org/D60607 Files: clangd/ClangdServer.c

[PATCH] D60607: [clangd] Wait for compile command in ASTWorker instead of ClangdServer

ioeric updated this revision to Diff 195139. ioeric marked 5 inline comments as done. ioeric added a comment. - use sync runAddDocument in test Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60607/new/ https://reviews.llvm.org/D60607 Files: clangd

[PATCH] D60607: [clangd] Wait for compile command in ASTWorker instead of ClangdServer

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE358400: [clangd] Wait for compile command in ASTWorker instead of ClangdServer (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D60607?vs=195139&id=195141#toc Re

[PATCH] D60687: [clangd] Check file path of declaring header when deciding whether to insert include.

ioeric marked an inline comment as done. ioeric added inline comments. Comment at: unittests/clangd/HeadersTests.cpp:208 TEST_F(HeadersTest, DoNotInsertIfInSameFile) { MainFile = testPath("main.h"); > IIUC, this already fixes the cases we'd seen of include-

[PATCH] D60687: [clangd] Check file path of declaring header when deciding whether to insert include.

This revision was automatically updated to reflect the committed changes. Closed by commit rL358496: [clangd] Check file path of declaring header when deciding whether to insert… (authored by ioeric, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository

[PATCH] D60815: [clangd] Recognize "don't include me directly" pattern, and suppress include insertion.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Comment at: clangd/index/SymbolCollector.cpp:631 + +bool SymbolCollector::isSelfContainedHeader(FileID FID) { + // The real computation (which will be memoized). --

[PATCH] D60815: [clangd] Recognize "don't include me directly" pattern, and suppress include insertion.

ioeric added inline comments. Comment at: clangd/index/SymbolCollector.cpp:631 + +bool SymbolCollector::isSelfContainedHeader(FileID FID) { + // The real computation (which will be memoized). sammccall wrote: > ioeric wrote: > > sammccall wrote: > > > this has b

[PATCH] D52078: [clangd] Store preamble macros in dynamic index.

ioeric added inline comments. Comment at: clangd/index/FileIndex.h:93 std::pair indexAST(ASTContext &AST, std::shared_ptr PP, + bool IndexMacros = false, sammccall wrote: > I'm concerned about the proliferation of parameters here. (Not new with this >

[PATCH] D52089: [clangd] Get rid of AST matchers in SymbolCollector. NFC

ioeric added a comment. In https://reviews.llvm.org/D52089#1235777, @malaperle wrote: > why? I wanted to get some numbers and update the patch summary, but somehow forgot. Sorry about that and thanks for asking! The AST matcher pops up in performance profile. Although not the most expensive

[PATCH] D52084: [clangd] NFC: Update documentation of Iterator's dump format

ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:96 /// ChildN is N-th iterator child. Raw iterators over PostingList are - /// represented as "[ID1, ID2, ..

[PATCH] D52089: [clangd] Get rid of AST matchers in SymbolCollector. NFC

This revision was automatically updated to reflect the committed changes. Closed by commit rL342362: [clangd] Get rid of AST matchers in SymbolCollector. NFC (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D52089 Files

[PATCH] D52047: [clangd] Add a "benchmark" for tracking memory

ioeric added a comment. The benchmark change looks fine to me. But I'm not very familiar with the trick, so I'll let Sam (who proposed the idea as you mentioned), stamp the patch. Comment at: clang-tools-extra/clangd/index/dex/Dex.h:75 +

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

ioeric updated this revision to Diff 165761. ioeric marked 4 inline comments as done. ioeric added a comment. - addressed review comments. Repository: rC Clang https://reviews.llvm.org/D52098 Files: include/clang/Index/IndexingAction.h lib/Index/IndexingAction.cpp unittests/CMakeLists.

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

ioeric added inline comments. Comment at: include/clang/Index/IndexingAction.h:44 bool IndexImplicitInstantiation = false; + bool IndexMacrosInPreprocessor = false; }; ilya-biryukov wrote: > Maybe add a comment or change a name to indicate that this currentl

[PATCH] D52222: [clangd] Merge ClangdServer::DynamicIndex into FileIndex. NFC.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. FileIndex now provides explicit interfaces for preamble and main file updates. This avoids growing parameter list when preamble and main sy

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

ioeric added inline comments. Comment at: unittests/Index/IndexTests.cpp:61 +S.Roles = Roles; +if (MI) + S.Info = getSymbolInfoForMacro(*MI); ilya-biryukov wrote: > ioeric wrote: > > ilya-biryukov wrote: > > > Can this actually happen? It seems weird

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL342451: [Index] Add an option to collect macros from preprocesor. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D52098 File

[PATCH] D52222: [clangd] Merge ClangdServer::DynamicIndex into FileIndex. NFC.

ioeric updated this revision to Diff 165918. ioeric marked 6 inline comments as done. ioeric added a comment. - addressed review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D5 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/index/FileIndex.cpp

[PATCH] D52222: [clangd] Merge ClangdServer::DynamicIndex into FileIndex. NFC.

ioeric added inline comments. Comment at: clangd/index/FileIndex.h:121 std::pair indexAST(ASTContext &AST, std::shared_ptr PP, llvm::ArrayRef URISchemes = {}); sammccall wrote: > indexPreamble would be clearer I think. What about `indexHeaderSymbols`?

[PATCH] D52222: [clangd] Merge ClangdServer::DynamicIndex into FileIndex. NFC.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE342460: [clangd] Merge ClangdServer::DynamicIndex into FileIndex. NFC. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D5?vs=165918&id=165923#toc Repository

[PATCH] D52222: [clangd] Merge ClangdServer::DynamicIndex into FileIndex. NFC.

This revision was automatically updated to reflect the committed changes. Closed by commit rL342460: [clangd] Merge ClangdServer::DynamicIndex into FileIndex. NFC. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D5

[PATCH] D52222: [clangd] Merge ClangdServer::DynamicIndex into FileIndex. NFC.

ioeric added inline comments. Comment at: clangd/index/FileIndex.h:77 + /// Update symbols from main file \p Path with symbols in \p TopLevelDecls. + void updateMain(PathRef Path, ASTContext &AST, + std::shared_ptr PP, sammccall wrote: > sammcc

[PATCH] D52078: [clangd] Store preamble macros in dynamic index.

ioeric updated this revision to Diff 165959. ioeric added a comment. - merge with origin/master Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52078 Files: clangd/index/FileIndex.cpp unittests/clangd/CodeCompleteTests.cpp unittests/clangd/FileIndexTests.cpp Index: unitte

[PATCH] D52084: [clangd] NFC: Update documentation of Iterator's dump format

ioeric accepted this revision. ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:66 + OS << "... "; if (Index != std::end(Documents)) + OS << *Index << " ..."; nit: should we drop the trailing `...` if Ind

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

ioeric updated this revision to Diff 165969. ioeric added a comment. - added lit test Repository: rC Clang https://reviews.llvm.org/D52079 Files: include/clang/Sema/CodeCompleteOptions.h lib/Sema/SemaCodeComplete.cpp test/Index/complete-pch-skip.cpp Index: test/Index/complete-pch-skip

[PATCH] D52078: [clangd] Store preamble macros in dynamic index.

ioeric added a comment. In https://reviews.llvm.org/D52078#1238301, @sammccall wrote: > Something not listed in cons: because macros aren't namespaced and we don't > have lots of signals, they can be really spammy. > Potentially, offering macros that aren't in the TU could be a loss even if >

[PATCH] D52047: [clangd] Add a "benchmark" for tracking memory

ioeric added inline comments. Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:69 +// This is not a *real* benchmark: it shows size of built MemIndex (in bytes). +// Same for the next "benchmark". The hack might not be obvious for other people

[PATCH] D52250: [clangd] expose MergedIndex class

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

[PATCH] D52233: [dexp] Allow users to dump JSON representations of fuzzy find requests

ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:131 } +if (ShowJSONRequest) + llvm::outs() << llvm::formatv("Request:\n{0}\n\n", toJSON(Request)); Any reason not to print this always? https://reviews.llvm

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

This revision was automatically updated to reflect the committed changes. Closed by commit rC342528: [Sema] Do not load macros from preamble when LoadExternal is false. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D52079?vs=165969&id=166077#toc Reposit

[PATCH] D52078: [clangd] Store preamble macros in dynamic index.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE342529: [clangd] Store preamble macros in dynamic index. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D52078?vs=165959&id=166078#toc Repository: rCTE Clang

[PATCH] D52264: Deduplicate replacements from diagnostics.

ioeric created this revision. ioeric added a reviewer: bkramer. Herald added a subscriber: cfe-commits. After r329813, clang-apply-replacements stopped deduplicating identical replacements; however, tools like clang-tidy relies on the deduplication of identical dignostics replacements from differe

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

ioeric added a comment. In https://reviews.llvm.org/D52273#1241281, @sammccall wrote: > This seems very clever, but extremely complicated - you've implemented much > of C++'s conversion logic, it's not clear to me which parts are actually > necessary to completion quality. > (Honestly this app

[PATCH] D52225: [clang] Implement Override Suggestions in Sema.

ioeric added a comment. Could you move the corresponding tests from clangd to sema? Comment at: lib/Sema/SemaCodeComplete.cpp:3063 +CodeCompletionString * +CodeCompletionResult::CreateCodeCompletionString(ASTContext &Ctx, + Prepro

[PATCH] D52300: [clangd] Implement VByte PostingList compression

ioeric added a comment. In https://reviews.llvm.org/D52300#1241754, @kbobyrev wrote: > Also, I'll refine https://reviews.llvm.org/D52047 a little bit and I believe > that is should be way easier to understand performance + memory consumption > once we have these benchmarks in. Both @ioeric and

[PATCH] D52357: [clangd] Force Dex to respect symbol collector flags

ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:27 +Token(Token::Kind::Sentinel, "Restricted For Code Completion"); +static const Token NotRestrictedForCodeCompletion = +Token(Token::Kind::Sentinel, "Not Restricted For Code Completion"

[PATCH] D52357: [clangd] Force Dex to respect symbol collector flags

ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:184 + // Filter symbols which are not indexed for code completion. + if (Req.RestrictForCodeCompletion) ---

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

ioeric added inline comments. Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:78 +static void DexQueries(benchmark::State &State) { + const auto Dex = buildDex(); Why did we move this benchmark (`DexQueries`)? It seems unnecessary. ===

[PATCH] D52364: [clangd] Initial supoprt for cross-namespace global code completion.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. When no scope qualifier is specified, allow completing index symbols from any scope and insert proper automatically. This is still experime

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

ioeric created this revision. ioeric added reviewers: sammccall, ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay. ioeric updated this revision to Diff 17. ioeric added a comment. - update comment The file stats can be reused when preamble is reused

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

ioeric updated this revision to Diff 17. ioeric added a comment. - update comment Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52419 Files: clangd/ClangdServer.cpp clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/CodeComplete.cpp clangd/CodeComplete.h unittest

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

ioeric added inline comments. Comment at: clangd/ClangdUnit.cpp:119 +/// Collect and cache all file status from the underlying file system. +class CollectStatCacheVFS : public vfs::FileSystem { Would it make sense to add a `clang::vfs::ProxyFS` that proxies cal

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

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

[PATCH] D52264: Deduplicate replacements from diagnostics.

This revision was automatically updated to reflect the committed changes. Closed by commit rL342951: Deduplicate replacements from diagnostics. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52264?vs=166112&id=166

[PATCH] D52264: Deduplicate replacements from diagnostics.

ioeric added a subscriber: bkramer. ioeric added a comment. I wasn't aware of the bug. I have just replied to the issue. Thanks for letting me know! Repository: rL LLVM https://reviews.llvm.org/D52264 ___ cfe-commits mailing list cfe-commits@list

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

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

[PATCH] D52455: [clangd] Check that scheme is valid when parsing URI.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52455 Files: clangd/URI.cpp unittests/clangd/URITests.cpp Index: uni

[PATCH] D52455: [clangd] Check that scheme is valid when parsing URI.

ioeric updated this revision to Diff 166839. ioeric added a comment. - lower case function name. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52455 Files: clangd/URI.cpp unittests/clangd/URITests.cpp Index: unittests/clangd/URITests.cpp =

[PATCH] D52455: [clangd] Check that scheme is valid when parsing URI.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE342961: [clangd] Check that scheme is valid when parsing URI. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D52455?vs=166839&id=166840#toc Repository: rCTE

[PATCH] D52462: [VFS] Add a proxy FS that delegates calls to underlying FS by default.

ioeric created this revision. ioeric added reviewers: ilya-biryukov, sammccall. Herald added a subscriber: cfe-commits. This is useful when derived file systems want to override some calls and still proxy other calls. Repository: rC Clang https://reviews.llvm.org/D52462 Files: include/clan

[PATCH] D52462: [VFS] Add a proxy FS that delegates calls to underlying FS by default.

ioeric updated this revision to Diff 166866. ioeric added a comment. - Make constructor explicit. Repository: rC Clang https://reviews.llvm.org/D52462 Files: include/clang/Basic/VirtualFileSystem.h Index: include/clang/Basic/VirtualFileSystem.h ===

[PATCH] D52462: [VFS] Add a proxy FS that delegates calls to underlying FS by default.

ioeric updated this revision to Diff 166865. ioeric added a comment. - Add an interface for getting the underlying FS from derived classes. Repository: rC Clang https://reviews.llvm.org/D52462 Files: include/clang/Basic/VirtualFileSystem.h Index: include/clang/Basic/VirtualFileSystem.h =

[PATCH] D52462: [VFS] Add a proxy FS that delegates calls to underlying FS by default.

This revision was automatically updated to reflect the committed changes. Closed by commit rC342976: [VFS] Add a proxy FS that delegates calls to underlying FS by default. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D52462?vs=166879&id=166882#toc Repo

[PATCH] D52462: [VFS] Add a proxy FS that delegates calls to underlying FS by default.

ioeric updated this revision to Diff 166879. ioeric added a comment. - remove `virtual` Repository: rC Clang https://reviews.llvm.org/D52462 Files: include/clang/Basic/VirtualFileSystem.h Index: include/clang/Basic/VirtualFileSystem.h =

[PATCH] D52503: [clangd] More precise index memory usage estimate

ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:251 Bytes += InvertedIndex.getMemorySize(); - for (const auto &P : InvertedIndex) -Bytes += P.second.bytes(); + for (const auto &TokenToPostingList : InvertedIndex) +Bytes += TokenTo

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

ioeric updated this revision to Diff 166951. ioeric marked 10 inline comments as done. ioeric added a comment. Herald added a subscriber: mgorny. - Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52419 Files: clangd/CMakeLists.txt clangd/ClangdServer.

<    5   6   7   8   9   10   11   12   13   14   >