[PATCH] D46496: [Tooling] Pull #include manipulation code from clangFormat into libToolingCore.

ioeric updated this revision to Diff 145384. ioeric added a comment. - Revert unintended change. Repository: rC Clang https://reviews.llvm.org/D46496 Files: include/clang/Format/Format.h include/clang/Tooling/Core/HeaderIncludes.h lib/Format/Format.cpp lib/Tooling/Core/CMakeLists.txt

[PATCH] D46496: [Tooling] Pull #include manipulation code from clangFormat into libToolingCore.

ioeric created this revision. ioeric added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, mgorny, klimek. Also pull #include related style out of FormatStyle as tooling::IncludeStyle. Repository: rC Clang https://reviews.llvm.org/D46496 Files: include/clang/Basic/SourceM

[PATCH] D46176: Add SourceManagerForFile helper which sets up SourceManager and dependencies for a single file with code snippet

ioeric added a comment. Thanks for the comment and suggestion! I've changed this to `SourceManagerForFile` and move it to SourceManager.h, which seems to be a more natural fit. `SourceManagerForFile` sounds like a sensible name to me, but I'm guess there might be a better name... Repository:

[PATCH] D46176: Pull helper class Environment from clangFormat into libToolingCore [NFC]

ioeric updated this revision to Diff 145318. ioeric added a comment. - Create SourceManagerForFile instead of Environment; put it in SourceManager.h which seems to be a better place. Repository: rC Clang https://reviews.llvm.org/D46176 Files: include/clang/Basic/SourceManager.h lib/Basi

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

This revision was automatically updated to reflect the committed changes. Closed by commit rC331544: [clang-format] Refactor #include insertion/deletion functionality into a class. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D46180?vs=145210&id=145225#

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

ioeric updated this revision to Diff 145210. ioeric marked an inline comment as done. ioeric added a comment. - Addressed review comment. Repository: rC Clang https://reviews.llvm.org/D46180 Files: lib/Format/Format.cpp unittests/Format/CleanupTest.cpp Index: unittests/Format/CleanupTes

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

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

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

ioeric added inline comments. Comment at: lib/Format/Format.cpp:1691 // 0. Otherwise, returns the priority of the matching category or INT_MAX. - int getIncludePriority(StringRef IncludeName, bool CheckMainHeader) { + int getIncludePriority(StringRef IncludeName, bool CheckM

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

ioeric updated this revision to Diff 144895. ioeric marked 3 inline comments as done. ioeric added a comment. - addressed more review comments. Repository: rC Clang https://reviews.llvm.org/D46180 Files: lib/Format/Format.cpp unittests/Format/CleanupTest.cpp Index: unittests/Format/Clea

[PATCH] D46176: Pull helper class Environment from clangFormat into libToolingCore [NFC]

ioeric updated this revision to Diff 144524. ioeric added a comment. - Revert last revision. Repository: rC Clang https://reviews.llvm.org/D46176 Files: include/clang/Tooling/Core/Environment.h lib/Format/Format.cpp lib/Format/SortJavaScriptImports.cpp lib/Format/TokenAnalyzer.cpp

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

ioeric marked 7 inline comments as done. ioeric added a comment. Thanks for reviewing this! In https://reviews.llvm.org/D46180#1080822, @ilya-biryukov wrote: > This change LG as an extraction of the helper functionality to be reused in > clang, clang-tidy, etc. > However, I feel there are pote

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

ioeric updated this revision to Diff 144319. ioeric marked 2 inline comments as done. ioeric added a comment. Addressed review comments. Repository: rC Clang https://reviews.llvm.org/D46180 Files: lib/Format/Format.cpp unittests/Format/CleanupTest.cpp Index: unittests/Format/CleanupTest

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

ioeric added a comment. There isn't actually as much code changed as it appears to be, but phabricator doens't understand the diff very well... `git diff` might be an easier way to review the patch. Repository: rC Clang https://reviews.llvm.org/D46180 ___

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

ioeric created this revision. ioeric added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, klimek. The class will be moved into libToolingCore as followup. The new behaviors in this patch: - New #include is inserted in the right position in a #include block to preserver sorted

[PATCH] D46176: Pull helper class Environment from clangFormat into libToolingCore [NFC]

ioeric created this revision. ioeric added a reviewer: djasper. Herald added subscribers: cfe-commits, mgorny, klimek. This can be used to create a virtual environment (incl. VFS, source manager) for code snippets. The existing clang::format::Environment is implemented based on the new clang::tool

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

ioeric added a comment. Looks good. We still need tests though :) Comment at: lib/AST/RawCommentList.cpp:376 +SourceMgr.getSpellingColumnNumber(Tok.getLocation(), &LocInvalid); +if (LocInvalid) + TokColumn = 0; This is a bit confusing... Could

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

ioeric added inline comments. Comment at: include/clang/AST/RawCommentList.h:118 + /// // Parts of it might be indented. + /// /* The comments styles might be mixed. */ + /// into I'm trying to understand how these cases and RawComment work. For t

[PATCH] D46065: [clangd] Add "str()" method to SymbolID.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm with a nit Comment at: clangd/index/Index.h:72 + // Returns a 40-bytes hex encoded string. + std::string str() const; I think Sam wants to reduce th

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

ioeric added a comment. Overall looks good. Could you add tests for the new methods? Comment at: lib/AST/CommentLexer.cpp:294 void Lexer::lexCommentText(Token &T) { + if (ParseCommands) +lexCommentTextWithCommands(T); micro-nit: I'd probably ``` return Pa

[PATCH] D46001: [CodeComplete] Expose helpers to get RawComment of completion result.

ioeric added a comment. This seems to do what we want for clangd, but we should also get the code owner or someone who knows the code better to take a look. Repository: rC Clang https://reviews.llvm.org/D46001 ___ cfe-commits mailing list cfe-co

[PATCH] D45692: [clangd] Fix "fail to create file URI" warnings in FileIndexTest.

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/D45692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h:75 +/// \brief Deduplicate, check for conflicts, and convert all Repla

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

ioeric added a comment. This looks great! Just a few nits left. Comment at: clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h:75 +/// \brief Deduplicate, check for conflicts, and convert all Replacements stored +/// in \c TUs to AtomicChang

[PATCH] D44248: [clangd][cmake] Provide libatomic when there is no native support for 64bit atomics

ioeric added a comment. In https://reviews.llvm.org/D44248#1041531, @sdardis wrote: > Thanks, I wasn't sure who to add as a reviewer. Authors/reviewers of recent patches for the same files are usually good approximations :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D4424

[PATCH] D44248: [clangd][cmake] Provide libatomic when there is no native support for 64bit atomics

ioeric added a comment. lgtm (Please add reviewers to your patch if you intend it to be reviewed.) Comment at: clangd/CMakeLists.txt:7 +if(NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB) + list(APPEND CLANGD_ATOMIC_LIB atomic) +endif() nit: s/atomic/"atomic"/ Repositor

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

ioeric added inline comments. Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:353 +const FileEntry *Entry = FileAndReplacements.first; +ReplacementsToAtomicChanges DeduplicatedChanges(SM, Entry); +for (const auto &R : FileAndReplacements.second)

[PATCH] D44517: [change-namespace] Don't match a function call/ref multiple times.

This revision was automatically updated to reflect the committed changes. Closed by commit rL327629: [change-namespace] Don't match a function call/ref multiple times. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44

[PATCH] D44517: [change-namespace] Don't match a function call/ref multiple times.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE327629: [change-namespace] Don't match a function call/ref multiple times. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D44517?vs=138550&id=138556#toc Reposi

[PATCH] D44517: [change-namespace] Don't match a function call/ref multiple times.

ioeric updated this revision to Diff 138550. ioeric added a comment. - small fix. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44517 Files: change-namespace/ChangeNamespace.cpp unittests/change-namespace/ChangeNamespaceTests.cpp Index: unittests/change-namespace/ChangeN

[PATCH] D44517: [change-namespace] Don't match a function call/ref multiple times.

ioeric created this revision. ioeric added a reviewer: hokein. Herald added subscribers: cfe-commits, klimek. Previously, the matcher matches a function call/ref multiple times, one for each decl ancestor. This might cause problems. For example, in the following case, `func()` would be matched onc

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

ioeric added inline comments. Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:353 +const FileEntry *Entry = FileAndReplacements.first; +ReplacementsToAtomicChanges DeduplicatedChanges(SM, Entry); +for (const auto &R : FileAndReplacements.second)

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

ioeric added inline comments. Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:353 +const FileEntry *Entry = FileAndReplacements.first; +ReplacementsToAtomicChanges DeduplicatedChanges(SM, Entry); +for (const auto &R : FileAndReplacements.second)

[PATCH] D44305: [clangd] Add an interface that finds symbol by SymbolID in SymbolIndex.

This revision was automatically updated to reflect the committed changes. Closed by commit rL327487: [clangd] Add an interface that finds symbol by SymbolID in SymbolIndex. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.or

[PATCH] D44305: [clangd] Add an interface that finds symbol by SymbolID in SymbolIndex.

ioeric updated this revision to Diff 138313. ioeric marked 5 inline comments as done. ioeric added a comment. - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44305 Files: clangd/index/FileIndex.cpp clangd/index/FileIndex.h clangd/index/Index.h

[PATCH] D44305: [clangd] Add an interface that finds symbol by SymbolID in SymbolIndex.

ioeric added inline comments. Comment at: clangd/index/Merge.cpp:69 + else +B.insert(mergeSymbol(*Sym, S, &Scratch)); +}); sammccall wrote: > This could also just be callback(mergeSymbol(...)), if we keep track of the > IDs we've emitted. > This

[PATCH] D44305: [clangd] Add an interface that finds symbol by SymbolID in SymbolIndex.

ioeric added a comment. Thanks for the review! Comment at: clangd/index/Index.h:268 + virtual bool + getSymbol(const SymbolID &ID, +llvm::function_ref Callback) const = 0; sammccall wrote: > sammccall wrote: > > sammccall wrote: > > > Can we make

[PATCH] D44305: [clangd] Add an interface that finds symbol by SymbolID in SymbolIndex.

ioeric updated this revision to Diff 138204. ioeric marked 6 inline comments as done. ioeric added a comment. - - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44305 Files: clangd/index/FileIndex.cpp clangd/index/FileIndex.h clangd/index/Index.

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

ioeric added inline comments. Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:188 -bool mergeAndDeduplicate(const TUReplacements &TUs, - FileToReplacementsMap &GroupedReplacements, - clang::SourceManager &SM

[PATCH] D44305: [clangd] Add an interface that finds symbol by SymbolID in SymbolIndex.

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek. Potential use case: argument go-to-definition result with symbol information (e.g. function definition in cc file) that might not be in the AST. Reposito

[PATCH] D44298: [clangd] Don't index template specializations.

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Comment at: unittests/clangd/SymbolCollectorTests.cpp:200 + UnorderedElementsAreArray( + {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"), QName("Tm

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL327009: [clangd:vscode] Resolve symlinks for file paths from clangd. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44158 F

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

ioeric added inline comments. Comment at: clangd/clients/clangd-vscode/src/extension.ts:3 import * as vscodelc from 'vscode-languageclient'; +import { realpathSync } from 'fs'; sammccall wrote: > nit: the braces don't do anything here, right? I'm not sure... t

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

ioeric updated this revision to Diff 137564. ioeric marked an inline comment as done. ioeric added a comment. - add context about the workaround. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44158 Files: clangd/clients/clangd-vscode/src/extension.ts Index: clangd/clients/

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

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek. For features like go-to-definition, clangd can point clients to symlink paths (e.g. in bazel execroot) which might not be desired if the symlink points to

[PATCH] D44138: [clangd] Sort includes when formatting code or inserting new includes.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE326773: [clangd] Sort includes when formatting code or inserting new includes. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D44138?vs=137137&id=137142#toc Re

[PATCH] D44138: [clangd] Sort includes when formatting code or inserting new includes.

ioeric created this revision. ioeric added reviewers: hokein, ilya-biryukov. Herald added subscribers: cfe-commits, jkorous-apple, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44138 Files: clangd/ClangdServer.cpp unittests/clangd/ClangdTests.cpp Index: unittests/cl

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

ioeric added inline comments. Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:207 + llvm::DenseMap> + GroupedReplacements; + jdemeule wrote: > ioeric wrote: > > I don't think we need to do the deduplication here anymore. AtomicChange

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

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE326456: [clangd] Support include canonicalization in symbol leve. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D43869?vs=136561&id=136565#toc Repository: r

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

ioeric added inline comments. Comment at: clangd/index/CanonicalIncludes.h:54 /// a canonical header name. - llvm::StringRef mapHeader(llvm::StringRef Header) const; + /// An optional qualified symbol name can be provided to check against the + /// symbol mapping. -

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

ioeric updated this revision to Diff 136561. ioeric marked 2 inline comments as done. ioeric added a comment. - Merge with origin/master - Address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43869 Files: clangd/index/CanonicalIncludes.cpp clangd/index/Ca

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

ioeric added inline comments. Comment at: clangd/index/CanonicalIncludes.cpp:83 + static const std::vector> SymbolMap = { + // Map symbols in to their preferred includes. + {"std::basic_filebuf", ""}, hokein wrote: > Looks like the list only contains

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

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE326313: [clangd] Prefer the definition of a TagDecl (e.g. class) as… (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D43823?vs=136247&id=136248#toc Repository:

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

ioeric created this revision. ioeric added reviewers: sammccall, hokein. Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek. Symbols with different canonical includes might be defined in the same header (e.g. symbols defined in STL ). This patch adds support for mapping fr

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

ioeric added inline comments. Comment at: clangd/index/SymbolCollector.cpp:260 BasicSymbol = addDeclaration(*ND, std::move(ID)); +else if (isPreferredDeclaration(OriginalDecl, Roles)) + BasicSymbol = addDeclaration(OriginalDecl, std::move(ID)); sa

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL326313: [clangd] Prefer the definition of a TagDecl (e.g. class) as… (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D43823 F

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

ioeric updated this revision to Diff 136247. ioeric marked 5 inline comments as done. ioeric added a comment. - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43823 Files: clangd/index/FileIndex.cpp clangd/index/SymbolCollector.cpp clangd/index/

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

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek. Currently, we pick the first declaration of a symbol in a TU, which is considered canonical in the clangIndex, as the canonical declaration in clangd. Thi

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

ioeric added a comment. Thanks for the cleanup! This looks really nice! Comment at: clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h:45 +/// \brief Map mapping file name to AtomicChange targeting that file. +typedef llvm::DenseMap +File

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL326070: [clangd] don't insert new includes if either original header or canonical… (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm

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

ioeric updated this revision to Diff 135857. ioeric marked an inline comment as done. ioeric added a comment. - Merge with origin/master - Use llvm::StringSet Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43510 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp cl

[PATCH] D43671: [clangd] Address FIXME and fix comment

ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Oops, just realized I forgot to push the "send" button! Comment at: clangd/tool/ClangdMain.cpp:153 + if (RunSynchronously) { +if (WorkerThreadsCount.getNumOccurrences())

[PATCH] D43671: [clangd] Address FIXME and fix comment

ioeric added a comment. Thanks for the cleanup Kirill :) Comment at: clangd/tool/ClangdMain.cpp:153 + if (RunSynchronously) { +if (WorkerThreadsCount != 0) { + llvm::errs() `-j` is non-zero by default, and we shouldn't show warning if users only spec

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

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE325813: [clangd] Extend textDocument/didChange to specify whether diagnostics should be… (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D43634?vs=135463&id=1354

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

ioeric updated this revision to Diff 135463. ioeric marked 2 inline comments as done. ioeric added a comment. Addressed review comments. Removed test. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43634 Files: clangd/ClangdLSPServer.cpp clangd/Protocol.cpp clangd/Protoco

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

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek. This would allow us to disable diagnostics when didChange is called but diagnostics are not wanted (e.g. code completion). Repository: rCTE Clang Tools

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

ioeric added a comment. In https://reviews.llvm.org/D43500#1015208, @jdemeule wrote: > In https://reviews.llvm.org/D43500#1013558, @malcolm.parsons wrote: > > > In https://reviews.llvm.org/D43500#1013497, @aaron.ballman wrote: > > > > > Is there a way to make clang-apply-replacements smarter rath

[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.

This revision was automatically updated to reflect the committed changes. Closed by commit rL325764: [clangd] Not collect include headers for dynamic index for now. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D43550

[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.

ioeric added inline comments. Comment at: clangd/index/FileIndex.cpp:18 -const CanonicalIncludes *canonicalIncludesForSystemHeaders() { - static const auto *Includes = [] { ilya-biryukov wrote: > Should we also remove the mutex from `CanonicalIncludes` now? I

[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.

ioeric updated this revision to Diff 135244. ioeric marked 2 inline comments as done. ioeric added a comment. Properly use toURI. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43550 Files: clangd/CodeComplete.cpp clangd/index/FileIndex.cpp clangd/index/Index.h clangd/i

[PATCH] D43567: [ASTMatchers] isTemplateInstantiation: also match explicit instantiation declaration.

This revision was automatically updated to reflect the committed changes. Closed by commit rL325678: [ASTMatchers] isTemplateInstantiation: also match explicit instantiation… (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.

[PATCH] D43567: [ASTMatchers] isTemplateInstantiation: also match explicit instantiation declaration.

ioeric created this revision. ioeric added a reviewer: bkramer. Herald added subscribers: cfe-commits, klimek. Example: template class X {}; class A {}; // Explicit instantiation declaration. extern template class X; Repository: rC Clang https://reviews.llvm.org/D43567 Files: include/cla

[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.

ioeric added a comment. In https://reviews.llvm.org/D43550#1014319, @ilya-biryukov wrote: > Is there a way to still enable include insertion but in a restricted manner, > e.g. only for the current project? I'm not sure if this is currently supported, as we don't have a good definition of a "p

[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.

ioeric created this revision. ioeric added reviewers: ilya-biryukov, sammccall, hokein. Herald added subscribers: cfe-commits, jkorous-apple, klimek. The new behaviors introduced by this patch: o When include collection is enabled, we always set IncludeHeader field in Symbol even if it's the same

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

ioeric updated this revision to Diff 135074. ioeric marked 16 inline comments as done. ioeric added a comment. - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43510 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h

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

ioeric added inline comments. Comment at: clangd/ClangdServer.h:243 + /// string quoted with <> or "" that can be #included directly. + /// \p PreferredHeader The preferred header to be inserted. The has the same + /// semantic as \p OriginalHeader. If empty, \p OriginalHeader

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

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek. Changes: o Store both the original header and the canonical header in LSP command. o Also check that both original and canonical headers are not already in

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

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE325523: [clangd] Fixes for #include insertion. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D43462?vs=134947&id=134952#toc Repository: rL LLVM https://rev

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL325523: [clangd] Fixes for #include insertion. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D43462 Files: clang-tools-ex

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

ioeric updated this revision to Diff 134947. ioeric added a comment. - assert in the very beginning! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43462 Files: clangd/ClangdServer.cpp clangd/Headers.cpp clangd/Headers.h clangd/index/CanonicalIncludes.cpp clangd/index

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

ioeric updated this revision to Diff 134944. ioeric marked 2 inline comments as done. ioeric added a comment. - added a comment about thread safety Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43462 Files: clangd/ClangdServer.cpp clangd/Headers.cpp clangd/Headers.h cl

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

ioeric marked 5 inline comments as done. ioeric added inline comments. Comment at: clangd/Headers.cpp:60 Argv.push_back(S.c_str()); IgnoringDiagConsumer IgnoreDiags; auto CI = clang::createInvocationFromCommandLine( ilya-biryukov wrote: > Not related t

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

ioeric updated this revision to Diff 134943. ioeric marked 3 inline comments as done. ioeric added a comment. - addressed comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43462 Files: clangd/ClangdServer.cpp clangd/Headers.cpp clangd/Headers.h clangd/index/Canon

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

ioeric updated this revision to Diff 134932. ioeric marked 6 inline comments as done. ioeric added a comment. - Stop indexing main files in dynamic index; addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43462 Files: clangd/ClangdServer.cpp clangd/H

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

ioeric added inline comments. Comment at: clangd/Headers.cpp:45 +bool hasHeaderExtension(PathRef Path) { + constexpr static const char *HeaderExtensions[] = {".h", ".hpp", ".hh", + ".hxx"}; ilya-biryukov wrote:

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

ioeric created this revision. ioeric added reviewers: sammccall, ilya-biryukov. Herald added subscribers: cfe-commits, jkorous-apple, klimek. o Avoid inserting a header include into the header itself. o Avoid inserting non-header files. o Canonicalize include paths for symbols in dynamic index.

[PATCH] D43437: Fix link failures for Preprocessor::addCommentHandler

ioeric accepted this revision. ioeric added a subscriber: malaperle. ioeric added a comment. This revision is now accepted and ready to land. lg. Thanks! fyi, @malaperle also sent https://reviews.llvm.org/D43429 for the same fix but has not landed it yet ;) Repository: rCTE Clang Tools Extra

[PATCH] D43429: [clangd] Add missing library (clangLex) in a few places

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

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

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE325343: [clangd] collect symbol #include & insert #include in global code completion. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D42640?vs=134600&id=134603#

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL325343: [clangd] collect symbol #include & insert #include in global code completion. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.l

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

ioeric updated this revision to Diff 134600. ioeric marked 3 inline comments as done. ioeric added a comment. - Merged with origin/master - Addressed review comments; removed .inc handling. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42640 Files: clangd/CMakeLists.txt cl

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

ioeric added a comment. Thank you for reviewing this! Comment at: clangd/index/CanonicalIncludes.h:52 + /// a canonical header name. + llvm::StringRef mapHeader(llvm::StringRef Header) const; + sammccall wrote: > ioeric wrote: > > sammccall wrote: > > > So I'

[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 `Documenta

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

ioeric updated this revision to Diff 134394. ioeric added a comment. - Merged 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 clangd/CodeCompl

[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 &Params) { + if (Params.rootUri && !Params.rootUri->file.empty()) +Se

[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. > > W

[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) { ilya-biryuk

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

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

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

[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

<    5   6   7   8   9   10   11   12   13   14   >