[PATCH] D90270: [clangd] Handle absolute/relative path specifications in Config

2020-10-29 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:162 return false; +llvm::StringRef Path = P.Path; +// Ignore the file if it is not nested under Fragment and strip the sammccall wrote: >

[PATCH] D90291: [clangd] Add lit tests for remote index

2020-10-29 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/test/remote-index/pipeline.test:1 +# RUN: rm -rf %/t +# RUN: clangd-indexer %/S/../Inputs/remote-index/Source.cpp > %t.in.dex nit: `/` shouldn't be needed here. the difference between %t and %/

[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-10-29 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman. Herald added a project: clang. kadircet requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. We were default initializing SymbolIDs befo

[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-10-29 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 301678. kadircet marked an inline comment as done. kadircet added a comment. - change invalid -> null. - add an implicit bool conversion operator. - update apis returning optionals. Note that all the functional changes are in SymbolID.h, rest is api updates.

[PATCH] D89670: [clangd] Store the containing symbol for refs

2020-10-29 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/Ref.h:91 RefKind Kind = RefKind::Unknown; + SymbolID Container; }; i am afraid we are going to have an indeterminate value here if for whatever reason `Container` detection fails. (e

[PATCH] D90291: [clangd] Add lit tests for remote index

2020-10-30 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/test/lit.cfg.py:26 config.clangd_binary_dir + "/benchmarks")) +config.substitutions.appen

[PATCH] D90270: [clangd] Handle absolute/relative path specifications in Config

2020-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 301842. kadircet marked an inline comment as done. kadircet added a comment. - Return "." if Path == FragmentDir in conifgRelative. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90270/new/ https://reviews.llvm

[PATCH] D90270: [clangd] Handle absolute/relative path specifications in Config

2020-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:51 +return llvm::StringRef(); + return Path; +} sammccall wrote: > not that if Path == FragmentDir you're going to return "" which means "not > under". > I don't think th

[PATCH] D90452: [clangd] Respect codeAction.isPreferredSupport in client capabilities

2020-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added reviewers: sammccall, adamcz. Herald added subscribers: cfe-commits, usaxena95, arphaman. Herald added a project: clang. kadircet requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Fixes https://github.com/clangd/clan

[PATCH] D90455: [clangd] Pass parameters to config apply functions

2020-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman. Herald added a project: clang. kadircet requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. This will enable some fragments to apply th

[PATCH] D90270: [clangd] Handle absolute/relative path specifications in Config

2020-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 301912. kadircet added a comment. - Explicitly handle FragmentDir.empty() case by returning Path as-is. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90270/new/ https://reviews.llvm.org/D90270 Files: clang-

[PATCH] D90270: [clangd] Handle absolute/relative path specifications in Config

2020-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked 2 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:170 + [PathMatch(std::move(PathMatch)), + FragmentDir(FragmentDirectory)](const Params &P) { if (P.Path.empty()) -

[PATCH] D90291: [clangd] Add lit tests for remote index

2020-11-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/test/remote-index/pipeline_helper.py:42 + found_init_message = False + for line in index_server_process.stderr: +if b'Server listening' in line: so if we think `.readline()` is blocking bu

[PATCH] D90587: [clangd] Control the delay between index hot reloading in remote-server-index

2020-11-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:78 +llvm::cl::desc("Delay between index hot reload checks (in seconds)"), +llvm::cl::init(90), +llvm::cl::Hidden, this was 30 before. not that it matte

[PATCH] D90526: [clangd] Add -log=public to redact all request info from index server logs

2020-11-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Thanks for figuring this one out! I think the final result looks pretty good, I especially loved only disabling when there's a request in question. My only hesitation is around logging all severities when we are outside a request, I suppose it shouldn't cause too much

[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-11-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 302230. kadircet marked 4 inline comments as done. kadircet added a comment. - Mention possibility of returning null SymbolIDs in comments. - Mark bool conversion operator explicit, fix the bug in returned value. (and run tests :)) Repository: rG LLVM Gi

[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-11-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolID.h:58 + bool isNull() const { return HashValue != std::array{}; } + operator bool() const { return isNull(); } + sammccall wrote: > sammccall wrote: > > nit: I think you want thi

[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-11-02 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 rG0df197516b69: [clangd] Value initialize SymbolIDs (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D89670: [clangd] Store the containing symbol for refs

2020-11-02 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/unittests/SymbolCollectorTests.cpp:782 + +EXPECT_EQ(Ref1->Container, Ref2->Container); + }; can y

[PATCH] D90526: [clangd] Add -log=public to redact all request info from index server logs

2020-11-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D90526#2367782 , @sammccall wrote: > So I can't remember anymore why I put this as an enum `-log=public` rather > than an independent switch `-log-public` or so that could combine with any > log level. > Sounds like the latte

[PATCH] D90291: [clangd] Add lit tests for remote index

2020-11-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/test/lit.site.cfg.py.in:11 config.host_triple = "@LLVM_HOST_TRIPLE@" +config.python_executable = "@Python3_EXECUTABLE@" hans wrote: > Could this use `PYTHON_EXECUTABLE` instead? I see other t

[PATCH] D90654: [clangd] Add index server request logging

2020-11-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. I wish there was an easy way to check redacted error logs too :/ Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:335 return std::make_unique(OS, LogLevel); +llvm::errs() << "non redacted logger\n"; + } fee

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

2020-11-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp:848 +// The first two times the queue goes idle, add a couple more tasks. +// This should caute OnIdle to run again. +if (++IdleCount <= 2) { --

[PATCH] D102418: [clangd] Always default to raw pch format

2021-05-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: adamcz. 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. Clang would emit a fatal error whe

[PATCH] D102431: [clangd][QueryDriver] Dont check for existence of driver

2021-05-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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. Execute implementations already ch

[PATCH] D102418: [clangd] Always default to raw pch format

2021-05-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGed339111bff6: [clangd] Always default to raw pch format (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102418/new/ https://reviews.ll

[PATCH] D102519: [clangd] Set FileSystem for tweaks in Check tool.

2021-05-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. thanks, lgtm! Comment at: clang-tools-extra/clangd/tool/Check.cpp:215 nullptr); - for (const auto &T : - prepareTweaks(Selection, Opts.TweakFilter, Opts.FeatureModules)) { + auto Tweaks = +

[PATCH] D102519: [clangd] Set FileSystem for tweaks in Check tool.

2021-05-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. oops, sorry. i thought i've accepted the revision already :D Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102519/new/ https://reviews.llvm.org/D102519 ___ cfe-commits mailing l

[PATCH] D102431: [clangd][QueryDriver] Dont check for existence of driver

2021-05-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGec2f7376e39f: [clangd][QueryDriver] Dont check for existence of driver (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102431/new/ htt

[PATCH] D102750: [clang] Fix a crash on CheckArgAlignment.

2021-05-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! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102750/new/ https://reviews.llvm.org/D102750

[PATCH] D102761: [clangd] New ParsingCallback for semantics changes

2021-05-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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-tools-extra. Previously notific

[PATCH] D102761: [clangd] New ParsingCallback for semantics changes

2021-05-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 347621. kadircet marked an inline comment as done. kadircet added a comment. - s/onSemanticsMaybeChanged/onPreamblePublished Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102761/new/ https://reviews.llvm.org/D

[PATCH] D102761: [clangd] New ParsingCallback for semantics changes

2021-05-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. > It seems like a lit test for this would be terrible. A ClangdServer one > should be possible, but I can't quite wrap my head around how to write it. > (Delivering the PreambleData as a param would make the test easier, you could > assert that the preamble version was

[PATCH] D102761: [clangd] New ParsingCallback for semantics changes

2021-05-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8f79203a22d8: [clangd] New ParsingCallback for semantics changes (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102761/new/ https://r

[PATCH] D103179: [clangd] Handle queries without an originating file in ProjectAwareIndex

2021-05-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added reviewers: sammccall, kbobyrev. Herald added subscribers: usaxena95, jfb, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. This enables hand

[PATCH] D103179: [clangd] Handle queries without an originating file in ProjectAwareIndex

2021-05-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Note that I don't feel strongly about making use of `Context` to figure out config::Params vs moving the `Params` into the `Config`. But I'd rather not only store `Path` in the `Config` since we might end up needing other environment variables in future. I didn't go w

[PATCH] D103387: [clangd] Fix feature modules to drop diagnostics

2021-05-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. Ignored diagnostics were only c

[PATCH] D103387: [clangd] Fix feature modules to drop diagnostics

2021-05-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 348733. kadircet added a comment. Exit after introducing cleanup function Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103387/new/ https://reviews.llvm.org/D103387 Files: clang-tools-extra/clangd/Diagnosti

[PATCH] D103387: [clangd] Fix feature modules to drop diagnostics

2021-05-31 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 348747. kadircet added a comment. Get rid of LastDiagWasSuppressed state in StoreDiags Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103387/new/ https://reviews.llvm.org/D103387 Files: clang-tools-extra/cla

[PATCH] D103377: [clangd] Add ability to change storage directory of index files

2021-05-31 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. > I mainly added this because I am doing profiling of indexing and needed a way > for multiple instances of index files not to conflict with each other. If this is solely for testing purposes, can't you make use of `--compile-commands-dir` instead? You'll also need to

[PATCH] D103377: [clangd] Add ability to change storage directory of index files

2021-05-31 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. > Strictly speaking you could, assuming you have a compile database and it's > okay to have copies or symlinks of the database. Well you wouldn't have index shards without a compilation database anyway though. > That does speak to another issue though; moving the comp

[PATCH] D103393: [clangd] Bump recommended gRPC version (1.33.2 -> 1.36.3)

2021-05-31 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. I think we should wait for build bot update on this one too. Otherwise we might miss some breakages. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103393/new/ https://reviews.llvm.org/D103393

[PATCH] D103449: [clangd][Protocol] Drop optional from WorkspaceEdit::changes

2021-06-01 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. This is causing weird code patt

[PATCH] D99523: [clangd] Use command line adjusters for inserting compile flags

2021-06-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:271 Out.Apply.push_back([Add(std::move(Add))](const Params &, Config &C) { C.CompileFlags.Edits.push_back([Add](std::vector &Args) { +

[PATCH] D99523: [clangd] Use command line adjusters for inserting compile flags

2021-06-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 348972. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99523/new/ https://reviews.llvm.org/D99523 Files: clang-tools-extra/clangd/CompileCommands.cpp clang-tool

[PATCH] D103387: [clangd] Fix feature modules to drop diagnostics

2021-06-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:732 +if (Adjuster) DiagLevel = Adjuster(DiagLevel, Info); sammccall wrote: > If I'm reading this right: > - we previously discarded the diagnostic "quickly" without

[PATCH] D103476: [clangd] TUScheduler uses last active file for file-less queries

2021-06-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, jfb, arphaman, javed.absar. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. This enables

[PATCH] D103538: [clangd] Run code completion on each token coverd by --check-lines

2021-06-02 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. --- A little bit thinking out loud, was there a particular reason to introduce `--check-line` into ClangdMain.cpp rather than Check.cpp? It feels like we should have --chec

[PATCH] D103476: [clangd] TUScheduler uses last active file for file-less queries

2021-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 349366. kadircet marked an inline comment as done. kadircet added a comment. Keep using `Path` in `runWithSemaphore`, by substituting `LastActiveFile` when empty. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D103179: [clangd] Handle queries without an originating file in ProjectAwareIndex

2021-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet abandoned this revision. kadircet added a comment. in favor of D103476 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103179/new/ https://reviews.llvm.org/D103179 _

[PATCH] D103476: [clangd] TUScheduler uses last active file for file-less queries

2021-06-02 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 rG6c2a4e28f4d1: [clangd] TUScheduler uses last active file for file-less queries (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D103449: [clangd][Protocol] Drop optional from WorkspaceEdit::changes

2021-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 349371. kadircet marked an inline comment as done. kadircet added a comment. Get rid of the special case around empty changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103449/new/ https://reviews.llvm.org

[PATCH] D103449: [clangd][Protocol] Drop optional from WorkspaceEdit::changes

2021-06-02 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 rGdc10bf1a4ed0: [clangd][Protocol] Drop optional from WorkspaceEdit::changes (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D100308: [clangd] Drop optional on ExternalIndexSpec

2021-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 349378. kadircet marked an inline comment as done. kadircet added a comment. Add comment to IndexFactory about pre-condition of spec never being none. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100308/new/

[PATCH] D100308: [clangd] Drop optional on ExternalIndexSpec

2021-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:555 + case Config::ExternalIndexSpec::None: +break; case Config::ExternalIndexSpec::Server: sammccall wrote: > I think you hit llvm_unreachable here - is this an inva

[PATCH] D100308: [clangd] Drop optional on ExternalIndexSpec

2021-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 349379. kadircet added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100308/new/ https://reviews.llvm.org/D100308 Files: clang-tools-extra/clangd/Config.h clang-tools-extra/clangd/Config

[PATCH] D100308: [clangd] Drop optional on ExternalIndexSpec

2021-06-02 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 rG9e9ac4138890: [clangd] Drop optional on ExternalIndexSpec (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D95131: [CodeComplete] Add ranged for loops code pattern.

2021-01-22 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/D95131/new/ https://reviews.llvm.org/D95131

[PATCH] D95087: [clangd] Inject context provider rather than config into ClangdServer. NFC

2021-01-22 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/ClangdServer.cpp:277 + D.getColumnNo(), D.getMessage()); +break; + } nit: maybe finish

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-01-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Thanks this looks great, and something i've been longing for some time! But I got a couple of questions about the how the interaction is designed (sorry if I missed some high level discussions elsewhere, feel free to just point me in that direction). - Why do we just

[PATCH] D91186: [clangd] Add documentation for building and testing clangd

2021-01-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG4cca7939: [clangd] Add documentation for building and testing clangd (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91186/new/ ht

[PATCH] D95229: [clangd] Treat optional field type mismatches as soft failures

2021-01-22 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. Clangd currently throws away any protocol m

[PATCH] D95270: [clangd][NFC] Simplify handing on methods with no params

2021-01-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. thanks this LG, but I suppose we can do some more trimming. after the trimming we are introducing a single partial specialization which is luckily more specialized than existing notification-binding specialization, so all should be fine. Comment at:

[PATCH] D95057: [clangd] Allow configuration database to be specified in config.

2021-01-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. I believe we should log some warning at startup if user has provided `--compile-commands-dir`. Saying that "CDB search customizations through config is disabled". (only emitting the warning when we hit a config with CDB search customization would be nicer, but plumbing

[PATCH] D95057: [clangd] Allow configuration database to be specified in config.

2021-01-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp:140 + EXPECT_THAT(Diags.Diagnostics, IsEmpty()); +} + maybe also try with an absolute path in `CompilationDatabase` ? Repository: rG LLVM Github Monorepo C

[PATCH] D95057: [clangd] Allow configuration database to be specified in config.

2021-01-25 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/ConfigCompile.cpp:278 + // Drop trailing slash to put the path in canonical form. + // Should makeAbs

[PATCH] D95365: [clangd] Add include-fixer fixit for no_member_template diagnostic.

2021-01-25 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! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95365/new/ https://reviews.llvm.org/D95365 __

[PATCH] D95419: [clangd] Fix filename completion at the end of file

2021-01-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added reviewers: hokein, 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. Fixes https://github.com/clangd/clang

[PATCH] D95423: [clangd] Add std::size_t to StdSymbol mapping

2021-01-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added reviewers: sammccall, 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. This is a common symbol that's missi

[PATCH] D95419: [clangd] Fix filename completion at the end of file

2021-01-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 319229. kadircet marked an inline comment as done. kadircet added a comment. - Rewrite the condition as suggested Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95419/new/ https://reviews.llvm.org/D95419 Files

[PATCH] D95419: [clangd] Fix filename completion at the end of file

2021-01-26 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 rG64cdba65bbfa: [clangd] Fix filename completion at the end of file (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D95439: [clangd] Add include-fixer fixit for field_incomplete_or_sizeless diagnostic.

2021-01-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeFixer.cpp:81 case diag::err_func_def_incomplete_result: + case diag::err_field_incomplete_or_sizeless: // Incomplete type diagnostics should have a QualType argument for the what

[PATCH] D95423: [clangd] Add std::size_t to StdSymbol mapping

2021-01-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9190f17a7cc5: [clangd] Add std::size_t to StdSymbol mapping (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95423/new/ https://reviews

[PATCH] D95229: [clangd] Treat optional field type mismatches as soft failures

2021-01-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 319321. kadircet marked 4 inline comments as done. kadircet added a comment. As discussed offline, only treat "null" fields as missing, rather than allowing type mismatches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D95451: [clangd] references: decls of overrides of x are refs to x, not decls

2021-01-26 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:1406 + Req.Limit = Limit; + auto QueryIndex = [&](bool AllowAttributes) { +if (Req.IDs.empty() || !Inde

[PATCH] D95439: [clangd] Add include-fixer fixit for field_incomplete_or_sizeless diagnostic.

2021-01-27 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! Comment at: clang-tools-extra/clangd/IncludeFixer.cpp:81 case diag::err_func_def_incomplete_result: + case diag::err_field_incomplete_or_sizeless: // Incom

[PATCH] D95229: [clangd] Treat optional field type mismatches as soft failures

2021-01-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 319551. kadircet marked 2 inline comments as done. kadircet added a comment. - rename to mapOptOrNull - revert leftover changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95229/new/ https://reviews.llvm.or

[PATCH] D95229: [clangd] Treat optional field type mismatches as soft failures

2021-01-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Protocol.cpp:837 return false; + tryMap(Params, "triggerCharacter", R.triggerCharacter, P); R.triggerKind = static_cast(TriggerKind); sammccall wrote: > why are we no longer checking th

[PATCH] D95229: [clangd] Treat optional field type mismatches as soft failures

2021-01-27 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 rGaf20232b8e18: [clangd] Treat "null" optional fields as missing (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-01-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added reviewers: MyDeveloperDay, curdeius. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently getStyle() fails immediately if FallbackStyle is not a predefined style, even when it is

[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-01-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Sure adding tests, sorry for leaving them out the first time. To give more insights, `format::getStyle()` tries to figure out `FallbackStyle` at the beginning of the function, and errors out if it can't. But in most cases `FallbackStyle` is not needed, e.g. there's alr

[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-01-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 319765. kadircet added a comment. - Add tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95538/new/ https://reviews.llvm.org/D95538 Files: clang/lib/Format/Format.cpp clang/unittests/Format/FormatTest

[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-01-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D95538#2527335 , @curdeius wrote: > Why is fixing (or removing) the fallback style in the .clang-format file not > enough?? sure that would be one way to go, and that's what the user did already. It feels confusing to me that

[PATCH] D95576: [clangd] Remove support for pre-standard semanticHighlighting notification

2021-01-28 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! there are also some comments on `ASTWorker::PublishMu` talking about semantic highlights publishing. Comment at: clang-tools-extra/clangd/SemanticHighligh

[PATCH] D95576: [clangd] Remove support for pre-standard semanticHighlighting notification

2021-01-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. oh also in addition to `This should not land until the new protocol has feature parity, see D77702.`, it might be better to hold on to it a little bit longer, since it is touching lots of pieces and might make cherry-picking some fixes that touch the same files harder

[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-01-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. > I see little value in not checking FallbackStyle (even if it's not used). note that this patch doesn't disable fallbackstyle checking, it is still checked, but not eagerly. > However, I do see value in an early warning (error) on an incorrect style > that can pop up

[PATCH] D95725: clang-extra: fix incorrect use of std::lock_guard by adding variable name (identified by MSVC [[nodiscard]] error)

2021-01-31 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/support/Function.h:54 if (Parent) { -std::lock_guard(Parent->ListenersMu); +std::lock_guard Guar

[PATCH] D95670: [clangd] Don't rely on builtin headers for document-link.test.

2021-01-31 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a subscriber: sammccall. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! but I would wait for a while to see if someone(that remembers why this test was specifically asserting for built-in headers) will objec

[PATCH] D95725: clang-extra: fix incorrect use of std::lock_guard by adding variable name (identified by MSVC [[nodiscard]] error)

2021-02-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG0b70c86e2007: clang-extra: fix incorrect use of std::lock_guard by adding variable name… (authored by poelmanc, committed by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

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

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

2021-02-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 321054. kadircet added a comment. - Also handle memberrefexpr case. Fixes https://github.com/clangd/clangd/issues/676. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95935/new/ https://reviews.llvm.org/D95935

[PATCH] D95942: [clangd] Deduplicate scopes in IncludeFixer queries

2021-02-03 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] D95942: [clangd] Deduplicate scopes in IncludeFixer queries

2021-02-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 321118. kadircet added a comment. Herald added a subscriber: mgrang. - sort+unique+erase instead of using a set - Inline visitor and directly collect scopes there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D95942: [clangd] Deduplicate scopes in IncludeFixer queries

2021-02-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 321120. kadircet added a comment. - add back the global scope. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95942/new/ https://reviews.llvm.org/D95942 Files: clang-tools-extra/clangd/IncludeFixer.cpp Ind

[PATCH] D96058: [CodeComplete] Guess type for designated initializers

2021-02-04 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! LG with a small concern around possible effects on member completions for uninstantiated templates. Comment at: clang/lib/Parse/ParseInit.cpp:163 +Designator

[PATCH] D96027: [clangd] Trace queue state for each TUScheduler action.

2021-02-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/TUScheduler.cpp:511 llvm::Optional> LatestPreamble; - std::queue PreambleRequests; /* GUARDED_BY(Mutex) */ + std::deque PreambleRequests; /* GUARDED_BY(Mutex) */ /// Signaled whenever LatestPreamble chan

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

2021-02-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95935/new/ https://reviews.llvm.org/D95935 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D96027: [clangd] Trace queue state for each TUScheduler action.

2021-02-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. LGTM! Comment at: clang-tools-extra/clangd/TUScheduler.cpp:1054 Requests.push_back({std::move(Task), std::string(Name), steady_clock::now(), -

[PATCH] D95942: [clangd] Deduplicate scopes in IncludeFixer queries

2021-02-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd1978fa4bf0d: [clangd] Deduplicate scopes in IncludeFixer queries (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95942/new/ https://r

[PATCH] D96123: [clangd] Expose actOnAllPArentDirectories helper

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. Will be used in other components that need

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