[PATCH] D97738: [clangd] Move DraftStore from ClangdLSPServer into ClangdServer.

kadircet added a comment. as discussed offline, i am hesitant about `validateEdits` layering at the moment. but i suppose the best thing to do is move it into clangdserver and expose a structrual api, as you proposed, which can be done independently. Comment at: clang-tools-e

[PATCH] D97773: [clangd] Make WorkspaceSymbols request work with empty queries

kadircet created this revision. kadircet added a reviewer: hokein. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Clangd uses codecompletion limit as the limit

[PATCH] D97738: [clangd] Move DraftStore from ClangdLSPServer into ClangdServer.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/clangd/DraftStore.cpp:66 // We treat versions as opaque, but the protocol says they increase. -if (*Version <= D.Vers

[PATCH] D97801: [clangd] Add `limit` extension on completion and workspace-symbols

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:784 Server->workspaceSymbols( - Params.query, Opts.CodeComplete.Limit, + Params.query, Params.li

[PATCH] D97773: [clangd] Make WorkspaceSymbols request work with empty queries

This revision was automatically updated to reflect the committed changes. Closed by commit rG188373fb4697: [clangd] Make WorkspaceSymbols request work with empty queries (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97773/new/

[PATCH] D97555: [clangd] Add diagnostic augmentation hook to Modules

kadircet added a comment. ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97555/new/ https://reviews.llvm.org/D97555 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D96245: [clangd] Propagate CodeActions in addition to Fixes for diagnostics

kadircet added a comment. ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96245/new/ https://reviews.llvm.org/D96245 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D97548: [clangd] Introduce client state invalidation

kadircet added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97548/new/ https://reviews.llvm.org/D97548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks (both for the patch and bearing with me :D), LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97535/new/ https://reviews.llvm.org/

[PATCH] D97548: [clangd] Introduce client state invalidation

kadircet updated this revision to Diff 328067. kadircet marked an inline comment as done. kadircet added a comment. - Rename method and update comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97548/new/ https://reviews.llvm.org/D97548 Files:

[PATCH] D97555: [clangd] Add diagnostic augmentation hook to Modules

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Module.h:97 + /// Called by auxilary threads as diagnostics encountered to generate fixes. + /// So this can be called concurrently from multiple threads. If a module sammccall wrote: > This

[PATCH] D97548: [clangd] Introduce client state invalidation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1d7b328198a7: [clangd] Introduce client state invalidation (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D97548?vs=328

[PATCH] D96245: [clangd] Propagate CodeActions in addition to Fixes for diagnostics

kadircet added a comment. In D96245#2602293 , @sammccall wrote: > Will the fixes/actions this enables really be available synchronously? The fixes stay as is today, but the action might be just a command that needs to be invoked by the client (so it is s

[PATCH] D97935: [clangd] Add config support for ResourceDir

kadircet created this revision. kadircet added a reviewer: hokein. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Repository: rG LLVM Github Monorepo https:/

[PATCH] D97950: [clangd] Rename Module -> FeatureModule to avoid confusion. NFC

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

[PATCH] D98029: [clangd] Introduce a CommandLineConfigProvider

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. This enables unifying command line flags wi

[PATCH] D98037: [clangd] Add config block for Completion and option for AllScopes

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Depends on D98029

[PATCH] D98029: [clangd] Introduce a CommandLineConfigProvider

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

[PATCH] D98037: [clangd] Add config block for Completion and option for AllScopes

kadircet updated this revision to Diff 328492. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98037/new/ https://reviews.llvm.org/D98037 Files: clang-tools-extra/clangd/ClangdServer.cpp clang-tools-extra/clang

[PATCH] D98164: [clangd] Drop explicit specifier on define out-of-line

kadircet created this revision. kadircet added a reviewer: kbobyrev. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Explicit specifier can only be mentioned on

[PATCH] D98029: [clangd] Introduce a CommandLineConfigProvider

kadircet added inline comments. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:555 +}; +if (Sync) { + IndexLoadTask(); sammccall wrote: > (This seems a little weird, get passed in a task runner and maybe ignore it. > Seems like the asyncpr

[PATCH] D98165: [clangd] Make ProjectAwareIndex optionally sync

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Depends on D98029

[PATCH] D98241: [clangd] Move logging out of LSPTest base class into a separate one.

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. This was causing TSan failures due to a rac

[PATCH] D98242: [clangd] Store system relative includes as verbatim

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. This makes our index more portable between

[PATCH] D95043: [clangd] Use Dirty Filesystem for cross file rename.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:408 +clangd::rename({Pos, NewName.getValueOr("__clangd_rename_dummy"), +InpAST->AST, File, TFS.view(llvm::None), +/*Index=*/nullptr, Renam

[PATCH] D98241: [clangd] Move logging out of LSPTest base class into a separate one.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd1531b08c3d1: [clangd] Move logging out of LSPTest base class into a separate one. (authored by kadircet). Repository: rG LLVM Github Monorepo CH

[PATCH] D98242: [clangd] Store system relative includes as verbatim

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:801 + if (IsSystem) +return "<" + ShorterInclude + ">"; // Standard case: just insert the file itself. sammccall wrote: > This is a great heuristic, now I'm thi

[PATCH] D98274: [clangd][NFC] Use std::string::replace in SourceCode:applyChange.

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

[PATCH] D98246: [clangd] Add basic monitoring info request for remote index server

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/remote/Service.proto:13 +import "google/protobuf/empty.proto"; import "Index.proto"; sammccall wrote: > question of style, but unless there's a concrete benefit I'd suggest just > defi

[PATCH] D98329: [clangd] Add cache for FID to Header mappings

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Depends on D98242

[PATCH] D98404: [clangd] Optionally add reflection for clangd-index-server

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/remote/server/CMakeLists.txt:12 +if (ENABLE_GRPC_REFLECTION) + set(REFLECTION_LIBRARY grpc++_reflection) can we move this piece into FindGRPC.cmake instead? Comment a

[PATCH] D98371: [clangd] Group filename calculations in SymbolCollector, and cache mroe.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. the savings look great, thanks! Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:217 + llvm::SmallString<256> AbsPath = Path; + // We don't perfor

[PATCH] D98329: [clangd] Add cache for FID to Header mappings

kadircet abandoned this revision. kadircet added a comment. abandoning in favor of D98371 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98329/new/ https://reviews.llvm.org/D98329 __

[PATCH] D98246: [clangd] Add basic monitoring info request for remote index server

kadircet added a comment. thanks! this mostly LG, just a couple comments here and there. > The current format is "store seconds since X event". I think this is great for certain things like freshness, so thanks! > Should we store UNIX timestamps instead? This is probably not portable until > C

[PATCH] D98164: [clangd] Drop explicit specifier on define out-of-line

This revision was automatically updated to reflect the committed changes. Closed by commit rGb1a5df174e1d: [clangd] Drop explicit specifier on define out-of-line (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98164/new/ https:

[PATCH] D98029: [clangd] Introduce a CommandLineConfigProvider

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4f1bbc0b8426: [clangd] Introduce a CommandLineConfigProvider (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D98242: [clangd] Store system relative includes as verbatim

kadircet updated this revision to Diff 329930. kadircet added a comment. - Rebase - Add a storage for returned verbatim header spellings, as getHeaderIncludeUncached can only return a ref. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98242/new/ h

[PATCH] D98404: [clangd] Optionally add reflection for clangd-index-server

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/remote/server/CMakeLists.txt:12 +if (ENABLE_GRPC_REFLECTION) + set(REFLECTION_LIBRARY grpc++_reflection) kbobyrev wrote: > kadircet wrote: > > can we move this piece into FindGRPC.cmake

[PATCH] D98414: [clangd] Turn off implicit cancellation based on client capabilities

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.h:153 +/// to cancel. Clients that always cancel stale requests should clear this. +bool ImplicitCancellation = true; + this makes sense as is, but i wonder if we should lif

[PATCH] D98414: [clangd] Turn off implicit cancellation based on client capabilities

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/clangd/ClangdServer.h:153 +/// to cancel. Clients that always cancel stale requests should clear this. +bool ImplicitC

[PATCH] D98165: [clangd] Make ProjectAwareIndex optionally sync

kadircet added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98165/new/ https://reviews.llvm.org/D98165 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D98037: [clangd] Add config block for Completion and option for AllScopes

kadircet added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98037/new/ https://reviews.llvm.org/D98037 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D98037: [clangd] Add config block for Completion and option for AllScopes

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

[PATCH] D98037: [clangd] Add config block for Completion and option for AllScopes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGac292dafa776: [clangd] Add config block for Completion and option for AllScopes (authored by kadircet). Repository: rG LLVM Github Monorepo CHANG

[PATCH] D98165: [clangd] Make ProjectAwareIndex optionally sync

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdc9c09632f1a: [clangd] Make ProjectAwareIndex optionally sync (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D98483: [clang] Mark re-injected tokens appropriately during pragma handling

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a subscriber: usaxena95. kadircet requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang. This hides such tokens from TokenWatcher, preventing crashes

[PATCH] D98459: [CodeCompletion] Don't track preferred types if code completion is disabled.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang/include/clang/Parse/Parser.h:944 public: -explicit TentativeParsingAction(Parser& p) : P(p) { - PrevPreferredType = P.Preferre

[PATCH] D98483: [clang] Mark re-injected tokens appropriately during pragma handling

kadircet updated this revision to Diff 330184. kadircet added a comment. - Add test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98483/new/ https://reviews.llvm.org/D98483 Files: clang/lib/Parse/ParsePragma.cpp clang/unittests/Tooling/Syntax/

[PATCH] D98498: [clangd] Enable modules to contribute tweaks.

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. First patch to enable diagnostic fix genera

[PATCH] D98499: [clangd] Introduce ASTHooks to FeatureModules

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman, javed.absar. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. These can be invoked at differ

[PATCH] D98505: [clangd] Propagate data in diagnostics

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Repository: rG LLVM Github Monorepo http

[PATCH] D98499: [clangd] Introduce ASTHooks to FeatureModules

kadircet updated this revision to Diff 330258. kadircet added a comment. - Add tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98499/new/ https://reviews.llvm.org/D98499 Files: clang-tools-extra/clangd/ClangdServer.cpp clang-tools-extra/cl

[PATCH] D98488: [CodeCompletion] Avoid spurious signature help for init-list args

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang/lib/Parse/Parser.cpp:2117 if (S->getFlags() & Scope::FnScope) { + cutOffParsing(); Actions.CodeCompleteOrdinaryName(getCu

[PATCH] D98483: [clang] Mark re-injected tokens appropriately during pragma handling

kadircet updated this revision to Diff 330266. kadircet added a comment. - Introduce a helper to mark tokens as reinjected Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98483/new/ https://reviews.llvm.org/D98483 Files: clang/lib/Parse/ParsePragm

[PATCH] D98483: [clang] Mark re-injected tokens appropriately during pragma handling

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf43ff34ae67a: [clang] Mark re-injected tokens appropriately during pragma handling (authored by kadircet). Repository: rG LLVM Github Monorepo CH

[PATCH] D98538: [clangd] Perform merging for stale symbols in MergeIndex

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Clangd drops symbols from static index when

[PATCH] D98404: [clangd] Optionally add reflection for clangd-index-server

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/remote/server/CMakeLists.txt:12 +if (ENABLE_GRPC_REFLECTION) + set(REFLECTION_LIBRARY grpc++_reflection) kadircet wrote: > kbobyrev wrote: > > kadircet wrote: > > > can we move this pie

[PATCH] D98404: [clangd] Optionally add reflection for clangd-index-server

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/clangd/CMakeLists.txt:190 option(CLANGD_ENABLE_REMOTE "Use gRPC library to enable remote index support for Clangd" OFF) +opt

[PATCH] D98246: [clangd] Add basic monitoring info request for remote index server

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/remote/MonitoringService.proto:17 + optional uint64 uptime = 1; + // Time since the index was built on the indexing machine. + optional uint64 freshness = 3; mention the units in here t

[PATCH] D113765: [clangd] Fix function-arg-placeholder suppression with macros.

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:517 +Completion.Kind == CompletionItemKind::Method || +Completion.Kind == CompletionItemKind::Te

[PATCH] D113891: [NFC][clangd] cleaning up unused "using"

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

[PATCH] D114058: [clang] Add ObjC decls to Decl::isFunctionOrFunctionTemplate

kadircet added a comment. I agree with Nathan on this one. It's unclear why there are two functions that does the same thing in different ways and some usages in sema looks suspicious enough to require caution (there are subtle checks for shadowing and whatnot and I don't know how these rules c

[PATCH] D113999: [clangd] Fix assertion crashes on unmatched NOLINTBEGIN comments.

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

[PATCH] D113899: [NFC][clangd] fix clang-tidy finding on isa_and_nonnull

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/Selection.cpp:504 bool TraverseDecl(Decl *X) { -if (X && isa(X)) +if (isa_and_nonnull(X)) return Base::Trav

[PATCH] D113390: [clangd] Dont include file version in task name

This revision was automatically updated to reflect the committed changes. Closed by commit rGe76e5729896c: [clangd] Dont include file version in task name (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113390/new/ https://revi

[PATCH] D114326: Update the list of CUDA versions up to 11.5

kadircet added inline comments. Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:131 + std::initializer_list Versions = { + "11.5", "11.4", "11.3", "11.2", "11.1", "11.0", "10.2", "10.1", + "10.0", "9.2", "9.1", "9.0", "8.0", "7.5", "7.0"}; looks

[PATCH] D114370: [clangd] IncludeCleaner: Attribute symbols from non self-contained headers to their parents

kadircet added a comment. thanks, LG in general, just a couple polishing touches Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:146 + // + // Returns true if the insertion into IDs took place. + bool insert(llvm::DenseSet &IDs, FileID ID, const SourceManager &SM, --

[PATCH] D114370: [clangd] IncludeCleaner: Attribute symbols from non self-contained headers to their parents

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:262 + ID != SM.getMainFileID() && FE && + !isSelfContainedHeader(PP, ID, FE);) { + ID = SM.getFileID(SM.getIncludeLoc(ID)); kbobyrev wrote: > sammccall wr

[PATCH] D114058: [clangd] Add ObjC method support to prepareCallHierarchy

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp:54 +void verifyIncomingMultiFile(std::string SourceExt, std::string HeaderExt, + Annotations &CalleeH, Annotations &Caller1H, nridg

[PATCH] D114058: [clangd] Add ObjC method support to prepareCallHierarchy

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! let me know of your email address (for commit attribution) if you want me to land this for you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D114525: [clang] Change ordering of PreableCallbacks to make sure PP can be referenced in them

kadircet added a comment. As discussed offline this definitely LGTM, let me summarize the reasoning here. The original review was in https://reviews.llvm.org/D41365. It's unclear why they went with before `BeginSourceFile`. In theory having the callback issued before `BeginSourceFile` allows se

[PATCH] D114058: [clangd] Add ObjC method support to prepareCallHierarchy

This revision was automatically updated to reflect the committed changes. Closed by commit rGe2cad4df22a6: [clangd] Add ObjC method support to prepareCallHierarchy (authored by sheldonneuberger-sc, committed by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D114609: [clang] Fix crash on broken parameter declarators

kadircet created this revision. kadircet added reviewers: sammccall, hokein. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D114609 Files: clang/lib/Parse/ParseDec

[PATCH] D114609: [clang] Fix crash on broken parameter declarators

This revision was automatically updated to reflect the committed changes. kadircet marked 2 inline comments as done. Closed by commit rGd026f2f7c688: [clang] Fix crash on broken parameter declarators (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D114609?vs=389851&id=

[PATCH] D114072: [clangd] Record IWYU pragma keep in the IncludeStructure

kadircet added a comment. It's annoying that we see comments and inclusion directives out-of-order, we can try fixing it on the parser side (I think it is incidental that these are issued in that order currently, they are eagerly trying to generate a fix/diagnostic for tokens after a pp-directi

[PATCH] D114072: [clangd] Record IWYU pragma keep in the IncludeStructure

kadircet added a comment. > Sorry for forgetting about this one. Hopefully I can still help now by > disagreeing with Kadir and creating an awkward stalemate instead. Haha, always welcome! > Partly because the ordering isn't some weird coincidence (see below) Right, I suppose it makes sense wh

[PATCH] D114621: [clangd] Show parameters for construct.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1065 + if (Parameters && !Parameters->empty()) { +Output.addParagraph().appendText("Parameters: "); it's a subtle invariant that we only have parameters for functions (which has

[PATCH] D114522: [clangd] Add desugared type to hover

kadircet added a comment. > My main remaining concern is just that this will trigger too often when > there's no confusion over types, and clutter the display. > Some common cases I'm worried about: > > - std::string -> basic_string. Maybe we should special-case this to > hide the aka? > - int64

[PATCH] D114723: [clangd] Make std symbol generation script python3 friendly

kadircet created this revision. kadircet added a reviewer: hokein. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository: rG LLVM Github Monor

[PATCH] D114724: [clangd][StdSymbolMap] Prefer std::remove from algorithm

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. std::remove from algorithm is a

[PATCH] D114723: [clangd] Make std symbol generation script python3 friendly

This revision was automatically updated to reflect the committed changes. Closed by commit rG3356d8837e46: [clangd] Make std symbol generation script python3 friendly (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114723/new/

[PATCH] D114724: [clangd][StdSymbolMap] Prefer std::remove from algorithm

kadircet added inline comments. Comment at: clang-tools-extra/clangd/StdSymbolMap.inc:958 SYMBOL(remainder, std::, ) -SYMBOL(remove, std::, ) SYMBOL(remove_all_extents, std::, ) sammccall wrote: > Is the stdio version so uncommon we are willing to be wrong abou

[PATCH] D114724: [clangd][StdSymbolMap] Prefer std::remove from algorithm

kadircet added a comment. | 1658 | remove | NULL | | 1731 | std::__u::remove | llvm/include/c++/v1/__algorithm/remove.h | | for whatever reason I remembered the std::remove to have been referenced ~17k. looks like these two are much closer. I don't t

[PATCH] D114864: [clangd] IncludeClenaer: Don't mark forward declarations of a class if it's declared in the main file

kadircet added a comment. I can't fully remember the discussion but what about the case in which we don't have a declaration in the main file, but see a definition and multiple forward declarations. Is there a good reason to still include re-decls in this case? Comment at: cl

[PATCH] D114864: [clangd] IncludeClenaer: Don't mark forward declarations of a class if it's declared in the main file

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:131 + const auto *MostRecent = RD->getMostRecentDecl(); + if (SM.isInMainFile(MostRecent->

[PATCH] D113896: [clangd] cleanup of header guard names

kadircet added a comment. LG apart from closing comments, some of the files have it and some don't (apart from the newly introduced ones, it didn't add it to the files that didn't have a closing comment, but updated the ones that had). The code from the tidy check looks like it should be handlin

[PATCH] D114621: [clangd] Show parameters for construct.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks LGTM! let me know if I should land this for you. Comment at: clang-tools-extra/clangd/Hover.cpp:1054 // editor, as they might be long. - if (ReturnType) { + i

[PATCH] D114949: [clangd] IncludeCleaner: Do not require forward declarations of RecordDecls when definition is available

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:151 + if (SM.isInMainFile(RD->getMostRecentDecl()->getLocation())) { +if (Definition) Result.insert(Definition->getLocation()); if we made it here, we

[PATCH] D113896: [clangd] cleanup of header guard names

kadircet added a comment. > This looks to be a deliberate decision: > https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h#L32 > > Looking at the code under `clang/include`, it looks like most headers don't > have the `#endif // COMMENT`, so this mi

[PATCH] D114949: [clangd] IncludeCleaner: Do not require forward declarations of RecordDecls when definition is available

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:127 +// declared multiple times. The most common two cases are: +// - Definition available in TU,

[PATCH] D136293: [IncludeCleaner] Add public API

kadircet created this revision. kadircet added reviewers: hokein, sammccall. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Introduces walkUsed, a very simple version of the public API to e

[PATCH] D136454: [clangd] Make file limit for textDocument/rename configurable

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

[PATCH] D135953: [IncludeCleaner] Introduce decl to location mapping

kadircet added a comment. In D135953#3861180 , @tschuett wrote: > We support Apple Clang 9.3, but `std:variant` ships with Apple Clang 10.0. :-( Hi @tschuett, there are other pieces of LLVM that use `` today already, some big components include TableGen

[PATCH] D136293: [IncludeCleaner] Add public API

kadircet marked 13 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:14 +#include "llvm/ADT/STLFunctionalExtras.h" +#include + sammccall wrote: > This is awful but: as

[PATCH] D136293: [IncludeCleaner] Add public API

kadircet marked 2 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:67 +llvm::errs() << "returning stdlib\n"; +return CB(Loc, Symbol(*SS), toHeader(SS->headers())); + } samm

[PATCH] D136293: [IncludeCleaner] Add public API

kadircet updated this revision to Diff 470106. 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/D136293/new/ https://reviews.llvm.org/D136293 Files: clang-tools-

[PATCH] D136293: [IncludeCleaner] Add public API

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:14 +#include "llvm/ADT/STLFunctionalExtras.h" +#include + kadircet wrote: > sammccall wrote: > > Th

[PATCH] D136293: [IncludeCleaner] Add public API

This revision was automatically updated to reflect the committed changes. Closed by commit rGce286eccacd6: [IncludeCleaner] Add public API (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136293/new/ https://reviews.llvm.org/D13

[PATCH] D132110: [IncludeCleaner] Handle more C++ constructs

kadircet updated this revision to Diff 470393. kadircet marked 4 inline comments as done. kadircet added a comment. - Rebase - Add FIXMEs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132110/new/ https://reviews.llvm.org/D132110 Files: clang-too

[PATCH] D132110: [IncludeCleaner] Handle more C++ constructs

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG772fc63f5bb5: [IncludeCleaner] Handle more C++ constructs (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

<    20   21   22   23   24   25   26   27   28   29   >