[PATCH] D96124: [clangd] Introduce BuildSystem

2021-02-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman, mgorny. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Interface for BuildSystem integrati

[PATCH] D96125: [clangd] Diagfixer for layering violations

2021-02-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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. Introduce a diagfixer that can update build

[PATCH] D96126: [clangd] Integrate BSP with ClangdServer

2021-02-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

2021-02-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. It looks like premerge tests skipped your last diff with id 321451 (https://reviews.llvm.org/harbormaster/?after=87950). You can re-trigger by uploading a new diff, in the meantime i would also file a bug to https://github.com/google/llvm-premerge-checks/issues. mentio

[PATCH] D96244: [clangd][RFC] Introudce Plugins

2021-02-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. Herald added subscribers: usaxena95, arphaman, mgorny. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Plugins can be used to augment clangd's behaviours in various features. I

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

2021-02-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Fixes only allow modifications to local file containing the diagnostics, with Cod

[PATCH] D108045: [clangd] Fix clangd crash when including a header

2021-09-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9c4a1686d7c4: [clangd] Fix clangd crash when including a header (authored by qdelacru, committed by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D109880: [clangd] PreamblePatch should be no-op if includes arent patched

2021-09-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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. Don't create a useless function

[PATCH] D109884: [clangd] Dont work on diags if we are not going to emit

2021-09-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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. Doesn't install clang-tidy chec

[PATCH] D109884: [clangd] Dont work on diags if we are not going to emit

2021-09-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 372938. kadircet added a comment. - Disable IncludeFixer too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109884/new/ https://reviews.llvm.org/D109884 Files: clang-tools-extra/clangd/ParsedAST.cpp clang

[PATCH] D109884: [clangd] Dont work on diags if we are not going to emit

2021-09-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp:464 - - TU.AdditionalFiles["a.h"] = ""; - TU.AdditionalFiles["b.h"] = ""; sammccall wrote: > Why are these tests deleted? they rely on the fact that clang-tidy che

[PATCH] D109884: [clangd] Dont work on diags if we are not going to emit

2021-09-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 372943. kadircet marked 2 inline comments as done. kadircet added a comment. - Get rid of static_casts - Update comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109884/new/ https://reviews.llvm.org/D1098

[PATCH] D109884: [clangd] Dont work on diags if we are not going to emit

2021-09-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 372947. kadircet added a comment. - Revert revert of IncludeFixer changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109884/new/ https://reviews.llvm.org/D109884 Files: clang-tools-extra/clangd/ParsedAST

[PATCH] D109884: [clangd] Dont work on diags if we are not going to emit

2021-09-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGea79b77da3ee: [clangd] Dont work on diags if we are not going to emit (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D109880: [clangd] PreamblePatch should be no-op if includes arent patched

2021-09-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rG64fe0458866d: [clangd] PreamblePatch should be no-op if includes arent patched (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D10988

[PATCH] D109894: [clangd] Bail-out when an empty compile flag is encountered

2021-09-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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. Fixes https://github.com/clangd

[PATCH] D109894: [clangd] Bail-out when an empty compile flag is encountered

2021-09-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. > I do half wonder whether we're going to get 3 steps further and then crash > again when we call Command.front() :-) I actually couldn't find any other places this could happen today (we seem to check for non-emptiness of compile commands most of the time). But I hadn

[PATCH] D109894: [clangd] Bail-out when an empty compile flag is encountered

2021-09-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 373163. kadircet added a comment. Herald added a project: clang. - Also handle OOB access while creating compiler invocation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109894/new/ https://reviews.llvm.org/

[PATCH] D110051: [clangd] Deduplicate inlay hints

2021-09-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. I am not aware of how the intersecting hints are handled in the implementation on the client side nor in the proposal today. After this patch we might still produce them if for whatever reason there are different kinds of hints for the same range. Is this OK? ==

[PATCH] D109894: [clangd] Bail-out when an empty compile flag is encountered

2021-09-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG444a5f304f6c: [clangd] Bail-out when an empty compile flag is encountered (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109894/new/

[PATCH] D110051: [clangd] Deduplicate inlay hints

2021-09-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! > I don't think we're likely to get a parameter hint and a type hint at the > same location, but if you're concerned, I could revise the patch to ignore > the hint kind duri

[PATCH] D110130: [clangd] Semantic highlighting for lambda init-capture

2021-09-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Naming of the patch is a little bit confusing. We're actually dropping the semantic highlighting for the type of lambdacaptures, which was showing up in the declarator names since there was no explicit type spelled in the source code. This turns on highlighting for the

[PATCH] D110130: [clangd] Ensure lambda init-capture gets semantic token

2021-09-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. thanks, LGTM! Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:518 + SourceLocation StartLoc = D->getTypeSpecStartLoc(); + // The AutoType may not have a corresponding token, e.g. in the case of + // init-captures, so there'

[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree

2021-09-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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. References to fields inside ano

[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree

2021-09-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:745 + if (auto *FD = llvm::dyn_cast(ME->getMemberDecl())) { +if (FD->getParent()->isAnonymousStructOrUnion()) + return SourceRange(ME->getMemberLoc(), ME->getEndLoc()); --

[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree

2021-09-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:443 // It would be nice if RAV handled this (!shouldTraverseImplicitCode()). if (auto *CTI = llvm::dyn_cast(S)) if (CTI->isImplicit()) sammccall wrote: > seems like it'd

[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree

2021-09-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 376238. kadircet marked 4 inline comments as done. kadircet added a comment. - Use `isImplicit` rather than `earlyClaim` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110825/new/ https://reviews.llvm.org/D1108

[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree

2021-09-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:443 // It would be nice if RAV handled this (!shouldTraverseImplicitCode()). if (auto *CTI = llvm::dyn_cast(S)) if (CTI->isImplicit()) sammccall wrote: > kadircet wrote:

[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree

2021-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 376445. kadircet marked 2 inline comments as done. kadircet added a comment. - update comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110825/new/ https://reviews.llvm.org/D110825 Files: clang-tools-e

[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree

2021-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG512aa8485010: [clangd] Handle members of anon structs in SelectionTree (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110825/new/ htt

[PATCH] D111039: [clangd] Include refs of base method in refs for derived method.

2021-10-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:1395 if (CMD->isVirtual()) -if (IdentifierAtCursor && SM.getSpellingLoc(CMD->getLocation()) == - IdentifierAtCursor->location()) { --

[PATCH] D111039: [clangd] Include refs of base method in refs for derived method.

2021-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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/XRefs.cpp:1397 + OverriddenBy.Subjects.insert(ID); getOverriddenMethods(CMD, OverriddenMeth

[PATCH] D110823: [clangd] Add code completion of param name on /* inside function calls.

2021-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1148 + std::set ParamNamesSeen; +}; // SignatureHelpCollector + change the ending comment (well, I'd actually drop it completely) Comment at: clang-tools-ext

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

2021-02-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. > Emitting fixes for a diagnostic in another file seems dangerous, what's the > intended use case for this? one particular example is fixing layering violations in more strict build systems like bazel/blaze. for example if you `#include "a.h"` in a file, but build tar

[PATCH] D96244: [clangd][RFC] Introudce Plugins

2021-02-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Plugin.h:26 + virtual llvm::Optional + actOnDiagnostic(DiagnosticsEngine::Level L, const clang::Diagnostic &Diag) { +return llvm::None; i think it would be better to have these as separate

[PATCH] D95935: [clang][CodeComplete] Fix crash on ParenListExprs

2021-02-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 322077. 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/D95935/new/ https://reviews.llvm.org/D95935 Files: clang/lib/Sema/

[PATCH] D95935: [clang][CodeComplete] Fix crash on ParenListExprs

2021-02-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf74318491134: [clang][CodeComplete] Fix crash on ParenListExprs (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D90595: [clangd] Fix race in background index rebuild, where index could stay stale.

2021-02-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp:850 +if (++IdleCount <= 2) { + Q.push(Task); + Q.push(Task); kadircet wrote: > as discussed offline there are no guarantees on these

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

2021-02-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 322743. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96245/new/ https://reviews.llvm.org/D96245 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp clang-tools-extra/cl

[PATCH] D96244: [clangd][RFC] Introudce Plugins

2021-02-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 322744. kadircet added a comment. - Rename Plugin to Module - Have a ModuleSet that enables type-safe traversal of existing modules (with questions about implementation options) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D96439: [clangd] Introduce DiagnosticAugmentationModule

2021-02-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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. A module interface to enable custom contrib

[PATCH] D96439: [clangd][WIP] Introduce DiagnosticAugmentationModule

2021-02-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Note that this doesn't handle the execution yet, it is mostly to demonstrate the idea and to get some early feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96439/new/ https://reviews.llvm.org/D96439 _

[PATCH] D96244: [clangd] Introduce Modules

2021-02-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 322754. kadircet added a comment. - Changes to propagate moduleset into clangd(lsp)server Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96244/new/ https://reviews.llvm.org/D96244 Files: clang-tools-extra/cl

[PATCH] D96439: [clangd][WIP] Introduce DiagnosticAugmentationModule

2021-02-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 322755. kadircet added a comment. - Integrate with parseinputs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96439/new/ https://reviews.llvm.org/D96439 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp

[PATCH] D96507: [clangd] Move command handlers into a map in ClangdLSPServer. NFC

2021-02-11 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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/ClangdLSPServer.cpp:587 + std::vector Commands; + llvm::copy(CommandHandlers.keys(), std::back_inserter(Commands)); +

[PATCH] D96244: [clangd] Introduce Modules

2021-02-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 323290. kadircet marked 7 inline comments as done. kadircet added a comment. - Define destruction order - Get rid of Module.cpp and Module::id - Define begin/end iterators for ModuleSet Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D96244: [clangd] Introduce Modules

2021-02-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Module.h:25 +/// - module hooks can be called afterwards. +/// - modules can be destroyed before/after ClangdServer and ClangdLSPServer +///FIXME: Once we make server facilities available to modules, we'll

[PATCH] D96244: [clangd] Introduce Modules

2021-02-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 323361. 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/D96244/new/ https://reviews.llvm.org/D96244 Files: clang-tools-ex

[PATCH] D96244: [clangd] Introduce Modules

2021-02-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.h:33 #include "clang/Tooling/Core/Replacement.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/FunctionExtras.h" sammccall wrote: > (include no longer used?) well it is still

[PATCH] D96608: [clangd] Delay binding LSP methods until initialize. NFC

2021-02-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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:165 return false; -if (!Server.Server) { - elog("Notification {0} before initialization", Met

[PATCH] D96244: [clangd] Introduce Modules

2021-02-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2423a3863e07: [clangd] Introduce Modules (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D96690: [clangd] Treat paths case-insensitively depending on the platform

2021-02-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Path{Match,Exclude} and MountPoint were checking paths case-sensitively on all pl

[PATCH] D96544: [clangd] Extract binding of typed->untyped LSP handlers to LSPBinder. NFC

2021-02-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! (I wish outgoing calls were not so different :/) Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:173 } else { - log("unhandled notificati

[PATCH] D96625: [clangd] Allow modules to bind LSP methods/notifications/commands

2021-02-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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/D96625/new/ https://reviews.llvm.org/D96625

[PATCH] D96123: [clangd] Expose actOnAllPArentDirectories helper

2021-02-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done. kadircet added a comment. > maybe we should expose absoluteParent instead? doing that instead. also updating tidyprovider and configyamlparser to make use of this helper now, PTAL. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D96123: [clangd] Expose actOnAllPArentDirectories helper

2021-02-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 323820. kadircet added a comment. - Expose absoluteParent instead of whole traverse action - Use helper in existing places that use path::parent_path for traversal Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D96717: [clangd] Bind outgoing calls through LSPBinder too. NFC

2021-02-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. mostly LG. some comments around the way we set up outgoinmethod stubs though. Comment at: clang-tools-extra/clangd/LSPBinder.h:88 + template + void outgoingMethod(llvm::StringLiteral Method, + OutgoingMethod &Handler) {

[PATCH] D96730: [clangd] Modules can have a public API. NFC

2021-02-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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:349 + ModuleSet *Modules; const GlobalCompilationDatabase &CDB; nit: ` = nullptr`

[PATCH] D96726: [clangd] Give modules access to filesystem, scheduler, and index.

2021-02-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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/Module.h:1 +//===--- Module.h - Plugging features into clangd -*-C++-*-===// +// oops

[PATCH] D96755: [clangd] Shutdown sequence for modules, and doc threading requirements

2021-02-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:180 +for (auto &Mod : *Modules) + Mod.blockUntilIdle(Deadline::infinity()); + } why is our contract saying that just calling `stop` is not enough? i think clangdserver

[PATCH] D89870: [clangd] Drop template argument lists from completions followed by

2021-02-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. (sorry for forgetting about this) Comment at: clang-tools-extra/clangd/CodeComplete.cpp:450 +if (Snippet->front() == '<') + return Snippet->substr(0, Snippet->find('(')); +return ""; what if we have `(` in the

[PATCH] D96690: [clangd] Treat paths case-insensitively depending on the platform

2021-02-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 323951. kadircet added a comment. Herald added subscribers: ormris, mgorny. - Don't case fold stored strings, or the ones passing interface boundaries - Define a `pathIsAncestor` helper instead and use that for comparison of MountPoint - Expose a CLANGD_PATH

[PATCH] D96690: [clangd] Treat paths case-insensitively depending on the platform

2021-02-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 323997. kadircet marked an inline comment as done. kadircet added a comment. - s/pathIsAncestor/pathStartsWith - Fix tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96690/new/ https://reviews.llvm.org/D966

[PATCH] D96690: [clangd] Treat paths case-insensitively depending on the platform

2021-02-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/support/Path.cpp:22 -bool pathEqual(PathRef A, PathRef B) { -#if defined(_WIN32) || defined(__APPLE__) - return A.equals_lower(B); -#else - return A == B; -#endif +bool pathIsAncestor(PathRef Ancestor, PathR

[PATCH] D96717: [clangd] Bind outgoing calls through LSPBinder too. NFC

2021-02-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! can't wait for the unique_function sfinae fix :) Comment at: clang-tools-extra/clangd/LSPBinder.h:185 + +LSPBinder::UntypedOutgoingNotification inline LSPBinder::

[PATCH] D96690: [clangd] Treat paths case-insensitively depending on the platform

2021-02-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGecea7218fb9b: [clangd] Treat paths case-insensitively depending on the platform (authored by kadircet). Changed prior to commit: https://reviews.l

[PATCH] D96755: [clangd] Shutdown sequence for modules, and doc threading requirements

2021-02-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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.cpp:180 +for (auto &Mod : *Modules) + Mod.blockUntilIdle(Deadline::infinity()); + }

[PATCH] D96717: [clangd] Bind outgoing calls through LSPBinder too. NFC

2021-02-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. still LG! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96717/new/ https://reviews.llvm.org/D96717 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D89870: [clangd] Drop template argument lists from completions followed by

2021-02-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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/CodeComplete.cpp:474 +// +// fu^(42) -> function(42); +if (Snippet->front() == '<') { -

[PATCH] D96856: [clangd] Narrow and document a loophole in blockUntilIdle

2021-02-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. LG for the `blockUntilIdle` update, but feels like two patches wind up together :( Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96856/new/ https://reviews.llvm.org/D96856 ___

[PATCH] D96856: [clangd] Narrow and document a loophole in blockUntilIdle

2021-02-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:893 + for (llvm::Optional Timeout : + {TimeoutSeconds, TimeoutSeconds, llvm::Optional(0)}) { +if (!CDB.blockUntilIdle(timeoutSeconds(Timeout))) this is extending the

[PATCH] D96944: [RecoveryAST] Add design doc to clang internal manual.

2021-02-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. thanks! i think this mostly looks great, leaving a couple of suggestions and questions in comments. Comment at: clang/docs/InternalsManual.rst:1882 + example. +- representing invalid node: the invalid node is preserved in the AST in some + form, e.g

[PATCH] D96950: [clang][CodeComplete] Ensure there are no crashes when completing with ParenListExprs as LHS

2021-02-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. 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/D96950 Files: clang/lib/Sema/SemaCodeComplete.

[PATCH] D97043: [clang][DeclPrinter] Pass Context into StmtPrinter whenever possible

2021-02-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. ASTContext were only passed to the StmtPrinter in some places, whi

[PATCH] D97043: [clang][DeclPrinter] Pass Context into StmtPrinter whenever possible

2021-02-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:526 HI.Name = "result"; - HI.Definition = "static constexpr int result = 1 + 2"; + HI.Definition = "static constexpr int result = a + b"; HI.Kind = in

[PATCH] D96950: [clang][CodeComplete] Ensure there are no crashes when completing with ParenListExprs as LHS

2021-02-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 324945. kadircet marked 4 inline comments as done. kadircet added a comment. - Pull unwrapping of parenlistexprs into a function - Merge comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96950/new/ https:

[PATCH] D96123: [clangd] Expose actOnAllPArentDirectories helper

2021-02-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 324946. kadircet marked 2 inline comments as done. kadircet added a comment. - Update comment. - Get rid of a parent_path usage. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96123/new/ https://reviews.llvm.or

[PATCH] D96123: [clangd] Expose actOnAllPArentDirectories helper

2021-02-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 324947. kadircet added a comment. - Get rid of parent_path usage for real Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96123/new/ https://reviews.llvm.org/D96123 Files: clang-tools-extra/clangd/ConfigProvi

[PATCH] D96123: [clangd] Expose actOnAllPArentDirectories helper

2021-02-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6329ce75da7a: [clangd] Expose absoluteParent helper (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D96123?vs=324947&id=

[PATCH] D96944: [RecoveryAST] Add design doc to clang internal manual.

2021-02-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added inline comments. Comment at: clang/docs/InternalsManual.rst:1882 + example. +- representing invalid node: the invalid node is preserved in the AST in some + form, e.g. when the "declaration" part of the declaration contains semant

[PATCH] D96856: [clangd] Narrow and document a loophole in blockUntilIdle

2021-02-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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/D96856/new/ https://reviews.llvm.org/D96856

[PATCH] D96950: [clang][CodeComplete] Ensure there are no crashes when completing with ParenListExprs as LHS

2021-02-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf10137399a3c: [clang][CodeComplete] Ensure there are no crashes when completing with… (authored by kadircet). Repository: rG LLVM Github Monorepo

[PATCH] D97123: [clangd] Support FixIts that use InsertFromRange instead of inserting raw text

2021-02-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. mostly LG, can you also add some tests? Comment at: clang-tools-extra/clangd/Diagnostics.cpp:695 + if (Insert.empty() && FixIt.InsertFromRange.isValid()) { +bool InvalidInsert = false; +Insert = Lexer::getSourceText(FixIt.InsertFro

[PATCH] D97123: [clangd] Support FixIts that use InsertFromRange instead of inserting raw text

2021-02-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.cpp:552 Result.newText = FixIt.CodeToInsert; + if (Result.newText.empty() && FixIt.InsertFromRange.isValid()) { +bool Invalid = false; oh btw, i think the condition should onl

[PATCH] D97043: [clang][DeclPrinter] Pass Context into StmtPrinter whenever possible

2021-02-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7c9c0a87c8ae: [clang][DeclPrinter] Pass Context into StmtPrinter whenever possible (authored by kadircet). Repository: rG LLVM Github Monorepo CH

[PATCH] D97123: [clangd] Support FixIts that use InsertFromRange instead of inserting raw text

2021-02-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.cpp:555 +auto Insert = Lexer::getSourceText(FixIt.InsertFromRange, M, L, &Invalid); +if (!Invalid) + Result.newText = Insert.str(); njames93 wrote: > kadircet wrote: > >

[PATCH] D97226: [clangd] Show hex value of numeric constants

2021-02-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! should we also have some tests for a negative value in hover? Comment at: clang-tools-extra/clangd/Hover.cpp:384 if (ECD->getInitVal() == Val) -

[PATCH] D96286: [clangd] Change TidyProvider cache to use a linked list approach

2021-02-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. just wanted to point out that, this is almost the same thing as https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp#L569. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D9628

[PATCH] D97366: [clangd] Fix a race

2021-02-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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] D97366: [clangd] Fix a race

2021-02-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc94ecf3f81ca: [clangd] Fix a race (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97366/new/ https://reviews.llvm.org/D97366 Files:

[PATCH] D97226: [clangd] Show hex value of numeric constants

2021-02-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. still lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97226/new/ https://reviews.llvm.org/D97226 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D97123: [clangd] Support FixIts that use InsertFromRange instead of inserting raw text

2021-02-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.cpp:555 +auto Insert = Lexer::getSourceText(FixIt.InsertFromRange, M, L, &Invalid); +if (!Invalid) + Result.newText = Insert.str(); njames93 wrote: > kadircet wrote: > >

[PATCH] D103685: [clangd] Drop TestTUs dependency on gtest

2021-06-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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. TestTU now prints errors to llv

[PATCH] D99540: [clangd] Preserve diags between tweak enumeration and execution

2021-06-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. > Sorry I've lost my context - did we decide to move forward with this patch? I don't think we've came to a conclusion, just decided to postpone until needed. I believe the `cases` design is really a good fit for making tweaks expose multiple code actions. But we've a

[PATCH] D103472: [clang] Fix a crash during code completion

2021-06-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. we discussed offline but i forgot to stamp this one. it would be nice to have a test case, but fix is relatively safe and getting a repro turned out to be hard (since it depends on a dense

[PATCH] D103685: [clangd] Drop TestTUs dependency on gtest

2021-06-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 350206. kadircet marked 3 inline comments as done. kadircet added a comment. - Use log + abort instead of llvm_unreachable to not rely on UB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103685/new/ https://r

[PATCH] D103685: [clangd] Drop TestTUs dependency on gtest

2021-06-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/TestTU.cpp:75 + if (llvm::sys::fs::createUniqueDirectory("module-cache", ModuleCachePath)) +llvm_unreachable("Failed to create temp directory for module-cache"); CI.getHeaderSearchOpts().Module

[PATCH] D103797: [clang] Use resolved path for headers in decluse diagnostics

2021-06-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. A header spelling in a source file could potentially mean multiple sources, especially in projects with multiple include search paths. We try to elimin

[PATCH] D103685: [clangd] Drop TestTUs dependency on gtest

2021-06-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 350231. kadircet added a comment. - Assert for daigs rather than abort. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103685/new/ https://reviews.llvm.org/D103685 Files: clang-tools-extra/clangd/index/Symbo

[PATCH] D103685: [clangd] Drop TestTUs dependency on gtest

2021-06-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG4728aca9a8ad: [clangd] Drop TestTUs dependency on gtest (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103685/new/ https://reviews.ll

<    18   19   20   21   22   23   24   25   26   27   >