[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet marked 16 inline comments as done. kadircet added a comment. Thanks for also taking a look at the implementation, it is not complete yet. I am rather waiting for a green light on struct itself, so that I can write tests. Comment at: clang-tools-extra/clangd/XRefs.cpp:

[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet updated this revision to Diff 199257. kadircet marked 2 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61497/new/ https://reviews.llvm.org/D61497 Files: clang-tools-ex

[PATCH] D62010: [CodeComplete] Complete enumerators when preferred type is an enum

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang/test/CodeCompletion/enum-preferred-type.cpp:13 + if (color == N::Color::Red) {} + + // RUN: %clang_cc1 -fsyntax-only -code-completion-at

[PATCH] D62010: [CodeComplete] Complete enumerators when preferred type is an enum

kadircet added inline comments. Comment at: clang/test/CodeCompletion/enum-preferred-type.cpp:19 + // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:16 %s -o - | FileCheck %s + // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:21 %s -o - | FileCheck %s + //

[PATCH] D62135: [clangd] Turn no-parse-completion on by when preamble isn't ready. Add flag to force it.

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clangd/CodeComplete.h:128 +/// Always use text-based completion. +NeverParse, + } RunParser = ParseIfReady; Do we really have an

[PATCH] D61865: [clangd] improve help message for limit-results

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61865/new/ https://reviews.llvm.org/D61865 ___

[PATCH] D62238: [CodeComplete] Complete a lambda when preferred type is a function

kadircet added inline comments. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4135 +!PotentialTemplateName.getAsIdentifierInfo()->getName().contains( +"function")) + return nullptr; This looks cheesy, do we really want to perform this o

[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet updated this revision to Diff 200732. kadircet marked 5 inline comments as done. kadircet added a comment. - Added tests, and went over implementation. I am looking for input on: - Behavior on lambdas, currently it just keeps the old behaviour. I believe they should look more like func

[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:940 )cpp", - "class std::initializer_list", + "template<> class initializer_list {}", }, kadircet wrote: > sammccall wrote: > > hmm,

[PATCH] D61865: [clangd] improve help message for limit-results

kadircet added a comment. I can land it for you, you can learn more about commit access from https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61865/new/ https://reviews.llvm.org/D61865

[PATCH] D61865: [clangd] improve help message for limit-results

This revision was automatically updated to reflect the committed changes. Closed by commit rL361388: [clangd] improve help message for limit-results (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://rev

[PATCH] D62238: [CodeComplete] Complete a lambda when preferred type is a function

kadircet added inline comments. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4135 +!PotentialTemplateName.getAsIdentifierInfo()->getName().contains( +"function")) + return nullptr; ilya-biryukov wrote: > kadircet wrote: > > This looks

[PATCH] D59407: [clangd] Add RelationSlab

kadircet added a comment. Mostly LG from my side, thanks! Comment at: clang-tools-extra/clangd/index/Relation.cpp:19 +llvm::iterator_range +RelationSlab::lookup(const SymbolID &Subject, + index::SymbolRole Predicate) const { I would suggest

[PATCH] D62238: [CodeComplete] Complete a lambda when preferred type is a function

kadircet accepted this revision. kadircet marked an inline comment as done. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4142 + // Handle other cases. + if (T->isPointerType()) +T

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

kadircet added inline comments. Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:48 + return llvm::None; +} else { + DirectoryWatcher::Event Front = Events.front(); nit: get rid of the else Comment at: clang

[PATCH] D62288: [clangd-vscode] Do not customize uri converters in vscode

kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay. Herald added a project: clang. Clangd is already resolving symlinks on the server side, therefore there is no more need to handle it in client side. This was

[PATCH] D62288: [clangd-vscode] Do not customize uri converters in vscode

kadircet updated this revision to Diff 200909. kadircet marked an inline comment as done. kadircet added a comment. - Delete unusued import Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62288/new/ https://reviews.llvm.org/D62288 Files: clang-too

[PATCH] D62288: [clangd-vscode] Do not customize uri converters in vscode

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE361475: [clangd-vscode] Do not customize uri converters in vscode (authored by kadircet, committed by ). Changed prior to commit: https://reviews.llvm.org/D62288?vs=200909&id=200913#toc Repository:

[PATCH] D62298: [CodeComplete] Filter override completions by function name

kadircet added inline comments. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3154 +/// The result contains only 'typed text' and 'text' chunks. +static void PrintOverrideString(const CodeCompletionString &CCS, +CodeCompletionBuilder &Result) { -

[PATCH] D62303: [Index] Fix reported references in presence of template type aliases

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang/lib/Index/IndexTypeSourceInfo.cpp:140 + bool IsTypeAlias) { +if (ResolvedClass) { + //

[PATCH] D62298: [CodeComplete] Filter override completions by function name

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM Have one question though, does it improve behavior in vscode? Since label seems to be the same, it will most definitely improve clangd's ranking but vscode ignores it anyway.

[PATCH] D62298: [CodeComplete] Filter override completions by function name

kadircet added inline comments. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3172 +// Add a space after return type. +if (Chunk.Kind == CodeCompletionString::CK_ResultType) { + assert(!SeenTypedChunk); ilya-biryukov wrote: > kadircet wrote: > > do

[PATCH] D62372: [clangd] Limit the size of synthesized fix message

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang-tools-extra/clangd/Diagnostics.cpp:449 + + size_t FirstNewline = R.find('\n'); + if (FirstNewline != llvm::StringRef::npos)

[PATCH] D59407: [clangd] Add RelationSlab

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, except the duplication issue. Thanks! Comment at: clang-tools-extra/clangd/index/Relation.h:75 + private: +std::vector Relations; + }; maybe

[PATCH] D62391: [CodeComplete] Complete 'return true/false' in boolean functions

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

[PATCH] D62471: [clangd] SymbolCollector support for relations

kadircet added a comment. Can you also add some tests ? we have some tests in `unittests/SymbolCollectorTests.cpp` Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:298 + + processRelations(*ND, *ID, Relations); + why do we want to process these

[PATCH] D62459: [clangd] Serialization support for RelationSlab

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/Serialization.cpp:29 + +llvm::Expected symbolRoleToRelationKind(index::SymbolRole Role) { + // SymbolRole is used to record relations in the index. no need for errors, use `llvm_unreachab

[PATCH] D62487: [clang] Respect TerseOutput when printing lambdas

kadircet created this revision. kadircet added reviewers: ilya-biryukov, hokein, sammccall. Herald added a project: clang. Herald added a subscriber: cfe-commits. kadircet added a child revision: D61497: [clangd] Introduce a structured hover response. Repository: rG LLVM Github Monorepo https:

[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet updated this revision to Diff 201536. kadircet marked 10 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61497/new/ https://reviews.llvm.org/D61497 Files: clang-tools-e

[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet marked 10 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:567 +const HoverInfo Expected; + } // namespace + Cases[] = { sammccall wrote: > namespace?! > > might be a clang-form

[PATCH] D62487: [clang] Respect TerseOutput when printing lambdas

This revision was automatically updated to reflect the committed changes. Closed by commit rL361771: [clang] Respect TerseOutput when printing lambdas (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://r

[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet updated this revision to Diff 201543. kadircet marked 7 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61497/new/ https://reviews.llvm.org/D61497 Files: clang-tools-ex

[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet updated this revision to Diff 201628. kadircet added a comment. - clang-format Definition Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61497/new/ https://reviews.llvm.org/D61497 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp cla

[PATCH] D61497: [clangd] Introduce a structured hover response

This revision was automatically updated to reflect the committed changes. Closed by commit rL361803: [clangd] Introduce a structured hover response (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://revi

[PATCH] D62514: [CodeComplete] Set preferred type for qualified-id

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm. one irrelevant question though, it looks like most of the calls to `ResultBuilder::setPreferredType` performs a `!isNull` check before hand. should we perform this check imp

[PATCH] D59407: [clangd] Add RelationSlab

kadircet added a comment. btw, I believe you have enough good quality patches to apply for commit access. Are there any obstacles that is keeping you from applying? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59407/new/ https://reviews.llvm.org/

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

kadircet added a comment. Ping, D61497 has landed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61601/new/ https://reviews.llvm.org/D61601 ___ cfe-commits mailing list

[PATCH] D62573: [Index] Correctly set symbol kind of IndirectFieldDecl

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM thanks! Do you think it would be beneficial to add some logging for the default case of that switch statement ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang-tools-extra/clangd/XRefs.cpp:1170 else if (NamespaceScope->empty()) - Out << "global namespace"; + Output.appendText(" glob

[PATCH] D62579: [Index] Compute correct symbol kind for variable templates

kadircet added inline comments. Comment at: clang/lib/Index/IndexSymbol.cpp:99 + if (auto *VT = dyn_cast(D)) { +Info.Properties |= (SymbolPropertySet)SymbolProperty::Generic; what about function and class templates? Are they handled in somewhere else? Re

[PATCH] D62579: [Index] Compute correct symbol kind for variable templates

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/Index/IndexSymbol.cpp:99 + if (auto *VT = dyn_cast(D)) { +Info.Properties |= (SymbolPropertySet)SymbolProperty::Generic; ---

[PATCH] D55648: [CodeComplete] Fill preferred type on binary expressions

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. It seems like we have some assumptions about what people might be trying to do in most of the cases, but i think it is OK since this never performs worse than the older version. ===

[PATCH] D55650: [clangd] Fix an assertion failure in background index.

kadircet added a comment. I think the description is misleading, you are saying that we were triggering an assertion and you are fixing that. But in reality, we were not triggering any assertions, this patch is adding the assertions right? Comment at: clangd/index/Background.

[PATCH] D55649: [clangd] Enable cross-namespace completions by default in clangd

kadircet added inline comments. Comment at: clangd/tool/ClangdMain.cpp:144 "can insert scope qualifiers."), -cl::init(false), cl::Hidden); +cl::init(true)); why not keep it hidden ? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACT

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

kadircet updated this revision to Diff 178052. kadircet marked 6 inline comments as done. kadircet added a comment. - Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55315/new/ https://reviews.llvm.org/D55315 Files: clangd/Threadin

[PATCH] D55417: [clangd] Change disk-backed storage to be atomic

kadircet updated this revision to Diff 178059. kadircet marked 2 inline comments as done. kadircet added a comment. - Use creauteUniqueFile. - Don't remove on success. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55417/new/ https://reviews.llvm.org

[PATCH] D55649: [clangd] Enable cross-namespace completions by default in clangd

kadircet added inline comments. Comment at: clangd/tool/ClangdMain.cpp:144 "can insert scope qualifiers."), -cl::init(false), cl::Hidden); +cl::init(true)); ioeric wrote: > kadircet wrote: > > why not keep it hidden ? > We hid the flag because

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clangd/index/Background.h:121 bool ShouldStop = false; - std::deque Queue; + std::list> Tasks; std::vector ThreadPool; // FIXME: Abstract this away. ilya-biryukov wrote

[PATCH] D55648: [CodeComplete] Fill preferred type on binary expressions

kadircet added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:4928 +if (LHSType->isIntegralOrEnumerationType()) + return S.getASTContext().IntTy; +return QualType(); ilya-biryukov wrote: > kadircet wrote: > > why not LHSType ? > The shift

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

kadircet added inline comments. Comment at: clangd/index/Background.h:121 bool ShouldStop = false; - std::deque Queue; + std::list> Tasks; std::vector ThreadPool; // FIXME: Abstract this away. ilya-biryukov wrote: > kadircet wrote: > > ilya-biryukov wrote

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

kadircet updated this revision to Diff 178094. kadircet marked 2 inline comments as done. kadircet added a comment. - Use std::deque instead of std::list Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55315/new/ https://reviews.llvm.org/D55315 Files

[PATCH] D55650: [clangd] Fix an assertion failure in background index.

kadircet added a comment. Ah you are right, the Optional.. Thanks! Comment at: clangd/index/Background.cpp:384 Action->EndSourceFile(); + if (Clang->hasDiagnostics() && + Clang->getDiagnostics().hasUncompilableErrorOccurred()) { Then don't need to keep

[PATCH] D55650: [clangd] Fix an assertion failure in background index.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clangd/index/Background.cpp:384 Action->EndSourceFile(); + if (Clang->hasDiagnostics() && + Clang->getDiagnostics().hasUncompilableErro

[PATCH] D55702: [clangd] Fix memory leak in ClangdTests.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55702/new/ https://reviews.llvm.org/D55702 ___

[PATCH] D55705: [dexp] Change FuzzyFind to also print scope of symbols

kadircet created this revision. kadircet added a reviewer: hokein. Herald added subscribers: cfe-commits, arphaman, jkorous, ioeric, ilya-biryukov. When there are multiple symbols in the result of a fuzzy find with the same name, one has to perform an additional query to figure out which of those

[PATCH] D55705: [dexp] Change FuzzyFind to also print scope of symbols

This revision was automatically updated to reflect the committed changes. Closed by commit rL349152: [dexp] Change FuzzyFind to also print scope of symbols (authored by kadircet, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://re

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

kadircet updated this revision to Diff 178443. kadircet marked an inline comment as done. kadircet added a comment. - Revert name from Tasks to Queue Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55315/new/ https://reviews.llvm.org/D55315 Files:

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE349345: [clangd] Only reduce priority of a thread for indexing. (authored by kadircet, committed by ). Changed prior to commit: https://reviews.llvm.org/D55315?vs=178443&id=178445#toc Repository: r

[PATCH] D55417: [clangd] Change disk-backed storage to be atomic

kadircet updated this revision to Diff 178448. kadircet marked 3 inline comments as done. kadircet added a comment. - Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55417/new/ https://reviews.llvm.org/D55417 Files: clangd/index/Ba

[PATCH] D55417: [clangd] Change disk-backed storage to be atomic

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE349348: [clangd] Change diskbackedstorage to be atomic (authored by kadircet, committed by ). Changed prior to commit: https://reviews.llvm.org/D55417?vs=178448&id=178449#toc Repository: rCTE Clang

[PATCH] D55770: [clangd] BackgroundIndex rebuilds symbol index periodically.

kadircet added inline comments. Comment at: clangd/index/Background.cpp:464 + if (BuildIndexPeriodMs > 0) +SymbolsUpdatedSinceLastIndex = true; + else why not simply check if `BuildIndexPeriodMs` has passed and issue rebuild here? That way we won't spawn a

[PATCH] D55818: [clangd] Unify path canonicalizations in the codebase

kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ioeric. There were a few different places where we canonicalized paths, each one had its own flavor. This patch tries to unify them all under one place. Re

[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

kadircet marked 4 inline comments as done. kadircet added a comment. Sending out everything related to canonical path at: D55818 Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55224/new/ https://reviews.llvm.org/D55

[PATCH] D55770: [clangd] BackgroundIndex rebuilds symbol index periodically.

kadircet added inline comments. Comment at: clangd/index/Background.h:112 + const size_t BuildIndexPeriodMs; + std::atomic SymbolsUpdatedSinceLastIndex; + std::mutex IndexMu; We already have a mutex and cv, maybe get rid of this one signal the CV whenever we

[PATCH] D55770: [clangd] BackgroundIndex rebuilds symbol index periodically.

kadircet added inline comments. Comment at: clangd/index/Background.cpp:352 + std::unique_lock Lock(IndexMu); + if (ShouldStop) +break; Is double checking really necessary? I suppose it is for the case that we miss the notification, if that's t

[PATCH] D55770: [clangd] BackgroundIndex rebuilds symbol index periodically.

kadircet added inline comments. Comment at: clangd/index/Background.h:112 + const size_t BuildIndexPeriodMs; + std::atomic SymbolsUpdatedSinceLastIndex; + std::mutex IndexMu; kadircet wrote: > ioeric wrote: > > kadircet wrote: > > > We already have a mutex and

[PATCH] D55770: [clangd] BackgroundIndex rebuilds symbol index periodically.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clangd/index/Background.h:112 + const size_t BuildIndexPeriodMs; + std::atomic SymbolsUpdatedSinceLastIndex; + std::mutex IndexMu; --

[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

kadircet updated this revision to Diff 178675. kadircet marked 20 inline comments as done. kadircet added a comment. - Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55224/new/ https://reviews.llvm.org/D55224 Files: clangd/SourceC

[PATCH] D55818: [clangd] Unify path canonicalizations in the codebase

kadircet marked 7 inline comments as done. kadircet added inline comments. Comment at: clangd/SourceCode.h:91 +/// possible. +llvm::Expected getCanonicalPath(const FileEntry *F, + const SourceManager &SourceMgr); ilya-b

[PATCH] D55818: [clangd] Unify path canonicalizations in the codebase

kadircet updated this revision to Diff 178685. kadircet marked 2 inline comments as done. kadircet added a comment. - Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55818/new/ https://reviews.llvm.org/D55818 Files: clangd/SourceCo

[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

kadircet added inline comments. Comment at: clangd/index/Background.cpp:259 + // if this thread sees the older version but finishes later. This should + // be rare in practice. + DigestIt.first->second = Hash; ilya-biryukov wrote: > > "should be ra

[PATCH] D55818: [clangd] Unify path canonicalizations in the codebase

kadircet updated this revision to Diff 178856. kadircet marked 3 inline comments as done. kadircet added a comment. - Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55818/new/ https://reviews.llvm.org/D55818 Files: clangd/SourceCo

[PATCH] D55818: [clangd] Unify path canonicalizations in the codebase

This revision was automatically updated to reflect the committed changes. Closed by commit rL349618: [clangd] Unify path canonicalizations in the codebase (authored by kadircet, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://rev

[PATCH] D55885: [CodeComplete] Properly determine qualifiers of 'this' in a lambda

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55885/new/ https://reviews.llvm.org/D55885 ___ c

[PATCH] D56263: [clangd] Always try to build absolute path

kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ioeric. When canonicalizing paths do not look at tryGetRealPathName, which contains the resolved path for files that are symlinks. Instead first build the ab

[PATCH] D56263: [clangd] Always try to build absolute path

This revision was automatically updated to reflect the committed changes. Closed by commit rL350306: [clangd] Always try to build absolute path (authored by kadircet, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

kadircet updated this revision to Diff 180706. kadircet marked 23 inline comments as done. kadircet added a comment. Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55224/new/ https://reviews.llvm.org/D55224 Files: clangd/SourceCod

[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

kadircet added inline comments. Comment at: clangd/index/Background.cpp:259 + // if this thread sees the older version but finishes later. This should + // be rare in practice. + DigestIt.first->second = Hash; ilya-biryukov wrote: > kadircet wrote:

[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

kadircet updated this revision to Diff 180823. kadircet marked 8 inline comments as done. kadircet added a comment. Address comments && rebase Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55224/new/ https://reviews.llvm.org/D55224 Files: clangd/

[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

kadircet added a comment. There seems to be no unexpected changes after rebase Comment at: clangd/index/Background.cpp:259 + // if this thread sees the older version but finishes later. This should + // be rare in practice. + DigestIt.first->second = Hash; --

[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

kadircet updated this revision to Diff 180859. kadircet marked 12 inline comments as done. kadircet added a comment. Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55224/new/ https://reviews.llvm.org/D55224 Files: clangd/URI.cpp

[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

kadircet added inline comments. Comment at: clangd/index/Background.cpp:259 + // if this thread sees the older version but finishes later. This should + // be rare in practice. + DigestIt.first->second = Hash; ilya-biryukov wrote: > kadircet wrote:

[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

kadircet updated this revision to Diff 181044. kadircet marked 10 inline comments as done. kadircet added a comment. Address comments & revert last changes Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55224/new/ https://reviews.llvm.org/D55224 Fil

[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

kadircet updated this revision to Diff 181072. kadircet marked 2 inline comments as done. kadircet added a comment. Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55224/new/ https://reviews.llvm.org/D55224 Files: clangd/index/Back

[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE350847: [clangd] Introduce loading of shards within auto-index (authored by kadircet, committed by ). Changed prior to commit: https://reviews.llvm.org/D55224?vs=181072&id=181075#toc Repository: rC

[PATCH] D67651: [clangd] No ExtractFunction on empty selections.

kadircet added a comment. this one is actually specific to vardecl's and has a pending fix in D66872 I would rather wait for it to land, instead of adding another branch. WDYT? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D67748: [clangd] Add a helper for extracting nonlocal decls in a FunctionDecl

kadircet created this revision. kadircet added a reviewer: hokein. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. To be used by define-inline code action to determine whether the function/method body will still be valid

[PATCH] D65433: [clangd] DefineInline action availability checks

kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:78 +// function decl. Skips local symbols. +llvm::DenseSet getNonLocalDeclRefs(const FunctionDecl *FD, + ParsedAST &AST) { ---

[PATCH] D65433: [clangd] DefineInline action availability checks

kadircet updated this revision to Diff 220821. kadircet marked 17 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65433/new/ https://reviews.llvm.org/D65433 Files: clang-tools-e

[PATCH] D67825: [AST] Extract Decl::printNestedNameSpecifier helper from Decl::printQualifiedName

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, I know it is trivial, but some more unittests wouldn't hurt anyone :D Also could you update the summary to mention "doing string manipulations in the presence of template arguments i

[PATCH] D67826: [clangd] A helper to find explicit references and their names

kadircet added a comment. Thanks this mostly LG, but I think we need some tests feel free to copy the ones in D66647 as a start point. But I believe coverage for this one is bigger, as this tries to figure-out all references, whereas the define-inline patch onl

[PATCH] D65433: [clangd] DefineInline action availability checks

kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:183 +/// a.h: +/// void foo() { return ; } +/// hokein wrote: > kadircet wrote: > > hokein wrote: > > > now we get a potential ODR violation in this example, m

[PATCH] D65433: [clangd] DefineInline action availability checks

kadircet updated this revision to Diff 221270. kadircet marked 5 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65433/new/ https://reviews.llvm.org/D65433 Files: clang-tools-ex

[PATCH] D67748: [clangd] Add a helper for extracting nonlocal decls in a FunctionDecl

kadircet updated this revision to Diff 221272. kadircet marked an inline comment as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67748/new/ https://reviews.llvm.org/D67748 Files: clang-tools-ex

[PATCH] D67748: [clangd] Add a helper for extracting nonlocal decls in a FunctionDecl

kadircet added a comment. As discussed offline, we decided to change the file once we have more helpers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67748/new/ https://reviews.llvm.org/D67748 ___ cfe

[PATCH] D67651: [clangd] No ExtractFunction on empty selections.

kadircet added a comment. In D67651#1672489 , @hokein wrote: > fair enough, didn't notice that fix before, the testcase here probably > doesn't work after that patch. yes it shouldn't work after that one. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D67907: [clangd] Implement a smart version of HeaderSource switch.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:21 +// file. +class CollectIndexableLocalDecls { +public: this class doesn't seem to have any state(apart from saving AST in constructor and using it in call to collect),

[PATCH] D67748: [clangd] Add a helper for extracting nonlocal decls in a FunctionDecl

kadircet updated this revision to Diff 221313. kadircet added a comment. - Use findExplicitReferences instead of libIndex. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67748/new/ https://reviews.llvm.org/D67748 Files: clang-tools-extra/clangd/X

[PATCH] D65433: [clangd] DefineInline action availability checks

kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:183 +/// a.h: +/// void foo() { return ; } +/// hokein wrote: > kadircet wrote: > > hokein wrote: > > > kadircet wrote: > > > > hokein wrote: > > > > > now we

[PATCH] D65433: [clangd] DefineInline action availability checks

kadircet updated this revision to Diff 221314. kadircet marked 7 inline comments as done. kadircet added a comment. - Rebase on top of D67748 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65433/new/ https://reviews

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