[PATCH] D90775: [clangd] ExternalIndex turns off BackgroundIndex only if it isn't set

2020-11-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:308 + if (C.Index.Background == Config::BackgroundPolicy::Auto) +C.Index.Background = Config::BackgroundPolicy::Skip; }); sammccall wrote: > doesn

[PATCH] D90748: [clangd] Introduce config parsing for External blocks

2020-11-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 304060. kadircet added a comment. - Preserve location of external block Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90748/new/ https://reviews.llvm.org/D90748 Files: clang-tools-extra/clangd/ConfigFragme

[PATCH] D90749: [clangd] Introduce config compilation for External blocks

2020-11-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 304064. kadircet marked 7 inline comments as done. kadircet added a comment. - Use ExternalBlock's range for diags. - Move diagnostic emitting to makeAbsolute. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D9074

[PATCH] D91131: [clangd] Bump index version number.

2020-11-10 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. Ah thanks, and sorry for missing it during the review :/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91131/new/ https://reviews.llvm.org/D

[PATCH] D89296: [clangd] Call hierarchy (Protocol layer)

2020-11-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Protocol.cpp:1210 + +llvm::json::Value toJSON(const CallHierarchyItem &I) { + llvm::json::Object Result{ we should also populate tags and detail conditionally. rather than providing empty stri

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

2020-11-10 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. Adds minimal cmake configuration required t

[PATCH] D91122: [clangd] Call hierarchy (XRefs layer, incoming calls)

2020-11-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:1317 -static Optional -symbolToTypeHierarchyItem(const Symbol &S, const SymbolIndex *Index, - PathRef TUPath) { that's a great change to have, but maybe leave

[PATCH] D91123: [clangd] Call hierarchy (ClangdServer layer)

2020-11-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. can you also add some tests to ClangdTests.cpp ? Comment at: clang-tools-extra/clangd/ClangdServer.cpp:695 +Callback>> CB) { + CB(clangd::incomingCalls(Item, Index)); +} why do we run this on the mainthread ? I suppose we should j

[PATCH] D91124: [clangd] Call hierarchy (ClangdLSPServer layer)

2020-11-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1228 +Callback>> Reply) { + Server->outgoingCalls(Params.Item, std::move(Reply)); +} as mentioned in the previous review, let's just reply with none/empty here, with a

[PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting

2020-11-11 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:636 +// Filter out any reformat fixits, we don't handle these. +// FIXME: Can we? +llvm::erase_if(FixIts, in theory yes, as we have access to source manager, we can f

[PATCH] D91124: [clangd] Call hierarchy (ClangdLSPServer layer)

2020-11-11 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/test/call-hierarchy.test:39 +--- +{"jsonrpc":"2.0","id":2,"method":"callHierarchy/incomingCalls","params":{"item":{"data":"F0E64FE3F8FEA480","kind":12,"name":"callee","range":{"end":{"character":16,"line":0},"st

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

2020-11-11 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/README.md:30 + We suggest building in RELEASE mode as building DEBUG binaries requires + considerably more resources. You can check [Building LLVM with CMake + documentation](https://llvm.org/docs/CMake.html)

[PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting

2020-11-11 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:677 // If requested and possible, create a message like "change 'foo' to 'bar'". -if (SyntheticMessage && FixIts.size() == 1) { - const auto &FixIt = FixIts.front(); +if (Synth

[PATCH] D91258: [clangd] Sanity-check array sizes read from disk before allocating them.

2020-11-11 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/Serialization.cpp:121 + template + LLVM_NODISCARD bool consumeSize(T &Container, unsigned MinSize = 1) { +auto Size = consumeVar(); regarding minsizes, i suppose the idea was to pas

[PATCH] D91299: [clangd] Also detect corrupt stri table size.

2020-11-11 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/index/Serialization.cpp:209 +// Theoretical max ratio from https://zlib.net/zlib_tech.html +constexpr int MaxCompressionR

[PATCH] D91258: [clangd] Sanity-check array sizes read from disk before allocating them.

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

[PATCH] D91330: [clangd] Ensure we test for compatibility of serialized index format

2020-11-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, wenlei, usaxena95, arphaman. Herald added a project: clang. kadircet requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Repository: rG LLVM Github Monore

[PATCH] D91330: [clangd] Ensure we test for compatibility of serialized index format

2020-11-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. As a side note, I've tested with a clangd-indexer before https://github.com/llvm/llvm-project/commit/71064b02701dd65065dd412fb01afe81ff83f746 and we got the failure (malformed/truncated refs). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D90750: [clangd] Introduce ProjectAwareIndex

2020-11-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 304795. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90750/new/ https://reviews.llvm.org/D90750 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/clangd/

[PATCH] D91258: [clangd] Sanity-check array sizes read from disk before allocating them.

2020-11-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. oops, this is actually a problem of the test. it is providing an invalid encoding :/ sent out https://reviews.llvm.org/D91405 to fix the issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91258/new/ https://reviews.llvm

[PATCH] D91405: [clangd] Assert on varint encoding

2020-11-13 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. 5th byte of a varint can't be bigger than 0

[PATCH] D91405: [clangd] Assert on varint encoding

2020-11-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 305055. kadircet added a comment. - Also fix the high byte Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91405/new/ https://reviews.llvm.org/D91405 Files: clang-tools-extra/clangd/index/Serialization.cpp

[PATCH] D91405: [clangd] Assert on varint encoding

2020-11-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 305094. kadircet marked an inline comment as done. kadircet added a comment. - Increase width of `B` to prevent integer promotion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91405/new/ https://reviews.llvm.

[PATCH] D91330: [clangd] Ensure we test for compatibility of serialized index format

2020-11-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 305096. kadircet added a comment. - Update comments - Query for Bar - Only run with zlib Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91330/new/ https://reviews.llvm.org/D91330 Files: clang-tools-extra/cla

[PATCH] D91330: [clangd] Ensure we test for compatibility of serialized index format

2020-11-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 305100. kadircet added a comment. - Drop comment markers - Make failure print the doc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91330/new/ https://reviews.llvm.org/D91330 Files: clang-tools-extra/clangd

[PATCH] D91330: [clangd] Ensure we test for compatibility of serialized index format

2020-11-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 305106. kadircet added a comment. - Drop subshell usage Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91330/new/ https://reviews.llvm.org/D91330 Files: clang-tools-extra/clangd/test/index-serialization/Inpu

[PATCH] D91405: [clangd] Assert on varint encoding

2020-11-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 305108. kadircet added a comment. - Only assert on the 5th byte, prior bytes can have any value. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91405/new/ https://reviews.llvm.org/D91405 Files: clang-tools-e

[PATCH] D91405: [clangd] Assert on varint encoding

2020-11-13 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 rG6e7dd1e3e117: [clangd] Assert on varint encoding (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D91330: [clangd] Ensure we test for compatibility of serialized index format

2020-11-13 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 rG8741a76f5dd1: [clangd] Ensure we test for compatibility of serialized index format (authored by kadircet). Repository: rG LLVM Github Monorepo CH

[PATCH] D91330: [clangd] Ensure we test for compatibility of serialized index format

2020-11-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. should be fixed with  https://github.com/llvm/llvm-project/commit/8dc2aa0e412171dad5cdc1aa60a92ddcd3800202. please let me know if it doesn't work. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91330/new/ https://reviews.l

[PATCH] D91330: [clangd] Ensure we test for compatibility of serialized index format

2020-11-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Looks like it is already green at http://lab.llvm.org:8011/#/builders/109/builds/2693 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91330/new/ https://reviews.llvm.org/D91330 _

[PATCH] D90750: [clangd] Introduce ProjectAwareIndex

2020-11-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 305695. kadircet marked 5 inline comments as done. kadircet added a comment. - Address comments - Only query the associated index - Use ExternalIndexSpec as the cache key Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D90750: [clangd] Introduce ProjectAwareIndex

2020-11-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/ProjectAware.cpp:100 + }); + addIndex(std::move(NewIndex)); + return PlaceHolder; sammccall wrote: > from memoize: > > > Concurrent calls for the same key may r

[PATCH] D91602: [clang-tidy] Make clang-format and include-order-check coherent

2020-11-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: hokein. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. kadircet requested review of this revision. LLVM style puts both gtest and gmock to the end of the include list. But llvm-include-order-check was only

[PATCH] D89296: [clangd] Call hierarchy (Protocol layer)

2020-11-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 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89296/new/ https://reviews.llvm.org/D89296 _

[PATCH] D91602: [clang-tidy] Make clang-format and include-order-check coherent

2020-11-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 305707. kadircet added a comment. - Add gmock header Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91602/new/ https://reviews.llvm.org/D91602 Files: clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp

[PATCH] D91122: [clangd] Call hierarchy (XRefs layer, incoming calls)

2020-11-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. mostly LG, i haven't looked at the tests yet though. Comment at: clang-tools-extra/clangd/XRefs.cpp:1344 + auto Result = declToHierarchyItem(ND); + if (Result) { +Result->deprecated = ND.isDeprecated(); nit: redundant braces ==

[PATCH] D91602: [clang-tidy] Make clang-format and include-order-check coherent

2020-11-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5a9f3867046c: [clang-tidy] Make clang-format and include-order-check coherent (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91602/new

[PATCH] D91602: [clang-tidy] Make clang-format and include-order-check coherent

2020-11-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp:1 // RUN: %check_clang_tidy %s llvm-include-order %t -- -- -isystem %S/Inputs/Headers njames93 wrote: > Would it be wise to specify a format style he

[PATCH] D91610: [clangd] Add OverriddenBy Relation to index.

2020-11-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/test/index-serialization/Inputs/sample.cpp:6 +struct Bar : public Foo { + void Func() override {} +}; could you also add a comment for the reason we have this in the test (i.e. Introduces an

[PATCH] D91297: Frontend: Take VFS and MainFileBuffer by reference in PrecompiledPreamble::CanReuse, NFC

2020-11-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang/include/clang/Frontend/PrecompiledPreamble.h:108 bool CanReuse(const CompilerInvocation &Invocation, -const llvm::MemoryBuffer *MainFileBuffer, PreambleBounds Bounds, -llvm::vfs::FileSystem *VFS

[PATCH] D90750: [clangd] Introduce ProjectAwareIndex

2020-11-19 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/D90750/new/ https://reviews.llvm.org/D90750 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D89571: [clangd] Add textDocument/ast extension method to dump the AST

2020-11-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/DumpASTTests.cpp:12 +#include "TestTU.h" +#include "llvm/Support/ScopedPrinter.h" +#include "gmock/gmock.h" sammccall wrote: > adamcz wrote: > > include order lint warning here > Not s

[PATCH] D90750: [clangd] Introduce ProjectAwareIndex

2020-11-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 306615. kadircet marked 6 inline comments as done. kadircet added a comment. - Internalize synchronization of indexstorage rather than using Memoize Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90750/new/ htt

[PATCH] D90751: [clangd] Use ProjectAwareIndex

2020-11-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 306621. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90751/new/ https://reviews.llvm.org/D90751 Files: clang-tools-extra/clangd/tool/ClangdMain.cpp Index: clang-tools-e

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

2020-11-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 306633. kadircet marked 2 inline comments as done. kadircet added a comment. - Mention assertions and ninja Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91186/new/ https://reviews.llvm.org/D91186 Files: cl

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

2020-11-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/README.md:26 +- Create a build directory, for example at `$LLVM_ROOT/build`. +- Inside the build directory run: `cmake $LLVM_ROOT/llvm/ + -DCMAKE_BUILD_TYPE=RELEASE -DLLVM_ENABLE_PROJECTS="clang;clang-tools-ext

[PATCH] D91859: [clangd] Fix shared-lib builds

2020-11-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added reviewers: sammccall, kbobyrev. Herald added subscribers: cfe-commits, usaxena95, arphaman, mgorny. Herald added a project: clang. kadircet requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Repository: rG LLVM Gith

[PATCH] D91860: [clangd] Move remote-index dependency from clangDaemon to ClangdMain

2020-11-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, mgorny. Herald added a project: clang. kadircet requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. This is a hack to prevent cyclic de

[PATCH] D88338: [clangd] clangd --check: standalone diagnosis of common problems

2020-09-29 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. thanks! this mostly looks good, as discussed offline I believe having an infra that we can improve over time is better than not having anything until we've got the "perfect" solution. i just got a couple of questions about some pieces, and some nits. ===

[PATCH] D88507: [clangd][remote] Make sure relative paths are absolute with respect to posix style

2020-09-29 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: kbobyrev. Herald added subscribers: cfe-commits, usaxena95, arphaman. Herald added a project: clang. kadircet requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Relative paths received from the server are

[PATCH] D88297: [clangd] Trivial setter support when moving items to fields

2020-09-29 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:466 +auto *ND = llvm::dyn_cast(CE->getCalleeDecl()); +if (!ND) + return llvm::None; njames93 wrote: > kadircet wrote: > > nit: combine wit

[PATCH] D88507: [clangd][remote] Make sure relative paths are absolute with respect to posix style

2020-09-30 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 rG64e8fd540ecc: [clangd][remote] Make sure relative paths are absolute with respect to posix… (authored by kadircet). Repository: rG LLVM Github Mon

[PATCH] D88567: [clangd] Fix invalid UTF8 when extracting doc comments.

2020-09-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! Should we also have another test for SymbolCollector, to ensure we don't regress this somehow in the future? Comment at: clang-tools-extra/clangd/CodeComp

[PATCH] D88338: [clangd] clangd --check: standalone diagnosis of common problems

2020-10-01 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/tool/Check.cpp:107 + +Cmd = CDB->getFallbackCommand(File); +if (auto TrueCmd = CDB->getCompileCommand(File)) { -

[PATCH] D88567: [clangd] Fix invalid UTF8 when extracting doc comments.

2020-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:93 + // Clang requires source to be UTF-8, but doesn't enforce this in comments. + if (!llvm::json::isUTF8(Doc)) +Doc = llvm::json::fixUTF8(Doc); sammccall wrote:

[PATCH] D88721: [clangd][lit] Update document-link.test to respect custom resource-dir locations

2020-10-02 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. Repository: rG LLVM Github Monorepo http

[PATCH] D87256: [clangd] Avoid relations being overwritten in a header shard

2020-10-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp:232 +TEST_F(BackgroundIndexTest, RelationsMultiFile) { + MockFS FS; kadircet wrote: > do we still need this test? this one was marked as resolved but i stil

[PATCH] D88411: [clangd] Introduce MemoryTrees

2020-10-02 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/support/MemoryTree.cpp:28 +size_t ChildrenSize = 0; +for (const auto &C : Children) { + C.traverseTree(CollapseDetails, sammccall wrote: >

[PATCH] D88721: [clangd][lit] Update document-link.test to respect custom resource-dir locations

2020-10-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 295779. kadircet added a comment. - update comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88721/new/ https://reviews.llvm.org/D88721 Files: clang-tools-extra/clangd/test/document-link.test Index: c

[PATCH] D88721: [clangd][lit] Update document-link.test to respect custom resource-dir locations

2020-10-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 rG54c03d8f7da7: [clangd][lit] Update document-link.test to respect custom resource-dir locations (authored by kadircet). Repository: rG LLVM Github

[PATCH] D85354: [clangd] Reduce availability of extract function

2020-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296110. kadircet marked 2 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85354/new/ https://reviews.llvm.org/D85354 Files: clang-tools-ex

[PATCH] D88411: [clangd] Introduce MemoryTrees

2020-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296111. 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/D88411/new/ https://reviews.llvm.org/D88411 Files: clang-tools-ex

[PATCH] D88413: [clangd] Add a metric for tracking memory usage

2020-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296112. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88413/new/ https://reviews.llvm.org/D88413 Files: clang-tools-extra/clangd/support/Trace.cpp clang-tools-extra/clan

[PATCH] D88414: [clangd] Introduce memory dumping to FileIndex, FileSymbols and BackgroundIndex

2020-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296113. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88414/new/ https://reviews.llvm.org/D88414 Files: clang-tools-extra/clangd/index/Background.cpp clang-tools-extra/c

[PATCH] D88415: [clangd] Introduce memory usage dumping to TUScheduler, for Preambles and ASTCache

2020-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296114. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88415/new/ https://reviews.llvm.org/D88415 Files: clang-tools-extra/clangd/TUScheduler.cpp clang-tools-extra/clangd

[PATCH] D88417: [clangd] Record memory usages after each notification

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

[PATCH] D87256: [clangd] Avoid relations being overwritten in a header shard

2020-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! (and sorry for taking too long on this one.) Comment at: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp:232 +TEST_F(BackgroundIndexTest, RelationsM

[PATCH] D88814: [clangd] Enable partial namespace matches for workspace symbols

2020-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. 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 queries like "clangd::" to find symbols under clangd namespace,

[PATCH] D88828: [clangd] Verify the diagnostic code in include-fixer diagnostic tests, NFC.

2020-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. LGTM, but can you add some description about why you've decided to do it now :D Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88828/new/ htt

[PATCH] D88844: [clangd] Add `score` extension to workspace/symbol response.

2020-10-06 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/FindSymbols.cpp:125 +Info.score = Score / Relevance.NameMatch; Top.push({Score, std::move(Info)}); ni

[PATCH] D88814: [clangd] Enable partial namespace matches for workspace symbols

2020-10-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296402. kadircet marked 6 inline comments as done. kadircet added a comment. - Perform sub-scope matching rather than substring match for partial namespaces. - Apply a penalty for partially matching namespaces. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D88885: [clangd] Disambiguate overloads of std::move for header insertion.

2020-10-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:563 if (auto Header = getIncludeHeader(QName, Entry.second)) { +// Hack: there are two std::move() overloads from different headers. +// CanonicalIncludes returns

[PATCH] D88885: [clangd] Disambiguate overloads of std::move for header insertion.

2020-10-06 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! As you mentioned I believe `std::move` is common enough to deserve the special casing. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:563

[PATCH] D88814: [clangd] Enable partial namespace matches for workspace symbols

2020-10-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/FindSymbols.cpp:44 +// Returns true if \p Query can be found as a sub-scope inside \p Scope. +bool approximateScopeMatch(llvm::StringRef Scope, sammccall wrote: > I had a little trouble follow

[PATCH] D85354: [clangd] Reduce availability of extract function

2020-10-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:291 + llvm::SmallSet DeclsInExtZone; + for (const Node *Child : ExtZone.Parent->Children) { +auto *RootStmt = Child->ASTNode.get(); sammccall wrote: > I

[PATCH] D88964: [clangd] Add a missing include-fixer test for incomplete_type, NFC.

2020-10-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeFixer.cpp:71 switch (Info.getID()) { + case diag::err_incomplete_base_class: case diag::err_incomplete_type: nit: maybe revert this change, or put `err_incomplete_type` to the last

[PATCH] D88411: [clangd] Introduce MemoryTrees

2020-10-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296667. kadircet marked 9 inline comments as done. kadircet added a comment. - Rename add{Detail,Child} -> {detail,child} - Get rid of `traverseTree` API and only expose `total` - Add a `children()` getter Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D88411: [clangd] Introduce MemoryTrees

2020-10-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/support/MemoryTree.h:36 + /// No copy of the \p Name. + MemoryTree *addChild(llvm::StringLiteral Name) { return &createChild(Name); } + sammccall wrote: > sammccall wrote: > > actually, why do

[PATCH] D88411: [clangd] Introduce MemoryTrees

2020-10-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296675. kadircet added a comment. - Add self Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88411/new/ https://reviews.llvm.org/D88411 Files: clang-tools-extra/clangd/support/CMakeLists.txt clang-tools-ext

[PATCH] D88411: [clangd] Introduce MemoryTrees

2020-10-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296677. kadircet marked 2 inline comments as done. kadircet added a comment. - Make MemoryTree move-only and return refs instead of pointers on child and detail. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D8

[PATCH] D88413: [clangd] Add a metric for tracking memory usage

2020-10-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296678. kadircet added a comment. - Rebase and introduce helper for recording a MemoryTree. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88413/new/ https://reviews.llvm.org/D88413 Files: clang-tools-extra/

[PATCH] D88414: [clangd] Introduce memory dumping to FileIndex, FileSymbols and BackgroundIndex

2020-10-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296684. kadircet marked 4 inline comments as done. kadircet added a comment. - Rename attachMemoryUsage to profile - Split file symbols into symbols/refs/relations granularity - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D88415: [clangd] Introduce memory usage dumping to TUScheduler, for Preambles and ASTCache

2020-10-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296691. kadircet marked 3 inline comments as done. kadircet added a comment. - Rename attachMemoryUsage to profile - Group by filename, rather than ast_cache vs preamble - Make use of UsedBytesAST rather than profiling IdleASTs - Rebase Repository: rG LLV

[PATCH] D88417: [clangd] Record memory usages after each notification

2020-10-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296696. kadircet added a comment. - Rebase and add tests for ClangdLSPServer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88417/new/ https://reviews.llvm.org/D88417 Files: clang-tools-extra/clangd/ClangdLS

[PATCH] D88885: [clangd] Disambiguate overloads of std::move for header insertion.

2020-10-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:557 // :-( llvm::SmallString<256> QName; for (const auto &Entry : IncludeFiles) this one is no longer needed. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D88964: [clangd] Add a missing include-fixer test for incomplete_type, NFC.

2020-10-08 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. still LGTM Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:715 auto TU = TestTU::withCode(Test.code()); + TU.ExtraArgs.push_back("-std=c++17");

[PATCH] D88411: [clangd] Introduce MemoryTrees

2020-10-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. > Still LGTM, commit it already :-D Haha, waiting for dependents to be stamped as well :D Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88411/new/ https://reviews.llvm.org/D88411 _

[PATCH] D88411: [clangd] Introduce MemoryTrees

2020-10-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296910. kadircet marked an inline comment as done. kadircet added a comment. - Inline MemoryTree::self Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88411/new/ https://reviews.llvm.org/D88411 Files: clang-t

[PATCH] D88414: [clangd] Introduce memory dumping to FileIndex, FileSymbols and BackgroundIndex

2020-10-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D88414#2317106 , @sammccall wrote: > Now there's lots of usage (which looks good!) i'm finding it a bit hard to > keep track of what the tree will look like overall. > > At some point it'd be great to: > a) bind this to an LS

[PATCH] D88417: [clangd] Record memory usages after each notification

2020-10-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296955. 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/D88417/new/ https://reviews.llvm.org/D88417 Files: clang-tools-ex

[PATCH] D88417: [clangd] Record memory usages after each notification

2020-10-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet requested review of this revision. kadircet added a comment. Bad news, I was testing this with remote-index, hence background-index was turned off. Unfortunately traversing all of the slabs in `FileSymbols` takes quite a while in this case (~15ms for LLVM). I don't think it is feasible

[PATCH] D89036: [clangd] Add more incomplete_type diagnostics that could be fixed by include-fixer.

2020-10-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Tests seem to be failing on premerge bots: - https://reviews.llvm.org/harbormaster/unit/view/177841/ - https://reviews.llvm.org/harbormaster/unit/view/177842/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89036/new/ https

[PATCH] D88414: [clangd] Introduce memory dumping to FileIndex, FileSymbols and BackgroundIndex

2020-10-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296971. kadircet added a comment. - Fix a bug in FileSymbols profiling. - Add unittests to ensure tree structure is as expected. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88414/new/ https://reviews.llvm.or

[PATCH] D88814: [clangd] Enable partial namespace matches for workspace symbols

2020-10-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 296992. kadircet marked 3 inline comments as done. kadircet added a comment. - Switch from substring to subscope matching. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88814/new/ https://reviews.llvm.org/D888

[PATCH] D88814: [clangd] Enable partial namespace matches for workspace symbols

2020-10-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/FindSymbols.cpp:44 +// Returns true if \p Query can be found as a sub-scope inside \p Scope. +bool approximateScopeMatch(llvm::StringRef Scope, sammccall wrote: > kadircet wrote: > > sammccall

[PATCH] D85354: [clangd] Reduce availability of extract function

2020-10-09 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 rG2ff44935a5f5: [clangd] Reduce availability of extract function (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D88814: [clangd] Enable partial namespace matches for workspace symbols

2020-10-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 297142. kadircet marked an inline comment as done. kadircet added a comment. - Move if checks into asserts. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88814/new/ https://reviews.llvm.org/D88814 Files: cl

[PATCH] D88814: [clangd] Enable partial namespace matches for workspace symbols

2020-10-09 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 rG6f1a56d37ac6: [clangd] Enable partial namespace matches for workspace symbols (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D88413: [clangd] Add a metric for tracking memory usage

2020-10-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 297172. kadircet added a comment. - Move MemoryTree recording from Trace into MemoryTree as suggested in D88417 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88413/new/ http

<    15   16   17   18   19   20   21   22   23   24   >