[PATCH] D43381: [clangd] Fix use-after-free in SymbolYAML: strings are owned by yaml::Input!

This revision was automatically updated to reflect the committed changes. Closed by commit rL325476: [clangd] Fix use-after-free in SymbolYAML: strings are owned by yaml::Input! (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://rev

[PATCH] D43379: [CodeComplete] Add a helper to print CodeCompletionContext::Kind

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: include/clang/Sema/CodeCompleteConsumer.h:355 +/// \brief Get string representation of \p Kind, useful for for debugging. +llvm::StringRef printCompleti

[PATCH] D43385: [clangd] Add "clangd.trace" VSCode setting to enable tracing.

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rCTE325477: [clangd] Add "clangd.trace" VSCode setting to enable tracing. (authored by sammccall, committed by ). Changed p

[PATCH] D43388: [clangd] Tracing: name worker threads, and enforce naming scheduled async tasks

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE325480: [clangd] Tracing: name worker threads, and enforce naming scheduled async tasks (authored by sammccall, committed by ). Changed prior to commit: https://reviews.llvm.org/D43388?vs=134617&id=13

[PATCH] D43388: [clangd] Tracing: name worker threads, and enforce naming scheduled async tasks

This revision was automatically updated to reflect the committed changes. Closed by commit rL325480: [clangd] Tracing: name worker threads, and enforce naming scheduled async tasks (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://

[PATCH] D43452: [clangd] Bump vs-code clangd extension v0.0.3

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. (Do we need to bump this for the market to update properly? How do we decide when to bump it?) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43452 _

[PATCH] D43379: [CodeComplete] Add a helper to print CodeCompletionContext::Kind

sammccall added inline comments. Comment at: include/clang/Sema/CodeCompleteConsumer.h:355 +/// \brief Get string representation of \p Kind, useful for for debugging. +llvm::StringRef printCompletionKind(enum CodeCompletionContext::Kind Kind); + ilya-biryukov wro

[PATCH] D43455: [clangd] Add brief instructions on how to make a release for vscode-clangd extension.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. thanks! couple of nits Comment at: clangd/clients/clangd-vscode/README.md:55 + +Everytime you make a change to `clangd-vscode`, you are expected to publish a +new versi

[PATCH] D43462: [clangd] Fixes for #include insertion.

sammccall added inline comments. Comment at: clangd/Headers.cpp:42-43 }; +/// Returns true if \p Path has header extensions like .h and .hpp etc. +bool hasHeaderExtension(PathRef Path) { As discussed offline, this seems dubious. - this is probably the generali

[PATCH] D41102: Setup clang-doc frontend framework

sammccall added a comment. Naming conventions tend to stick around for a while - `clang-doc/ClangDocXYZ.h` seems a bit unwieldy compared to `clang-doc/XYZ.h` - might be worth considering. Comment at: clang-doc/ClangDoc.cpp:37 + Context = Result.Context; + if (const auto *M =

[PATCH] D43462: [clangd] Fixes for #include insertion.

sammccall added inline comments. Comment at: clangd/Headers.h:26 /// /// \param Header is an absolute file path. +/// \return A quoted "path" or . This returns an empty string if: File also needs to be absolute. (May want to add asserts for this at the start of

[PATCH] D43510: [clangd] don't insert new includes if either original header or canonical header is already included.

sammccall added inline comments. Comment at: clangd/ClangdServer.h:239 + /// Inserts a new #include into \p File, if it's not present in \p Code. + /// \p OriginalHeader The original header corresponding to this insertion. + /// This may be different from preferredHeader as a

[PATCH] D43510: [clangd] don't insert new includes if either original header or canonical header is already included.

sammccall added a comment. Mostly nits around naming/doc: you've convinced me that the two headers that are paths or uris or at different times is the right thing, but we need to be really clear about it. Comment at: clangd/ClangdServer.cpp:320 +S

[PATCH] D43518: [clangd] Allow embedders some control over when diagnostics are generated.

sammccall created this revision. sammccall added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, ioeric, jkorous-apple, klimek. Through the C++ API, we support for a given snapshot version: - Yes: make sure we generate diagnostics for exactly this version - Auto: generate eventu

[PATCH] D43518: [clangd] Allow embedders some control over when diagnostics are generated.

sammccall updated this revision to Diff 135382. sammccall marked an inline comment as done. sammccall added a comment. add param names to decls Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43518 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServ

[PATCH] D43518: [clangd] Allow embedders some control over when diagnostics are generated.

sammccall added a comment. Not convinced about the flow control stuff - I think you might be underestimating the complexity of the cancellation-based approach with the extra functionality. But if you think it would still be more readable the other way, happy to get a third opinion. =

[PATCH] D43518: [clangd] Allow embedders some control over when diagnostics are generated.

This revision was automatically updated to reflect the committed changes. Closed by commit rL325774: [clangd] Allow embedders some control over when diagnostics are generated. (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://revie

[PATCH] D43634: [clangd] Extend textDocument/didChange to specify whether diagnostics should be generated.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Nice, this will be useful for at least a couple of editor integrations. Comment at: clangd/Protocol.h:301 + + /// If this is not set, diagnostics will be generated fo

[PATCH] D43648: [clangd] Debounce streams of updates.

sammccall created this revision. sammccall added reviewers: hokein, ilya-biryukov. Herald added subscribers: cfe-commits, ioeric, jkorous-apple, klimek. Don't actually start building ASTs for new revisions until either: - 500ms have passed since the last revision, or - we actually need the revisi

[PATCH] D43510: [clangd] don't insert new includes if either original header or canonical header is already included.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks, and sorry for the delay! Comment at: clangd/Headers.cpp:45 private: - std::set &Headers; + std::set &WrittenHeaders; + std::set &ResolvedHeaders; --

[PATCH] D43648: [clangd] Debounce streams of updates.

sammccall added inline comments. Comment at: clangd/TUScheduler.cpp:324 +if (*Deadline) + RequestsCV.wait_until(Lock, **Deadline); +else ilya-biryukov wrote: > It looks like if we unwrap `Optional` to `Deadline`, we could > replace this

[PATCH] D43648: [clangd] Debounce streams of updates.

sammccall updated this revision to Diff 135660. sammccall marked 3 inline comments as done. sammccall added a comment. Make Deadline a real type with 0/finite/inf semantics. Pull out another wait() overload. Expose debounce delay option in clangdserver. Repository: rCTE Clang Tools Extra http

[PATCH] D43648: [clangd] Debounce streams of updates.

sammccall updated this revision to Diff 135661. sammccall added a comment. rebase Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43648 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/TUScheduler.cpp clangd/TUScheduler.h clangd/Threading.cpp clangd/Thread

[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks for the split & refactoring! This looks nice. Just nits, plus some thoughts on the test. Comment at: clangd/index/Index.cpp:11 #include "Index.h" - +#include "

[PATCH] D40488: [clangd] Implemented tracing using Context

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/Trace.cpp:123 + // Clone the context, so that the original Context can be moved. + this->Ctx.emplace(Ctx.clone()); + This is a

[PATCH] D41178: [clangd] Construct SymbolSlab from YAML format.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/index/Index.h:17 #include "llvm/ADT/StringExtras.h" +#include "llvm/Support/YAMLTraits.h" This isn't needed anymore (if you r

[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.

sammccall accepted this revision. sammccall added inline comments. Comment at: unittests/clangd/IndexTests.cpp:29 + +struct SlabAndPointers { + SymbolSlab Slab; this could be local to generateNumSymbols, up to you Comment at: unittests/clangd/

[PATCH] D41232: [clangd] Add a FileSymbols container that manages symbols from multiple files.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/index/FileSymbols.h:39 + + /// \brief Removes snapshots of \p Path. + void remove(PathRef Path); Consider just accepting `upda

[PATCH] D41276: [clangd] Build in-memory index on symbols in files.

sammccall added inline comments. Comment at: clangd/index/FileMemIndexManager.h:23 +/// all symbols. +class FileMemIndexManager { +public: Discussed offline a bit: - FileIndex or PerFileIndex is a great name for this, if it implements `Index`. So I think it sho

[PATCH] D40548: [clangd] Symbol index interfaces and an in-memory index implementation.

sammccall added inline comments. Comment at: clangd/index/Index.h:134 + virtual bool + fuzzyFind(Context &Ctx, const FuzzyFindRequest &Req, +std::function Callback) const = 0; ioeric wrote: > malaperle wrote: > > ioeric wrote: > > > malaperle wrote:

[PATCH] D41276: [clangd] Build in-memory index on symbols in files.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. I don't see `FileSymbols.{h,cpp}` being deleted, but maybe I'm bad at reading diffs. Comment at: clangd/index/FileIndex.h:8 +// +//===-

[PATCH] D41280: [clangd] Don't use the optional "severity" when comparing Diagnostic.

sammccall added a comment. Can you give some more details about the problem (maybe offline)? In my testing, I do see a FixIt in VSCode. It fails to apply, for reasons I don't yet understand. Comment at: clangd/Protocol.h:326 - friend bool operator==(const Diagnostic &LHS, c

[PATCH] D41281: [clangd] Index-based code completion.

sammccall added a comment. A few drive-by comments, will look deeper if I have time but don't wait for me. Comment at: clangd/Position.cpp:34 + int Lines = JustBefore.count('\n'); + int Cols = JustBefore.size() - JustBefore.rfind('\n') - 1; + return {Lines, Cols}; --

[PATCH] D41343: [clangd] in VSCode client, filter extensions properly and only accept file: URIs

sammccall created this revision. sammccall added a reviewer: hokein. Herald added subscribers: cfe-commits, ilya-biryukov, klimek. The filtering wasn't previously working as intended - the string list is interpreted as a list of editor modes, not file extensions. (It happens to mostly work as "c"

[PATCH] D41281: [clangd] Index-based code completion.

sammccall added a comment. Hope you don't mind me piling on... let me know! Mostly readability, but i'd also like to reduce the scope w.r.t namespace completion. Comment at: clangd/CodeComplete.cpp:90 +CompletionItemKind getKindOfSymbol(index::SymbolKind Kind) { + using SK

[PATCH] D41343: [clangd] in VSCode client, filter extensions properly and only accept file: URIs

This revision was automatically updated to reflect the committed changes. Closed by commit rL320972: [clangd] in VSCode client, filter extensions properly and only accept file: URIs (authored by sammccall, committed by ). Repository: rL LLVM https://reviews.llvm.org/D41343 Files: clang-tool

[PATCH] D41343: [clangd] in VSCode client, filter extensions properly and only accept file: URIs

sammccall updated this revision to Diff 127320. sammccall added a comment. Fix quotes Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41343 Files: clangd/clients/clangd-vscode/src/extension.ts Index: clangd/clients/clangd-vscode/src/extension.ts =

[PATCH] D41351: [clangd] Expose offset <-> LSP position functions, and fix bugs

sammccall created this revision. sammccall added a reviewer: ioeric. Herald added subscribers: cfe-commits, ilya-biryukov, mgorny, klimek. - Moved these functions to SourceCode.h - added unit tests - fix off by one in positionToOffset: Offset - 1 in final calculation was wrong - fixed formatOnType

[PATCH] D41367: [clangd] Support filtering by fixing scopes in fuzzyFind.

sammccall added a comment. Nice, thanks! Some suggestions for further tightening the semantics, I'm happy to do these as a followup if you think it's too invasive here. Comment at: clangd/index/Index.h:127 /// \brief A query string for the fuzzy find. This is matched agains

[PATCH] D35894: [clangd] Code hover for Clangd

sammccall added inline comments. Comment at: clangd/ClangdUnit.cpp:941 createInvocationFromCommandLine(ArgStrs, CommandLineDiagsEngine, VFS); + CI->LangOpts->CommentOpts.ParseAllComments = true; // createInvocationFromCommandLine sets DisableFree.

[PATCH] D41345: [clangd] Add more symbol information for code completion.

sammccall added inline comments. Comment at: clangd/index/Index.h:92 + // Documentation including comment for the symbol declaration. + std::string Documentation; AFAIK this information isn't needed for retrieval/scoring, just for display. LSP has `completio

[PATCH] D41367: [clangd] Support filtering by fixing scopes in fuzzyFind.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. just nits, all this stuff up to you Comment at: clangd/index/Index.h:131 + /// un-qualified identifiers and should not contain qualifiers like "::". If + /// any scop

[PATCH] D41351: [clangd] Expose offset <-> LSP position functions, and fix bugs

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE321073: [clangd] Expose offset <-> LSP position functions, and fix bugs (authored by sammccall, committed by ). Changed prior to commit: https://reviews.llvm.org/D41351?vs=127345&id=127495#toc Reposi

[PATCH] D41281: [clangd] Index-based code completion.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/CodeComplete.cpp:111 + case SK::Using: +return CompletionItemKind::Reference; + case SK::Function: ioeric wrote: > sammcca

[PATCH] D41391: [clangd] Use the clang-tools-extra as the official repo for `vscode-clangd` extension.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/clients/clangd-vscode/README.md:16 +To use `vscode-clangd` extension in VS Code, you need to install `vscode-clangd` +from VS Code extension mar

[PATCH] D41280: [clangd] Don't use the optional "severity" when comparing Diagnostic.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Sorry, I did miss it! Comment at: clangd/Protocol.h:326 +}; +/// A LSP-specific comparator used to find Disgnostic in a container like +/// std:map. ni

[PATCH] D41289: [clangd] Build dynamic index and use it for code completion.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. OK, this is pretty clean now! :-) Comment at: clangd/ClangdServer.cpp:139 + FileIdx(BuildDynamicSymbolIndex ? new FileIndex() : nullptr), + Units(FileIdx +

[PATCH] D41432: [clangd] Switch xrefs and documenthighlight to annotated-code unit tests. NFC

sammccall created this revision. sammccall added a reviewer: ioeric. Herald added subscribers: cfe-commits, ilya-biryukov, mgorny, klimek. The goal here is again to make it easier to read and write the tests. I've extracted `parseTextMarker` from CodeCompleteTests into an `Annotations` class, add

[PATCH] D41432: [clangd] Switch xrefs and documenthighlight to annotated-code unit tests. NFC

sammccall updated this revision to Diff 127674. sammccall added a comment. Remove test PrintTos for basic types after r321161. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41432 Files: test/clangd/definitions.test test/clangd/documenthighlight.test test/clangd/xrefs.tes

[PATCH] D41432: [clangd] Switch xrefs and documenthighlight to annotated-code unit tests. NFC

sammccall marked 2 inline comments as done. sammccall added inline comments. Comment at: unittests/clangd/Annotations.h:12 +// +//Annotations Example(R"cpp( +// int complete() { x.pri^ } // ^ indicates a point ioeric wrote: > Does this support

[PATCH] D41432: [clangd] Switch xrefs and documenthighlight to annotated-code unit tests. NFC

This revision was automatically updated to reflect the committed changes. sammccall marked 2 inline comments as done. Closed by commit rL321184: [clangd] Switch xrefs and documenthighlight to annotated-code unit tests. NFC (authored by sammccall, committed by ). Changed prior to commit: https:/

[PATCH] D41450: [clangd] Pull CodeCompletionString handling logic into its own file and add unit test.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/CompletionString.cpp:51 + std::string Result; + Result.reserve(Text.size()); // Assume '$', '}' and '\\' are rare. + for (const auto Character

[PATCH] D41454: [clangd] Add ClangdUnit diagnostics tests using annotated code.

sammccall created this revision. sammccall added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, mgorny, klimek. This adds checks that our diagnostics emit correct ranges in a bunch of cases, as promised in https://reviews.llvm.org/D41118. The diagnostics-preamble test is also c

[PATCH] D41345: [clangd] Add more symbol information for code completion.

sammccall added inline comments. Comment at: clangd/index/Index.h:105 + /// What to insert when completing this symbol (plain text version). + std::string CompletionPlainInsertText; + /// What to insert when completing this symbol (snippet version). insert tex

[PATCH] D41345: [clangd] Add more symbol information for code completion.

sammccall added inline comments. Comment at: clangd/index/Index.h:122 + + llvm::Optional Detail; + ioeric wrote: > sammccall wrote: > > I think you probably want a raw pointer rather than optional: > > - reduce the size of the struct when it's absent > > - mak

[PATCH] D41454: [clangd] Add ClangdUnit diagnostics tests using annotated code.

sammccall updated this revision to Diff 127843. sammccall added a comment. Tighten preprocessor test Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41454 Files: clangd/Protocol.cpp clangd/Protocol.h test/clangd/diagnostics-preamble.test unittests/clangd/CMakeLists.txt

[PATCH] D41483: [clangd] Index symbols share storage within a slab.

sammccall created this revision. sammccall added a reviewer: hokein. Herald added subscribers: cfe-commits, ilya-biryukov, klimek. Symbols are not self-contained - it's only safe to hand them out if you guarantee the lifetime of the underlying data. Before this lands, I'm going to measure the bef

[PATCH] D41483: [clangd] Index symbols share storage within a slab.

sammccall updated this revision to Diff 127849. sammccall added a comment. Don't intern unless the symbol was actually inserted. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41483 Files: clangd/index/Index.cpp clangd/index/Index.h unittests/clangd/FileIndexTests.cpp u

[PATCH] D41483: [clangd] Index symbols share storage within a slab.

sammccall marked an inline comment as done. sammccall added inline comments. Comment at: clangd/index/Index.h:136 + // Intern table for strings. Not StringPool as we don't refcount, just insert. + llvm::StringSet Strings; llvm::DenseMap Symbols; ilya-biryuk

[PATCH] D41483: [clangd] Index symbols share storage within a slab.

sammccall updated this revision to Diff 127877. sammccall marked 4 inline comments as done. sammccall added a comment. Comment changes requiested in review. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41483 Files: clangd/index/Index.cpp clangd/index/Index.h unittests/c

[PATCH] D41483: [clangd] Index symbols share storage within a slab.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE321272: [clangd] Index symbols share storage within a slab. (authored by sammccall, committed by ). Changed prior to commit: https://reviews.llvm.org/D41483?vs=127877&id=127879#toc Repository: rCTE

[PATCH] D41506: [clangd] Use Builder for symbol slabs, and use sorted-vector for storage

sammccall created this revision. sammccall added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, mgrang, klimek. This improves a few things: - the insert -> freeze -> read sequence is now enforced/communicated by the type system - SymbolSlab::const_iterator iterates over symbol

[PATCH] D41506: [clangd] Use Builder for symbol slabs, and use sorted-vector for storage

sammccall updated this revision to Diff 127918. sammccall added a comment. minor doc and code layout tweaks Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41506 Files: clangd/index/FileIndex.cpp clangd/index/Index.cpp clangd/index/Index.h clangd/index/SymbolCollector.cp

[PATCH] D41514: [Index] Reduce size of SymbolInfo struct.

sammccall created this revision. sammccall added a reviewer: akyrtzi. Herald added subscribers: cfe-commits, ilya-biryukov. This is currently 16 bytes, the patch reduces it to 4. (Building with clang on linux x84, I guess others are similar) The only subfield that might need a bigger type is Symb

[PATCH] D41514: [Index] Reduce size of SymbolInfo struct.

sammccall updated this revision to Diff 127986. sammccall added a comment. Avoid repeating uint8_t type for SymbolProperty/SymbolPropertySet. Repository: rC Clang https://reviews.llvm.org/D41514 Files: include/clang/Index/IndexSymbol.h lib/Index/IndexSymbol.cpp tools/libclang/CXIndexDa

[PATCH] D41495: [clangd] Skip function bodies when building the preamble

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Those are some impressive numbers! Excited to see this land, thanks for tracking down all the weird problems. Comment at: clangd/ClangdUnit.cpp:536 + // so we set

[PATCH] D41491: [clangd] Add a tool to build YAML-format global symbols.

sammccall added inline comments. Comment at: clangd/CMakeLists.txt:50 add_subdirectory(tool) +add_subdirectory(global-symbol-builder) I think generally we run `check-clang-tools` before committing - I guess it's OK not to have tests for this experimental tool,

[PATCH] D41491: [clangd] Add a tool to build YAML-format global symbols.

sammccall added a comment. This looks like it does the right thing, ISTM it has a few too many moving parts. I might be missing why they're necessary, or we might not want to bother polishing this - maybe you can give me more context offline. Repository: rCTE Clang Tools Extra https://revie

[PATCH] D41491: [clangd] Add a tool to build YAML-format global symbols.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks for the simplification! How long does it take to run over LLVM on your machine? Comment at: clangd/CMakeLists.txt:50 add_subdirectory(tool) +add_subdirectory(gl

[PATCH] D41491: [clangd] Add a tool to build YAML-format global symbols.

sammccall added inline comments. Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:45 +IndexOpts.SystemSymbolFilter = +index::IndexingOptions::SystemSymbolFilterKind::All; +IndexOpts.IndexFunctionLocals = false; hokein wrote: >

[PATCH] D41506: [clangd] Use Builder for symbol slabs, and use sorted-vector for storage

sammccall marked 6 inline comments as done. sammccall added a comment. Thanks! Comment at: clangd/index/Index.cpp:39 + [](const Symbol &S, const SymbolID &I) { + return S.ID == I; + }); --

[PATCH] D41506: [clangd] Use Builder for symbol slabs, and use sorted-vector for storage

sammccall updated this revision to Diff 128049. sammccall marked an inline comment as done. sammccall added a comment. Address review comments, and update new GlobalSymbolBuilder tool for new API. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41506 Files: clangd/global-symbo

[PATCH] D41514: [Index] Reduce size of SymbolInfo struct.

This revision was automatically updated to reflect the committed changes. Closed by commit rL321411: [Index] Reduce size of SymbolInfo struct. (authored by sammccall, committed by ). Repository: rL LLVM https://reviews.llvm.org/D41514 Files: cfe/trunk/include/clang/Index/IndexSymbol.h cfe

[PATCH] D41506: [clangd] Use Builder for symbol slabs, and use sorted-vector for storage

This revision was automatically updated to reflect the committed changes. Closed by commit rL321412: [clangd] Use Builder for symbol slabs, and use sorted-vector for storage (authored by sammccall, committed by ). Repository: rL LLVM https://reviews.llvm.org/D41506 Files: clang-tools-extra

[PATCH] D41661: [clangd] Don't navigate to forward class declaration when go to definition.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/XRefs.cpp:54 if (isSearchedLocation(FID, Offset)) - Decls.push_back(D); + Decls.push_back(ASTNode.OrigD); return true; --

[PATCH] D41575: [index] Return when DC is null in handleReference

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Sorry, I was hoping @akyrtzi would see this. I don't know enough about the code to know whether DC==null is an indicator of some other problem, but if this is crashing today then let's no

[PATCH] D41730: [clangd] Use ToolExecutor to write the global-symbol-builder tool.

sammccall added a comment. This makes `SymbolCollector` (production code) depend on YAML (explicitly experimental), as well as on Tooling interfaces. At least we should invert this by making the thing passed to SymbolCollector a `function` or so. Moreover, it's unfortunate we'd need to change S

[PATCH] D41345: [clangd] Add more symbol information for code completion.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Just some nits Comment at: clangd/index/Index.h:122 + + llvm::Optional Detail; + ioeric wrote: > sammccall wrote: > > ioeric wrote: > > > sammccall wr

[PATCH] D41668: [clangd] Add static index for the global code completion.

sammccall added inline comments. Comment at: clangd/ClangdLSPServer.h:40 + bool BuildDynamicSymbolIndex, + std::unique_ptr StaticIdx); is there a reason ClangdServer should own this? I can imagine this complicating life for em

[PATCH] D41345: [clangd] Add more symbol information for code completion.

sammccall accepted this revision. sammccall added inline comments. Comment at: clangd/index/SymbolYAML.cpp:78 +assert(io.getContext()); +Detail = static_cast(io.getContext()) + ->Allocate(); this removes the Detail object if it

[PATCH] D41668: [clangd] Add static index for the global code completion.

sammccall accepted this revision. sammccall added a comment. Ilya's suggestion to have a single index on the CodeCompleteOptions that encapsulates the merging seems reasonable to me, but not urgent. So let's get this in as-is and we can make that change whenever. Comment at: c

[PATCH] D41668: [clangd] Add static index for the global code completion.

sammccall accepted this revision. sammccall added a comment. Nice! LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41668 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D41668: [clangd] Add static index for the global code completion.

sammccall added inline comments. Comment at: clangd/CodeComplete.cpp:661 + if (CompletedName.SSInfo) { +// For a namespace scope specifier ("ns::"), we have disabled Sema from +// collecting completion results by setting IncludeNamespaceLevelDecls to I d

[PATCH] D41911: [clangd] Include scanner that finds compile commands for headers.

sammccall created this revision. sammccall added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, mgorny, klimek. Not enabled because we need a threadsafe way to change VFS working directories. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41911 Files: clang

[PATCH] D43648: [clangd] Debounce streams of updates.

sammccall marked 4 inline comments as done. sammccall added a comment. Thanks for catching all those things! Comment at: unittests/clangd/TUSchedulerTests.cpp:127 + /*ASTParsedCallback=*/nullptr, + /*UpdateDebounce=*/std::chrono::milliseconds(5

[PATCH] D43648: [clangd] Debounce streams of updates.

sammccall updated this revision to Diff 136045. sammccall marked an inline comment as done. sammccall added a comment. review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43648 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h cl

[PATCH] D43823: [clangd] Prefer the definition of a TagDecl (e.g. class) as CanonicalDeclaration.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/index/FileIndex.cpp:23 SymbolCollector::Options CollectorOpts; // Although we do not index symbols in main files (e.g. cpp file), informat

[PATCH] D43869: [clangd] Support include canonicalization in symbol leve.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/index/CanonicalIncludes.h:47 + /// Sets the canonical include for any symbol with \p QualifiedName. + /// Header mappings are ignored if \p Qua

[PATCH] D43648: [clangd] Debounce streams of updates.

This revision was automatically updated to reflect the committed changes. Closed by commit rL326546: [clangd] Debounce streams of updates. (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D43648 Files: clang-tools-

[PATCH] D44000: [clang] Fix use-after-free on code completion

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. This feels like a bug in the underlying clang libraries, but since none of the lifetimes are documented and everyone just does it this way... Comment at: clangd/CodeCo

[PATCH] D44003: [clangd] Fix unintentionally loose fuzzy matching, and the tests masking it.

sammccall created this revision. Herald added subscribers: cfe-commits, ioeric, jkorous-apple, ilya-biryukov, klimek. sammccall updated this revision to Diff 136719. sammccall added a comment. test tweaks The intent was that [ar] doesn't match "FooBar"; the first character must match a Head cha

[PATCH] D44003: [clangd] Fix unintentionally loose fuzzy matching, and the tests masking it.

sammccall updated this revision to Diff 136719. sammccall added a comment. test tweaks Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44003 Files: clangd/FuzzyMatch.cpp clangd/FuzzyMatch.h unittests/clangd/FuzzyMatchTests.cpp unittests/clangd/IndexTests.cpp Index: unit

[PATCH] D44088: [clangd] Extract ClangdServer::Options struct.

sammccall created this revision. sammccall added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, ioeric, jkorous-apple, klimek. This subsumes most of the params to ClangdServer and ClangdLSPServer. Adjacent changes: - tests use a consistent set of options, except when testing sp

[PATCH] D44088: [clangd] Extract ClangdServer::Options struct.

sammccall updated this revision to Diff 136991. sammccall marked 3 inline comments as done. sammccall added a comment. address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44088 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServ

[PATCH] D44088: [clangd] Extract ClangdServer::Options struct.

This revision was automatically updated to reflect the committed changes. Closed by commit rL326719: [clangd] Extract ClangdServer::Options struct. (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44088 Files: cla

[PATCH] D44003: [clangd] Fix unintentionally loose fuzzy matching, and the tests masking it.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE326721: [clangd] Fix unintentionally loose fuzzy matching, and the tests masking it. (authored by sammccall, committed by ). Changed prior to commit: https://reviews.llvm.org/D44003?vs=136719&id=13702

[PATCH] D44141: [clang-format] Use NestedBlockIndent as a 0 column in formatted raw strings

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. LGTM, probably want to wait for djasper's opinion. Repository: rC Clang https://reviews.llvm.org/D44141 ___ cfe-commits mailing list cfe

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

sammccall added a comment. Behavior looks good. I think we can do a bit better with (most) fixits - your call on whether it makes sense to do here. As discussed i'd simplify the diagnostic containers until we know there's a strong need. Comment at: clangd/ClangdLSPServer.cpp

[PATCH] D44158: [clangd:vscode] Resolve symlinks for file paths from clangd.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. LG. Can you expand the comment and fix the formatting changes? Comment at: clangd/clients/clangd-vscode/src/extension.ts:3 import * as vscodelc from 'vscode-languagecl

[PATCH] D44247: [clangd] Use identifier range as the definition range.

sammccall added inline comments. Comment at: clangd/SourceCode.cpp:55 + const auto& SM = D->getASTContext().getSourceManager(); + SourceLocation SpellingLoc = SM.getSpellingLoc(D->getLocation()); + if (D->getLocation().isMacroID()) { As discussed offline, this

<    1   2   3   4   5   6   7   8   9   10   >