[PATCH] D42113: [clangd] Deduplicate symbols collected in global-symbol-builder tool.

This revision was automatically updated to reflect the committed changes. Closed by commit rL322722: [clangd] Deduplicate symbols collected in global-symbol-builder tool. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D42111: [Tooling] Don't deduplicate tool results in the All-TUs executor.

ioeric created this revision. ioeric added a reviewer: bkramer. Herald added subscribers: cfe-commits, klimek. As result deduplication or reduction is not supported in the framework, we should leave the deplication to tools (if needed) until the framework supports it. Repository: rC Clang

[PATCH] D42113: [clangd] Deduplicate symbols collected in global-symbol-builder tool.

ioeric created this revision. ioeric added reviewers: bkramer, sammccall. Herald added subscribers: cfe-commits, ilya-biryukov, klimek. After https://reviews.llvm.org/D42111, the executor framework no longer deduplicate tool results. Repository: rCTE Clang Tools Extra

[PATCH] D39050: Add index-while-building support to Clang

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Nice! Thanks for making the refactoring and adding tests! I think this is good to go now. I'm not very familiar with code outside of the index library (Driver, Basic etc), but the changes

[PATCH] D42181: [clangd] Merge index-provided completions with those from Sema.

ioeric added a comment. LGTM Comment at: clangd/CodeComplete.cpp:743 + int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging. + bool Incomplete = false; + llvm::Optional Filter; // Initialized once Sema runs. `InComplete` can probably be output

[PATCH] D41946: [clangd] Add support for different file URI schemas.

ioeric added inline comments. Comment at: clangd/URI.cpp:57 + Body = "/"; +Body += path::convert_to_slash(AbsolutePath, path::Style::posix); +return (llvm::Twine(Scheme) + ":" + percentEncode(Body)).str(); sammccall wrote: > ioeric wrote: > >

[PATCH] D41946: [clangd] Add support for different file URI schemas.

ioeric updated this revision to Diff 130766. ioeric marked 16 inline comments as done. ioeric added a comment. - Addressed review comments. - Check windows vs unit paths in tests. - Properly check absolute paths. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41946 Files:

[PATCH] D41946: [clangd] Add support for different file URI schemas.

ioeric updated this revision to Diff 130621. ioeric marked 14 inline comments as done. ioeric added a comment. - Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41946 Files: clangd/CMakeLists.txt clangd/URI.cpp clangd/URI.h

[PATCH] D41946: [clangd] Add support for different file URI schemas.

ioeric added inline comments. Comment at: clangd/URI.cpp:43 + Body.consume_front("/"); +return llvm::sys::path::convert_to_slash(Body); + } sammccall wrote: > Don't you want the opposite here, convert from unix to native? Ah, right!

[PATCH] D42361: [Tooling] Returns non-zero status code when files are skipped.

ioeric created this revision. ioeric added a reviewer: hokein. Herald added subscribers: cfe-commits, klimek. Repository: rC Clang https://reviews.llvm.org/D42361 Files: include/clang/Tooling/Tooling.h lib/Tooling/Tooling.cpp Index: lib/Tooling/Tooling.cpp

[PATCH] D41946: [clangd] Add support for different file URI schemas.

This revision was automatically updated to reflect the committed changes. Closed by commit rL323101: [clangd] Add support for different file URI schemas. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D41946 Files:

[PATCH] D42361: [Tooling] Returns non-zero status code when files are skipped.

ioeric added inline comments. Comment at: include/clang/Tooling/Tooling.h:331 + /// \returns 0 on success; 1 if any error occured; 2 if there is no error but + /// some files are skipped due to missing compile commands. int run(ToolAction *Action); hokein

[PATCH] D41946: [clangd] Add support for different file URI schemas.

ioeric updated this revision to Diff 130862. ioeric marked 4 inline comments as done. ioeric added a comment. - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41946 Files: clangd/CMakeLists.txt clangd/URI.cpp clangd/URI.h

[PATCH] D41946: [clangd] Add support for different file URI schemas.

ioeric added inline comments. Comment at: clangd/URI.h:31 +public: + /// \brief Returns decoded scheme e.g. "https" + llvm::StringRef scheme() const { return Scheme; } sammccall wrote: > nit: prefer to omit `\brief` unless you want the brief comment to be >

[PATCH] D42419: [clangd] Use new URI with scheme support in place of the existing LSP URI

ioeric updated this revision to Diff 131576. ioeric marked 8 inline comments as done. ioeric added a comment. - Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42419 Files: clangd/ClangdLSPServer.cpp clangd/Protocol.cpp clangd/Protocol.h

[PATCH] D42419: [clangd] Use new URI with scheme support in place of the existing LSP URI

ioeric added a comment. Thanks for the comments! Comment at: clangd/ClangdLSPServer.cpp:284 + std::string ResultUri = ""; + if (Result) { +auto U = URI::create(*Result); sammccall wrote: > But basically I think this shows that the API is awkward. We

[PATCH] D42577: [Lexer] Support adding working directory to relative search dir for #include shortening in HeaderSearch.

ioeric created this revision. ioeric added a reviewer: bkramer. Herald added subscribers: cfe-commits, hintonda, mgorny. Repository: rC Clang https://reviews.llvm.org/D42577 Files: include/clang/Lex/HeaderSearch.h lib/Lex/HeaderSearch.cpp unittests/Lex/CMakeLists.txt

[PATCH] D42113: [clangd] Deduplicate symbols collected in global-symbol-builder tool.

ioeric updated this revision to Diff 130207. ioeric marked 3 inline comments as done. ioeric added a comment. Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42113 Files: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp Index:

[PATCH] D42480: [clangd] Provide a helper to report estimated memory usage per-file

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lg Comment at: clangd/ClangdUnit.cpp:664 + std::lock_guard Lock(Mutex); + return ASTMemUsage + PreambleMemUsage; +} ilya-biryukov wrote: > ioeric wrote: >

[PATCH] D42480: [clangd] Provide a helper to report estimated memory usage per-file

ioeric added inline comments. Comment at: clangd/ClangdServer.h:324 + /// Returns estimated memory usage for each of the currently files. + /// The order of results is unspecified. s/currently/current/ ? or current open? Comment at:

[PATCH] D42361: [Tooling] Returns non-zero status code when files are skipped.

ioeric added inline comments. Comment at: lib/Tooling/Tooling.cpp:404 if (CompileCommandsForFile.empty()) { // FIXME: There are two use cases here: doing a fuzzy // "find . -name '*.cc' |xargs tool" match, where as a user I don't care bkramer

[PATCH] D42669: [clangd] Enable completion index by default, limit results to 100.

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

[PATCH] D42361: [Tooling] Returns non-zero status code when files are skipped.

ioeric added a comment. Ping? Repository: rC Clang https://reviews.llvm.org/D42361 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42735: [clangd] Add a test URI scheme for lit tests to unbreak platform-specific URI failures.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek. This should also fix the current windows buildbot breakage

[PATCH] D42361: [Tooling] Returns non-zero status code when files are skipped.

ioeric added inline comments. Comment at: lib/Tooling/Tooling.cpp:404 if (CompileCommandsForFile.empty()) { // FIXME: There are two use cases here: doing a fuzzy // "find . -name '*.cc' |xargs tool" match, where as a user I don't care bkramer

[PATCH] D42857: testing phabricator email reply

ioeric created this revision. Herald added subscribers: cfe-commits, klimek. ioeric removed rCTE Clang Tools Extra as the repository for this revision. ioeric removed subscribers: klimek, cfe-commits. ioeric added a comment. top Comment Comment at:

[PATCH] D42361: [Tooling] Returns non-zero status code when files are skipped.

This revision was automatically updated to reflect the committed changes. Closed by commit rC324113: [Tooling] Returns non-zero status code when files are skipped. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D42361?vs=132624=132626#toc Repository:

[PATCH] D42361: [Tooling] Returns non-zero status code when files are skipped.

ioeric updated this revision to Diff 132624. ioeric added a comment. removed FIXME Repository: rC Clang https://reviews.llvm.org/D42361 Files: include/clang/Tooling/Tooling.h lib/Tooling/Tooling.cpp Index: lib/Tooling/Tooling.cpp

[PATCH] D42361: [Tooling] Returns non-zero status code when files are skipped.

This revision was automatically updated to reflect the committed changes. Closed by commit rL324113: [Tooling] Returns non-zero status code when files are skipped. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D42361

[PATCH] D42796: [clangd] Skip inline namespace when collecting scopes for index symbols.

ioeric updated this revision to Diff 132549. ioeric marked 2 inline comments as done. ioeric edited the summary of this revision. ioeric added a comment. Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42796 Files: clangd/index/SymbolCollector.cpp

[PATCH] D42577: [Lexer] Support adding working directory to relative search dir for #include shortening in HeaderSearch.

This revision was automatically updated to reflect the committed changes. Closed by commit rL323647: [Lexer] Support adding working directory to relative search dir for #include… (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D42638: [clangd] Add a fallback directory for collected symbols with relative paths.

ioeric created this revision. ioeric added reviewers: hokein, sammccall. Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42638 Files: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp

[PATCH] D42419: [clangd] Use new URI with scheme support in place of the existing LSP URI

ioeric updated this revision to Diff 131775. ioeric added a comment. - Merge remote-tracking branch 'origin/master' into uri Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42419 Files: clangd/ClangdLSPServer.cpp clangd/Protocol.cpp clangd/Protocol.h clangd/URI.cpp

[PATCH] D42577: [Lexer] Support adding working directory to relative search dir for #include shortening in HeaderSearch.

ioeric updated this revision to Diff 131777. ioeric marked 2 inline comments as done. ioeric added a comment. - Addressed review comments. Repository: rC Clang https://reviews.llvm.org/D42577 Files: include/clang/Lex/HeaderSearch.h lib/Lex/HeaderSearch.cpp unittests/Lex/CMakeLists.txt

[PATCH] D42575: [clangd] Better handling symbols defined in macros.

ioeric added inline comments. Comment at: clangd/index/SymbolCollector.cpp:108 +std::string PrintableLoc(SourceLocation Loc, SourceManager ) { + if (Loc.isInvalid()) `PrintLoc`? `PrintableLoc` sounds like a checker for whether a location is printable.

[PATCH] D42577: [Lexer] Support adding working directory to relative search dir for #include shortening in HeaderSearch.

This revision was automatically updated to reflect the committed changes. Closed by commit rC323647: [Lexer] Support adding working directory to relative search dir for #include… (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D42577?vs=131777=131781#toc

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

ioeric created this revision. Herald added subscribers: cfe-commits, hintonda, jkorous-apple, ilya-biryukov, mgorny, klimek. o Collect suitable #include paths for index symbols. This also does smart mapping for STL symbols and IWYU pragma. o For global code completion, add a command for

[PATCH] D42419: [clangd] Use new URI with scheme support in place of the existing LSP URI

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE323660: [clangd] Use new URI with scheme support in place of the existing LSP URI (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D42419?vs=131802=131803#toc

[PATCH] D42638: [clangd] Add a fallback directory for collected symbols with relative paths.

ioeric added inline comments. Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:41 +llvm::cl::desc( +"For symbols' file paths that cannot be resolved to absolute " +"paths (i.e. in-memory VFS without absolute paths), "

[PATCH] D42638: [clangd] Add a fallback directory for collected symbols with relative paths.

ioeric updated this revision to Diff 131799. ioeric marked 3 inline comments as done. ioeric added a comment. - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42638 Files: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp

[PATCH] D42796: [clangd] Skip inline namespace when collecting scopes for index symbols.

ioeric created this revision. ioeric added reviewers: sammccall, hokein. Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42796 Files: clangd/index/SymbolCollector.cpp

[PATCH] D42735: [clangd] Add a test URI scheme for lit tests to unbreak platform-specific URI failures.

ioeric updated this revision to Diff 132180. ioeric marked 4 inline comments as done. ioeric added a comment. - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42735 Files: clangd/ClangdLSPServer.cpp clangd/Protocol.cpp

[PATCH] D42735: [clangd] Add a test URI scheme for lit tests to unbreak platform-specific URI failures.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE323885: [clangd] Add a test URI scheme for lit tests to unbreak platform-specific URI… (authored by ioeric, committed by ). Changed prior to commit:

[PATCH] D42735: [clangd] Add a test URI scheme for lit tests to unbreak platform-specific URI failures.

This revision was automatically updated to reflect the committed changes. Closed by commit rL323885: [clangd] Add a test URI scheme for lit tests to unbreak platform-specific URI… (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D42796: [clangd] Skip inline namespace when collecting scopes for index symbols.

ioeric added inline comments. Comment at: clangd/index/SymbolCollector.cpp:69 +// qualifier. Inline namespaces and unscoped enums are skipped. +llvm::Expected getScope(const NamedDecl *ND) { + llvm::SmallVector Contexts; hokein wrote: >

[PATCH] D42796: [clangd] Skip inline namespace when collecting scopes for index symbols.

ioeric updated this revision to Diff 132552. ioeric added a comment. - clang-format Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42796 Files: clangd/index/SymbolCollector.cpp unittests/clangd/SymbolCollectorTests.cpp Index: unittests/clangd/SymbolCollectorTests.cpp

[PATCH] D42796: [clangd] Skip inline namespace when collecting scopes for index symbols.

This revision was automatically updated to reflect the committed changes. Closed by commit rL324065: [clangd] Skip inline namespace when collecting scopes for index symbols. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D42639: [clang-move] Clever on handling header file which includes itself.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lg Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D42419: [clangd] Use new URI with scheme support in place of the existing LSP URI

ioeric updated this revision to Diff 131802. ioeric marked an inline comment as done. ioeric added a comment. - addressed review comments; removed URI hack in vscode extenstion. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42419 Files: clangd/ClangdLSPServer.cpp

[PATCH] D42638: [clangd] Add a fallback directory for collected symbols with relative paths.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE323658: [clangd] Add a fallback directory for collected symbols with relative paths. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D42638?vs=131799=131800#toc

[PATCH] D42638: [clangd] Add a fallback directory for collected symbols with relative paths.

This revision was automatically updated to reflect the committed changes. Closed by commit rL323658: [clangd] Add a fallback directory for collected symbols with relative paths. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D42639: [clang-move] Clever on handling header file which includes itself.

ioeric added inline comments. Comment at: clang-move/ClangMove.cpp:708 +// Find old.h includes "old.h". +if (AbsoluteOldHeader == AbsoluteOldHeader) { + OldHeaderIncludeRangeInHeader = IncludeFilenameRange; This check is always true?

[PATCH] D42915: [clangd] Use URIs in index symbols.

ioeric created this revision. ioeric added reviewers: hokein, sammccall. Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42915 Files: clangd/index/Index.cpp clangd/index/Index.h

[PATCH] D43068: [clangd] Remove codeComplete that returns std::future<>

ioeric added inline comments. Comment at: unittests/clangd/SyncAPI.h:8 +// +//===-===// +#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H ilya-biryukov wrote: > ioeric wrote: > >

[PATCH] D43068: [clangd] Remove codeComplete that returns std::future<>

ioeric added inline comments. Comment at: unittests/clangd/SyncAPI.h:8 +// +//===-===// +#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H ilya-biryukov wrote: > ioeric wrote: > > Any

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

ioeric added a comment. Thanks for the comments! I addressed comments in the symbol collect part (I think?). Will add tests in a followup patch. Comment at: clangd/index/HeaderMapCollector.h:48 + // A map from header patterns to header names. + // The header names are not

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

ioeric added a comment. Thanks for the comments! I have addressed comments in the symbol collect side (I think?). Will add tests in a followup patch. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42640 ___ cfe-commits mailing

[PATCH] D42915: [clangd] Use URIs in index symbols.

This revision was automatically updated to reflect the committed changes. Closed by commit rL324358: [clangd] Use URIs in index symbols. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D42915 Files:

[PATCH] D42942: [clangd] Collect definitions when indexing.

ioeric added a comment. looks good Comment at: clangd/index/Index.h:25 struct SymbolLocation { // The absolute path of the source file where a symbol occurs. It might be worth mentioning here whether the range covers the entire declaration/definition

[PATCH] D42915: [clangd] Use URIs in index symbols.

ioeric updated this revision to Diff 132958. ioeric marked 2 inline comments as done. ioeric added a comment. - Make URIScheme customizable in SymbolCollector. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42915 Files: clangd/index/Index.cpp clangd/index/Index.h

[PATCH] D42915: [clangd] Use URIs in index symbols.

ioeric added inline comments. Comment at: clangd/index/Index.h:27 + // The URI of the source file where a symbol occurs. + llvm::StringRef FileUri; // The 0-based offset to the first character of the symbol from the beginning sammccall wrote: > nit:

[PATCH] D42913: [clangd] Fix incorrect file path for symbols defined by the compile command-line option.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lg Comment at: clangd/index/SymbolCollector.cpp:132 +// * symbols controlled and defined by a compile command-line option +// `-DName=foo`, the spelling

[PATCH] D42919: [clangd] Support simpler JSON-RPC stream parsing for lit tests.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. LGTM Have we kept a lit test that uses content-length? It's unclear from the patch. Comment at: clangd/tool/ClangdMain.cpp:89 +static llvm::cl::opt Test( +"test", +

[PATCH] D42915: [clangd] Use URIs in index symbols.

ioeric added a comment. I was thinking about leaving URI scheme customization to the postprocessing phase, but you are right, it would be better to make the URI scheme extendable in SymbolCollector. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42915

[PATCH] D42915: [clangd] Use URIs in index symbols.

ioeric added inline comments. Comment at: clangd/index/Index.h:27 + // The URI of the source file where a symbol occurs. + llvm::StringRef FileUri; // The 0-based offset to the first character of the symbol from the beginning hokein wrote: > sammccall

[PATCH] D42942: [clangd] Collect definitions when indexing.

ioeric added inline comments. Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:67 + // XXX this is just to make running the tool fast during dev! + bool BeginInvocation(CompilerInstance ) override { +const auto =

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

ioeric updated this revision to Diff 133021. ioeric marked 10 inline comments as done. ioeric added a comment. - Merge with origin/master - Renamed and moved a bunch files according to review comments. - Merge remote-tracking branch 'origin/master' into include - Half way - Merge with uri

[PATCH] D42915: [clangd] Use URIs in index symbols.

ioeric updated this revision to Diff 133000. ioeric added a comment. - s/Uri/URI/ - Address review comments. Support multiple schemes. - Merged with origin/master - Merge branch 'master' of http://llvm.org/git/clang-tools-extra into uri Repository: rCTE Clang Tools Extra

[PATCH] D42915: [clangd] Use URIs in index symbols.

ioeric updated this revision to Diff 133009. ioeric added a comment. - removed leftover logs. - Merge branch 'master' of http://llvm.org/git/clang-tools-extra into uri Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42915 Files: clangd/index/Index.cpp clangd/index/Index.h

[PATCH] D43174: [clang-move] Fix the incorrect expansion end location.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43174 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43174: [clang-move] Fix the incorrect expansion end location.

ioeric added a comment. The fix is quite subtle and not obvious to tell from the test. Could you elaborate on the issue this fixes in the patch description? Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43174 ___

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

ioeric updated this revision to Diff 133635. ioeric marked 3 inline comments as done. ioeric added a comment. - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42640 Files: clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

ioeric added inline comments. Comment at: clangd/ClangdServer.cpp:368 +/// Calculates the shortest possible include path when inserting \p Header to \p +/// File, by matching \p Header against all include search directories for \p sammccall wrote: > ioeric

[PATCH] D39050: Add index-while-building support to Clang

ioeric added a comment. I'm wondering if there is any further plan for this? ;) https://reviews.llvm.org/D39050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

ioeric updated this revision to Diff 133895. ioeric marked 15 inline comments as done. ioeric added a comment. - Addressed some review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42640 Files: clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

ioeric added inline comments. Comment at: clangd/ClangdServer.cpp:370 +/// File, by matching \p Header against all include search directories for \p +/// File via `clang::HeaderSearch`. +/// sammccall wrote: > ioeric wrote: > > sammccall wrote: > > > sammccall

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

ioeric added a comment. I think it's useful to explicit initialize non-trivial types like `FileChangeType`. But I think it's safe and probably easier to rely on default values of primitive types like int, bool etc. Explicitly setting default values is probably fine (so this change LGTM), but

[PATCH] D43246: [clangd] Assert path is absolute when assigning to URIForFile.

ioeric added a comment. I think another option to prevent the bug in r325029 from happening would be providing a canonical approach for retrieving (absolute) paths from `FileManager`. We already have code in symbol collector that does this, and we have similar code in XRefs.cpp now. We should

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

ioeric added a comment. In https://reviews.llvm.org/D43230#1006469, @ilya-biryukov wrote: > In https://reviews.llvm.org/D43230#1006104, @ioeric wrote: > > > But I think it's safe and probably easier to rely on default values of > > primitive types like int, bool etc > > > It's not always safe,

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

ioeric added inline comments. Comment at: clangd/ClangdLSPServer.cpp:194 +if (!Replacements) + return replyError(ErrorCode::InternalError, +llvm::toString(Replacements.takeError())); nit: since we are not spelling out

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

ioeric added inline comments. Comment at: clangd/ClangdLSPServer.cpp:194 +if (!Replacements) + return replyError(ErrorCode::InternalError, +llvm::toString(Replacements.takeError())); ilya-biryukov wrote: > ioeric

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

ioeric accepted this revision. ioeric added a comment. lg Comment at: clangd/ClangdServer.cpp:209 + auto Action = [Contents, Pos, TaggedFS, + PCHs](Path File, decltype(Callback) Callback, + llvm::Expected IP) {

[PATCH] D43246: [clangd] Assert path is absolute when assigning to URIForFile.

ioeric added inline comments. Comment at: clangd/Protocol.h:52 struct URIForFile { + URIForFile() = default; ilya-biryukov wrote: > sammccall wrote: > > I don't like how the API changes here take us further away from the other > > structs in this file. > >

[PATCH] D43075: [clang-move] Don't dump macro symbols.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Ooops, forgot to stamp! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43075 ___ cfe-commits mailing list

[PATCH] D42942: [clangd] Collect definitions when indexing.

ioeric accepted this revision. ioeric added a comment. Lg Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:67 + // XXX this is just to make running the tool fast during dev! + bool BeginInvocation(CompilerInstance ) override { +const auto

[PATCH] D43075: [clang-move] Don't dump macro symbols.

ioeric added a comment. Lg. Thanks for the change! Comment at: clang-move/ClangMove.cpp:526 unless(usingDirectiveDecl()), // using namespace decl. + notInMacro(), InOldHeader, I'd probably relax the condition a bit; theoretically tools would

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

ioeric updated this revision to Diff 133595. ioeric marked 5 inline comments as done. ioeric added a comment. - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42640 Files: clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

ioeric added a comment. Thanks! PTAL Comment at: clangd/ClangdServer.cpp:465 +auto = Clang->getPreprocessor().getHeaderSearchInfo(); +std::string Suggested = HeaderSearchInfo.suggestPathToFileForDiagnostics( +*Resolved, CompileCommand.Directory);

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

ioeric updated this revision to Diff 133599. ioeric added a comment. - fix a leftover bug Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42640 Files: clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

ioeric updated this revision to Diff 133411. ioeric marked 13 inline comments as done. ioeric added a comment. - Added tests for all components; more cleanup; s/IncludeURI/IncludeHeader/ Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42640 Files: clangd/CMakeLists.txt

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

ioeric added a comment. Thanks for the comments! Sorry that I didn't clean up the code before sending out the prototype. I planned to deal with code structure and style issues after getting some early feedback, but I think the patch is ready for review now. Comment at:

[PATCH] D43065: [clangd] Remove threading-related code from ClangdUnit.h

ioeric added a comment. Nice! The code looks much simpler! Just some drive-by nits. I don't know the threading work well enough to give useful comments. Will leave the approval to others. Comment at: clangd/ClangdUnit.cpp:399 + std::unique_ptr CI; + { +//

[PATCH] D43068: [clangd] Remove codeComplete that returns std::future<>

ioeric added inline comments. Comment at: unittests/clangd/SyncAPI.h:8 +// +//===-===// +#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H Any reason not to put sync APIs in ClangdServer

[PATCH] D43065: [clangd] Remove threading-related code from ClangdUnit.h

ioeric added inline comments. Comment at: clangd/ClangdUnit.cpp:399 + std::unique_ptr CI; + { +// FIXME(ibiryukov): store diagnostics from CommandLine when we start ilya-biryukov wrote: > ioeric wrote: > > Do we still need this block? > I added it to avoid

[PATCH] D43009: [clangd] Do not precent-encode numbers in URI.

ioeric created this revision. ioeric added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, jkorous-apple, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43009 Files: clangd/URI.cpp unittests/clangd/URITests.cpp Index: unittests/clangd/URITests.cpp

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

ioeric updated this revision to Diff 133203. ioeric added a comment. - Merge with origin/master Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42640 Files: clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h

[PATCH] D43009: [clangd] Do not precent-encode numbers in URI.

This revision was automatically updated to reflect the committed changes. Closed by commit rL324475: [clangd] Do not precent-encode numbers in URI. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D43009 Files:

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

ioeric updated this revision to Diff 133202. ioeric added a comment. - Added tests for IncludeURI and CanonicalIncludes and minor cleanup. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42640 Files: clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp

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

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Thanks a lot for the fix! Suggest a test case which reproduced the bug: // comment with 'quote' void f() {} `runSymbolCollector` -> `SymbolToYAML` -> `SymbolFromYAML` -> check

[PATCH] D43229: [clangd] Enable snippet completion based on client capabilities.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Comment at: clangd/ClangdLSPServer.cpp:90 void ClangdLSPServer::onInitialize(InitializeParams ) { + if (Params.rootUri && !Params.rootUri->file.empty()) +

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