[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] 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] 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] 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] 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] 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

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 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] 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 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] 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 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] 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 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 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: > 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] 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] 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] 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] 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] 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] 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] 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.

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 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] 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] 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] 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] 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 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] 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] 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] 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.

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 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 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 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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 added a comment. In D59811#1445701 , @ilya-biryukov wrote: > I think this option should be configurable (and off by default) for the > transition period. A few reasons to do so: > > - Before we have an actual implementation of fallback completions

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

ioeric updated this revision to Diff 192683. 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/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.

ioeric created this revision. ioeric added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, jdoerfert, kadircet, arphaman, jkorous, MaskRay, javed.absar. Herald added a project: clang. When calling TUScehduler::runWithPreamble (e.g. in code compleiton), allow entering a fallback

[PATCH] D59700: Make clang-move use same file naming convention as other tools

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59700/new/ https://reviews.llvm.org/D59700 ___ cfe-commits mailing list cfe-commits@l

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

ioeric added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:149 +} else { + // FIXME: Fix cases when getTypeAsWritten returns null, e.g. friend decls. + printTemplateArgumentList(OS, Cls->getTemplateArgs().asArray(), Policy); I'm not

[PATCH] D59683: [Tooling] Avoid working-dir races in AllTUsToolExecutor

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Neat! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59683/new/ https://reviews.llvm.org/D59683 __

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

ioeric added inline comments. Comment at: clang/lib/AST/TypePrinter.cpp:1640 + +static void printArgument(const TemplateArgumentLoc &A, + const PrintingPolicy &PP, llvm::raw_ostream &OS) { It's unclear to me what the new behavior is with

[PATCH] D59640: [clangd] Add TemplateArgumentList into Symbol

ioeric added a comment. should we update YAML? Comment at: clang-tools-extra/clangd/index/Symbol.h:48 + /// non-specializations. Example: "" + llvm::StringRef TemplateArgumentList; /// The location of the symbol's definition, if one was found. How about `

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

ioeric added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:88 static const TemplateArgumentList * getTemplateSpecializationArgs(const NamedDecl &ND) { if (auto *Func = llvm::dyn_cast(&ND)) can we unify this with `getTemplateSpecializationArgL

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

ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:33 +// If MergedSyms is provided, increments a symbols Reference count once for +// every file it was mentioned in. +void mergeRefs(llvm::ArrayRef> RefSlabs, nit: s/every file/

[PATCH] D59487: [Tooling] Add more scope specifiers until spelling is not ambiguous.

This revision was automatically updated to reflect the committed changes. ioeric marked an inline comment as done. Closed by commit rL356446: [Tooling] Add more scope specifiers until spelling is not ambiguous. (authored by ioeric, committed by ). Herald added a project: LLVM. Herald added a subsc

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

ioeric added a comment. > I don't follow why this can over-count, FileIndex keeps only one RefSlab per > each file, so we can't over-count? Did you mean it will be more than #TUs ? Yes. This is how `Symbol::References` is described in its documentation. If we want to change/expand the semantic,

[PATCH] D59487: [Tooling] Add more scope specifiers until spelling is not ambiguous.

ioeric marked 2 inline comments as done. ioeric added inline comments. Comment at: lib/Tooling/Core/Lookup.cpp:165 +if (UnspelledScopes.empty()) { + Disambiguated = "::" + Disambiguated; +} else { kadircet wrote: > maybe return or break afterwards? I

[PATCH] D59487: [Tooling] Add more scope specifiers until spelling is not ambiguous.

ioeric created this revision. ioeric added reviewers: kadircet, gribozavr. Herald added subscribers: cfe-commits, jdoerfert. Herald added a project: clang. Previously, when the renamed spelling is ambiguous, we simply use the full-qualfied name (with leading "::"). This patch makes it try adding a

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

ioeric added a comment. I'm not sure if FileIndex is the correct layer to make decision about how References is calculated. Currently, we have two use cases in clangd 1) one slab per TU and 2) one slab per file. It seems to me that we would want different strategies for the two use cases, so it

[PATCH] D59300: [clangd] Tune the fuzzy-matching algorithm

ioeric added inline comments. Comment at: clang-tools-extra/clangd/FuzzyMatch.cpp:285 + if (Pat[P] == Word[W] || + (WordRole[W] == Head && (IsPatSingleCase || PatRole[P] == Head))) ++S; ilya-biryukov wrote: > ioeric wrote: > > could you explain the int

[PATCH] D59300: [clangd] Tune the fuzzy-matching algorithm

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. (The result looks great) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59300/new/ https://reviews.llvm.org/D59300 ___

[PATCH] D59300: [clangd] Tune the fuzzy-matching algorithm

ioeric added inline comments. Comment at: clang-tools-extra/clangd/FuzzyMatch.cpp:74 static bool isAwful(int S) { return S < AwfulScore / 2; } -static constexpr int PerfectBonus = 3; // Perfect per-pattern-char score. +static constexpr int PerfectBonus = 4; // Perfect per-patter

[PATCH] D58345: [clangd] Using symbol name to map includes for STL symbols.

ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: clangd/index/CanonicalIncludes.cpp:111 - {"std::addressof", ""}, - // Map symbols in to their preferred includes. {"std::basic_filebuf", ""}

[PATCH] D58345: [clangd] Using symbol name to map includes for STL symbols.

ioeric added inline comments. Comment at: clangd/include-mapping/gen_std.py:46 +// +// Generated from cppreference offline HTML book v%s. +//===--===// I'd suggest dropping `v` prefix as it might

[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

ioeric added a comment. just a few drive-by comments ;) Comment at: clang-tools-extra/clangd/FindSymbols.h:28 +// https://github.com/Microsoft/language-server-protocol/issues/344 +SymbolKind indexSymbolKindToSymbolKind(index::SymbolKind Kind); + This could prob

[PATCH] D58345: [clangd] Using symbol name to map includes for STL symbols.

ioeric added inline comments. Comment at: clangd/StdGen/StdGen.py:2 +#!/usr/bin/env python +#===- StdGen.py - ---*- python -*--===# +# hokein wrote: > ioeric wrote: > > I'd avoid abbreviation in the file name and the new

[PATCH] D58345: [clangd] Using symbol name to map includes for STL symbols.

ioeric added inline comments. Comment at: clangd/StdGen/StdGen.py:2 +#!/usr/bin/env python +#===- StdGen.py - ---*- python -*--===# +# I'd avoid abbreviation in the file name and the new directory name. `StdGen` sounds a

[PATCH] D58977: [clang-tidy] Add the abseil-time-comparison check

ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: clang-tidy/abseil/TimeComparisonCheck.cpp:23 + auto Matcher = + binaryOperator(anyOf(hasOperatorName(">"), hasOperatorName(">="), +

[PATCH] D59086: [clangd] Adjust compile commands to be applicable for tooling

ioeric added inline comments. Comment at: clangd/GlobalCompilationDatabase.cpp:25 + // Clangd does not generate dependency file. + Cmd.CommandLine = tooling::getClangStripDependencyFileAdjuster()( + Cmd.CommandLine, Cmd.Filename); Please use `clang::toolin

[PATCH] D59086: [clangd] Adjust compile commands to be applicable for tooling

ioeric added inline comments. Comment at: clangd/GlobalCompilationDatabase.h:116 + /// Adjusts given compile command for clangd. + tooling::CompileCommand adjustArguments(tooling::CompileCommand Cmd) const; + This doesn't seem to be used in this patch (except f

[PATCH] D59083: [clangd] Store explicit template specializations in index for code navigation purposes

ioeric added a comment. > We need that information for providing better code navigation support, like > find references, children/base classes etc. It's not trivial how they make code navigation better. Can you explain and provide examples in the summary? Thanks! Comment at:

[PATCH] D58977: [clang-tidy] Add the abseil-time-comparison check

ioeric added inline comments. Comment at: clang-tidy/abseil/TimeComparisonCheck.cpp:23 + auto Matcher = + binaryOperator(anyOf(hasOperatorName(">"), hasOperatorName(">="), + hasOperatorName("=="), hasOperatorName("<="), `DurationCo

[PATCH] D58772: [clangd] Enable SuggestMissingIncludes by default.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE355200: [clangd] Enable SuggestMissingIncludes by default. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D58772?vs=188701&id=188894#toc Repository: rCTE Cla

[PATCH] D58782: Use ArrayRef::copy, instead of copying data manually

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Nice! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58782/new/ https://reviews.llvm.org/D58782 __

[PATCH] D58781: Added missing license headers

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lg. thanks!!! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58781/new/ https://reviews.llvm.org/D58781 __

[PATCH] D58778: Moved Ref into its own header and implementation file

ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/Ref.cpp:1 +#include "Ref.h" + License? Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:14 #include "CodeCompletionStrings.h" +#include "ExpectedTypes.h" #

[PATCH] D58774: Moved Symbol into its own header and implementation file

ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/Index.cpp:19 -llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, Symbol::SymbolFlag F) { - if (F == Symbol::None) -return OS << "None"; - std::string S; - if (F & Symbol::Deprecated) -S += "de

[PATCH] D58774: Moved Symbol into its own header and implementation file

ioeric added a comment. The design of Symbol and SymbolSlab are closely related. I would suggest putting them close together in the same library. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58774/new/ https://reviews.llvm.org/D58774 _

[PATCH] D58773: Moved SymbolOrigin into its own header and implementation file

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. In D58773#1413486 , @gribozavr wrote: > SymbolOrigin is used by itself, for example, in CodeComplete.h to define > `struct CodeCompletion`. Fair enou

[PATCH] D58773: Moved SymbolOrigin into its own header and implementation file

ioeric added a comment. I'm not sure if SymbolOrigin is interesting enough to be its own library. It's usually only used along with Symbol. Maybe we could pull a library Symbol.h instead and put SymbolOrigin by Symbol? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D58772: [clangd] Enable SuggestMissingIncludes by default.

ioeric created this revision. ioeric added a reviewer: kadircet. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. This seems to work stably now. Turn on by default. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D58

[PATCH] D56830: Prototype for include-fixer workflow in clangd. [NOT FOR REVIEW]

ioeric abandoned this revision. ioeric added a comment. Herald added a subscriber: jdoerfert. Herald added a project: clang. Include-fixer has been landed in clangd. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56830/new/ https://reviews.llvm.org/D

[PATCH] D58768: Moved SymbolLocation into its own header and implementation file

ioeric added a comment. I think another option here is to move Symbol+SymbolLocations to a Symbol.h library. SymbolLocation is often used along with Symbol. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58768/new/ https://reviews.llvm.org/D58768

[PATCH] D58448: [clangd] Improve global code completion when scope specifier is unresolved.

This revision was automatically updated to reflect the committed changes. Closed by commit rL354963: [clangd] Improve global code completion when scope specifier is unresolved. (authored by ioeric, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository:

[PATCH] D58448: [clangd] Improve global code completion when scope specifier is unresolved.

ioeric updated this revision to Diff 188512. ioeric added a comment. - Rebase and minor cleanup Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58448/new/ https://reviews.llvm.org/D58448 Files: clangd/CodeComplete.cpp unittests/clangd/CodeComplet

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL354570: [CodeComplete] Collect visited contexts when scope specifier is invalid. (authored by ioeric, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: r

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

ioeric updated this revision to Diff 187756. ioeric added a comment. - Cleanup unit tests (forgot to run... sorry) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58446/new/ https://reviews.llvm.org/D58446 Files: lib/Sema/SemaCodeComplete.cpp unittests/Sema/Co

[PATCH] D58448: [clangd] Improve global code completion when scope specifier is unresolved.

ioeric created this revision. ioeric added reviewers: ilya-biryukov, sammccall. Herald added subscribers: cfe-commits, jdoerfert, kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Suppose `clangd::` is unresolved in the following example. Currently, we simply use "clangd::" as

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

ioeric created this revision. ioeric added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, jdoerfert. Herald added a project: clang. This will allow completion consumers to guess the specified scope by putting together scopes in the context with the specified scope (e.g. when the

[PATCH] D58185: [clangd] Handle unresolved scope specifier when fixing includes.

This revision was automatically updated to reflect the committed changes. Closed by commit rL354330: [clangd] Handle unresolved scope specifier when fixing includes. (authored by ioeric, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D58185: [clangd] Handle unresolved scope specifier when fixing includes.

ioeric updated this revision to Diff 187359. ioeric added a comment. - minor cleanup Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58185/new/ https://reviews.llvm.org/D58185 Files: clangd/IncludeFixer.cpp unittests/clangd/DiagnosticsTests.cpp

[PATCH] D58185: [clangd] Handle unresolved scope specifier when fixing includes.

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

[PATCH] D58185: [clangd] Handle unresolved scope specifier when fixing includes.

ioeric added inline comments. Comment at: clangd/IncludeFixer.cpp:191 +//~~ +llvm::Optional qualifiedByUnresolved(llvm::StringRef Code, + size_t Offset) { sammccall wrote: > this isn't wrong per se

  1   2   3   4   5   6   7   8   9   10   >