[PATCH] D89852: [clangd] Get rid of llvm::Optional in Remote- and LocalIndexRoot; NFC

2020-10-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/index/remote/marshalling/Marshalling.cpp:60 +llvm::StringRef Path(this->RemoteIndexRoot); if

[PATCH] D89935: [clangd] NFC: Update FIXME comment regarding lack of c/dtor support

2020-10-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. > I thought the point of the comment was us not handling it properly rather > than LSP not supporting it (e.g. LSP does support Operator but we do not). > Then, the comment about ctor and dtor being indistinguishable probably > belongs to Protocol.h/cpp and SymbolKind

[PATCH] D89935: [clangd] NFC: Update FIXME comment regarding lack of c/dtor support

2020-10-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. > Yeah but the LSP SymbolKind which we are converting to does not have a > destructor type, same thing with CompletionItemKind, so I guess we really do > treat ctors and dtors the same way from the LSP perspective, aren't we? Yes, and that's what the previous fixme

[PATCH] D89852: [clangd] Use SmallString instead of std::string in marshalling code; NFC

2020-10-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Sorry I don't really follow what we are gaining by changes from string to SmallString. Could you explain if I am missing something? But I think it makes sense to get rid of Optionals in the Marshaller for `{Local,Remote}IndexRoot`, as there's no difference between

[PATCH] D89935: [clangd] NFC: Update FIXME comment regarding lack of c/dtor support

2020-10-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Am I missing something? We still have: case index::SymbolKind::Constructor: case index::SymbolKind::Destructor: return SymbolKind::Constructor; in Protocol.cpp. E.g. Constructors and Destructors are still classified badly. I suppose the bit around `they're all

[PATCH] D89862: [clangd] Give the server information about client's remote index protocol version

2020-10-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Regarding versioning of grpc layer. In addition to including a version number in every request, looks like there's the concept of "versioned-services". So we basically change the package name to be versioned, i.e. `package clang.clangd.remote.v1` and every time we

[PATCH] D89297: [clangd] Add a TestWorkspace utility

2020-10-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. thanks, lgtm! Comment at: clang-tools-extra/clangd/unittests/TestWorkspace.cpp:17 + for (const auto : Inputs) { +if (!Input.second.IsMainFile) { + continue;

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

2020-10-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:349 +SymbolRef{SM.getFileLoc(Loc), Roles, + dyn_cast_or_null(ASTNode.Parent)}); // Don't continue indexing if this is a mere reference.

[PATCH] D89685: [clangd] Rename edge name for filesymbols to slabs in memorytree

2020-10-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG40749141030b: [clangd] Rename edge name for filesymbols to slabs in memorytree (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D89297: [clangd] Add a TestWorkspace utility

2020-10-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. thanks this mostly looks good. can you move implementations from TestWorkspace.h to TestWorkspace.cpp ? Comment at: clang-tools-extra/clangd/unittests/FileIndexTests.cpp:431 +TEST(FileIndexTest, RelationsMultiFile) { + TestWorkspace

[PATCH] D89685: [clangd] Rename edge name for filesymbols to slabs in memorytree

2020-10-19 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 was causing duplicate `symbols`

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

2020-10-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 rGd0f287464d8a: [clangd] Add $/memoryUsage LSP extension (authored by kadircet). Changed prior to commit:

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

2020-10-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 298644. kadircet added a comment. s/memoryTree/memoryUsage/g Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89277/new/ https://reviews.llvm.org/D89277 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

2020-10-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 298578. kadircet marked 5 inline comments as done. kadircet added a comment. - Add memoryTreeProvider to initialization params - Move serialization logic to Protocol.h and return MemoryTree directly. - Drop `dump` from method name. Repository: rG LLVM

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

2020-10-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 298460. kadircet added a comment. - As discussed offline moving with compact version of the output, while preserving names starting with an `_` to be leaves. This is also ready for an implementation-wise review now. Repository: rG LLVM Github Monorepo

[PATCH] D89297: [clangd] Add a TestWorkspace utility

2020-10-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. This looks like a useful infra to have indeed, we currently don't have an easy way to setup a testcase that would require interactions between multiple files (e.g. a workspace), as you've noticed while working on callhierarchy tests (sorry for that). A couple of

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

2020-10-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D89277#2329947 , @sammccall wrote: > (sorry out today and haven't looked at code yet) no worries it is a prototype, I wouldn't spend time looking at the implementation until we agree on the interaction :D OTHO, checking out

[PATCH] D89307: [clangd] Disable extract variable for RHS of assignments

2020-10-14 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 rG82a71822a54d: [clangd] Disable extract variable for RHS of assignments (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D89307: [clangd] Disable extract variable for RHS of assignments

2020-10-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: 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. Repository: rG LLVM Github Monorepo

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

2020-10-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D89277#2326376 , @nridge wrote: > This is neat! Are there plans to add vscode client support to invoke this? Yes, since we control the plugin in vscode we should have a way to invoke it there. (we already have support for

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

2020-10-12 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. Performs a detailed profiling of clangd

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

2020-10-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 rG23a53301c545: [clangd] Introduce memory usage dumping to TUScheduler, for Preambles and… (authored by kadircet). Repository: rG LLVM Github

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

2020-10-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 rG35871fde55ac: [clangd] Record memory usages after each notification (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE

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

2020-10-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 rGa74d59494861: [clangd] Introduce memory dumping to FileIndex, FileSymbols and BackgroundIndex (authored by kadircet). Repository: rG LLVM Github

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

2020-10-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 rGc9d2876da95c: [clangd] Add a metric for tracking memory usage (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D88411: [clangd] Introduce MemoryTrees

2020-10-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 rGf9317f7bf6bd: [clangd] Introduce MemoryTrees (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2020-10-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 297568. kadircet marked an inline comment as done. kadircet added a comment. - Update stale comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88417/new/ https://reviews.llvm.org/D88417 Files:

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

2020-10-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 297565. kadircet marked 5 inline comments as done. kadircet added a comment. - Separate profiling and exporting into 2 functions - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88417/new/

[PATCH] D89135: [clangd] Stop capturing trace args if the tracer doesn't need them.

2020-10-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. still LGTM. thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89135/new/ https://reviews.llvm.org/D89135 ___ cfe-commits mailing list

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

2020-10-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 297542. kadircet marked an inline comment as done. kadircet added a comment. - Make traversal a free function and take metric to populate as a parameter. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88204: [clangd] Drop path suffix mapping for std symbols

2020-10-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 297528. kadircet added a comment. - Only drop STL mapping instead of getting rid of suffix mapping completely. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88204/new/ https://reviews.llvm.org/D88204 Files:

[PATCH] D88204: [clangd] Drop path suffix mapping for std symbols

2020-10-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done. kadircet added a comment. > We don't have an alternative database for these. Maybe we should keep the > mechanism but stop using it for the standard library? What do you think? Yes i think you are right. Just dropping STL mappings from the list.

[PATCH] D89135: [clangd] Stop capturing trace args if the tracer doesn't need them.

2020-10-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/support/Trace.cpp:91 public: -JSONSpan(JSONTracer *Tracer, llvm::StringRef Name, llvm::json::Object *Args) +

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

2020-10-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 297173. kadircet marked 2 inline comments as done. kadircet added a comment. - Implement periodic profiling Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88417/new/ https://reviews.llvm.org/D88417 Files:

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

2020-10-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. > What about something like a 5 minute throttle, but have ClangdLSPServer's > constructor set the timestamp to now+1 minute? (Without profiling) SGTM. Note that this means we can't easily test this in LSP layer anymore. (We've got couple of components depending on

[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/

[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

[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:

[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

[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: > >

[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/

[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/

[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/

[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

[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:

[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

[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:

[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] 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] 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 : IncludeFiles) this one is no longer needed. Repository: rG LLVM Github Monorepo CHANGES SINCE

[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:

[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

[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

[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:

[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

[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

[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

[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 (Name); } + sammccall wrote: > sammccall wrote: > > actually, why do these

[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

[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: >

[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

[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] 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

[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

[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)});

[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/

[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] 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,

[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

[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

[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

[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

[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:

[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:

[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] 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:

[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 : Children) { + C.traverseTree(CollapseDetails, sammccall wrote: >

[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

[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

[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

[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-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:

[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

[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

[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] 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] D88417: [clangd] Record memory usages after each notification

2020-09-28 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. Depends on D88415 Repository: rG LLVM

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

2020-09-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, javed.absar. Herald added a project: clang. kadircet requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. File-granular information is

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

2020-09-28 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. File-granular information is considered

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

2020-09-28 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

[PATCH] D88411: [clangd] Introduce MemoryTrees

2020-09-28 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. A structure that can be used to

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

2020-09-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:414 // If CMD is one of the forms: // void foo(T arg) { FieldName = arg; } // R foo(T arg) { FieldName = arg; return *this; } can you also update the docs?

[PATCH] D88204: [clangd] Drop path suffix mapping for std symbols

2020-09-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, jfb, arphaman. Herald added a project: clang. kadircet requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. As mentioned in D88144

[PATCH] D88144: [clangd] Disable suffix matching fallback for C during include insertion

2020-09-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG64168c6d996b: [clangd] Disable suffix matching fallback for C during include insertion (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88144: [clangd] Disable suffix matching fallback for C during include insertion

2020-09-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D88144#2290830 , @sammccall wrote: > (I do wonder whether it's safe to just drop the mapping table entirely now...) As discussed offline; It still helps with mapping symbols missing from our cppreference extraction back to

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Hey! Sorry for the late reply, this has been open in my tabs since day 1 just didn't get a chance to take a look at it. The biggest problem I see is, this is not changing the fact that we are still traversing the whole file: - You do one traversal over the whole file

[PATCH] D88144: [clangd] Disable suffix matching fallback for C during include insertion

2020-09-23 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. Clangd currently doesn't respect language

[PATCH] D88103: [JSON] Add error reporting facility, used in fromJSON and ObjectMapper.

2020-09-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! This looks great. I've mostly did the full review anyway but feel free to land in small patches just in case some compiler becomes upset and you need to revert. Mostly nits, and

[PATCH] D86077: [clangd] Add a way for exporting memory usage metrics

2020-09-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet planned changes to this revision. kadircet added a comment. In D86077#2281218 , @sammccall wrote: > Just for the record, as I think Kadir and Adam are both aware... > > We discussed making this a bit richer and less reliant on static state. >

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

2020-09-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:152 + // Attribute relations to both their subject's and object's locations. + // See https://github.com/clangd/clangd/issues/510 for discussion of why. if (Index.Relations) {

  1   2   3   4   5   6   7   8   9   10   >