[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

kadircet updated this revision to Diff 191465. kadircet marked an inline comment as done. kadircet added a comment. - Make limit a hardcoded value rather then a command line argument - Limit diags per include directive Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://rev

[PATCH] D59407: [clangd] Add RelationSlab

kadircet added a comment. In D59407#1432543 , @nridge wrote: > If it's (B), how many bytes should the index be? Are the space gains worth > the complexity, given that `SymbolID` is only 8 bytes to begin with? (As > compared to say, the filenames in `Ref`

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

kadircet added a comment. Could you do a rebase? I've tried to land it but there are too many merge conflicts currently. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150 ___ cfe-commits mailing list

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

kadircet added a comment. In D52150#1436278 , @to-miz wrote: > Is it because the diff was made with the svn repository? It is for the newest > revision. > Should I make a new diff with the git repo instead? > Or is it not because of the diff, but someth

[PATCH] D59599: [clangd] Fix a crash while printing Template Arguments

kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, jdoerfert, arphaman, jkorous, MaskRay, ioeric. Herald added a project: clang. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D59599 Files: clangd/AST.cpp unittests

[PATCH] D59605: [clangd] Introduce background-indexer

kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, jdoerfert, erik.pilkington, arphaman, jkorous, MaskRay, ioeric, mgorny. Herald added a project: clang. Sample output: I[18:37:50.083] BackgroundIndex: build symbol index periodically e

[PATCH] D59639: [clangd] Print template arguments helper

kadircet created this revision. kadircet added reviewers: ioeric, ilya-biryukov. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, mgorny. Herald added a project: clang. Prepares ground for printing template arguments as written in the source code, part of re-landing rC356541

[PATCH] D59640: [clangd] Add TemplateArgumentList into Symbol

kadircet created this revision. kadircet added reviewers: ioeric, ilya-biryukov, gribozavr. Herald added subscribers: cfe-commits, jdoerfert, arphaman, jkorous, MaskRay. Herald added a project: clang. Part of re-landing rC356541 with D59599

[PATCH] D59641: [clangd] Show template argument list in workspacesymbols and documentsymbols responses

kadircet created this revision. kadircet added reviewers: ioeric, ilya-biryukov. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay. Herald added a project: clang. kadircet added a parent revision: D59640: [clangd] Add TemplateArgumentList into Symbol. kadircet added a subscriber: n

[PATCH] D59605: [clangd] Introduce background-indexer

kadircet updated this revision to Diff 191684. kadircet marked 11 inline comments as done. kadircet added a comment. Herald added a subscriber: jfb. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59605/new/ https://reviews.llvm.or

[PATCH] D59605: [clangd] Introduce background-indexer

kadircet added inline comments. Comment at: clang-tools-extra/clangd/background-indexer/BackgroundIndexer.cpp:13 + +#include "/usr/local/google/home/kadircet/repos/llvm/clang-tools-extra/clangd/Context.h" +#include "/usr/local/google/home/kadircet/repos/llvm/clang-tools-extra/c

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

kadircet added inline comments. Comment at: clangd/Diagnostics.cpp:396 + for (llvm::StringRef Inc : IncludeStack) +OS << "In file included from: " << Inc << ":\n"; +} ilya-biryukov wrote: > NIT: could we reuse the function from clang

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

kadircet updated this revision to Diff 191831. kadircet marked 8 inline comments as done. kadircet added a comment. - Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59302/new/ https://reviews.llvm.org/D59302 Files: clangd/ClangdUn

[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex

kadircet updated this revision to Diff 191844. kadircet marked 11 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59481/new/ https://reviews.llvm.org/D59481 Files: clang-tools-e

[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:46 +auto It = MergedSyms->find(Sym.first); +assert(It != MergedSyms->end() && "Reference to unknown symbol!"); +// Note that we increment References per each file ment

[PATCH] D59639: [clangd] Print template arguments helper

kadircet marked 15 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:88 static const TemplateArgumentList * getTemplateSpecializationArgs(const NamedDecl &ND) { if (auto *Func = llvm::dyn_cast(&ND)) ioeric

[PATCH] D59639: [clangd] Print template arguments helper

kadircet updated this revision to Diff 191857. kadircet marked an inline comment as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59639/new/ https://reviews.llvm.org/D59639 Files: clang-tools-ex

[PATCH] D59640: [clangd] Add TemplateArgumentList into Symbol

kadircet added a comment. In D59640#1438248 , @ioeric wrote: > should we update YAML? I suppose we are only keeping it alive for the sake of tests, but that seems like an enough reason updating that as well. Comment at: clang-tools-e

[PATCH] D59640: [clangd] Add TemplateArgumentList into Symbol

kadircet updated this revision to Diff 191860. kadircet marked 3 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59640/new/ https://reviews.llvm.org/D59640 Files: clang-tools-ex

[PATCH] D60323: [clangd] Include compile command inference in fileStatus API

kadircet added a comment. What about surfacing compile command itself as well? IMO, it is an important information that we always extract from logs by digging currently. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60323/new/ https://reviews.llvm.

[PATCH] D60267: [clangd] Support relatedInformation in diagnostics.

kadircet added inline comments. Comment at: clangd/Protocol.cpp:280 R.DiagnosticFixes = *CodeActions; + if (auto CodeActions = Diagnostics->getBoolean("relatedInformation")) +R.DiagnosticRelatedInformation = *CodeActions; `s/CodeActions/Rela

[PATCH] D59599: [clangd] Fix a crash while printing Template Arguments

kadircet updated this revision to Diff 194470. kadircet marked 3 inline comments as done. kadircet added a comment. - Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59599/new/ https://reviews.llvm.org/D59599 Files: clangd/AST.cpp

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

kadircet added inline comments. Comment at: clangd/Diagnostics.cpp:396 + for (llvm::StringRef Inc : IncludeStack) +OS << "In file included from: " << Inc << ":\n"; +} ilya-biryukov wrote: > kadircet wrote: > > ilya-biryukov wrote: > >

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

kadircet updated this revision to Diff 194528. kadircet marked 10 inline comments as done. kadircet added a comment. - Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59302/new/ https://reviews.llvm.org/D59302 Files: clangd/ClangdU

[PATCH] D59641: [clangd] Show template argument list in workspacesymbols and documentsymbols responses

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:112 if (!Out.str().empty()) { -// FIXME(ibiryukov): do not show args not explicitly written by the user. -if (auto *ArgList = getTemplateSpecializati

[PATCH] D59639: [clangd] Print template arguments helper

kadircet added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:133 +printTemplateArgumentList(OS, *Args, Policy); + else if (auto *Cls = llvm::dyn_cast(&ND)) { +if (const TypeSourceInfo *TSI = Cls->getTypeAsWritten()) { ioeric wrote: > kadi

[PATCH] D59639: [clangd] Print template arguments helper

kadircet updated this revision to Diff 194657. kadircet marked 7 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59639/new/ https://reviews.llvm.org/D59639 Files: clang-tools-ex

[PATCH] D59639: [clangd] Print template arguments helper

kadircet updated this revision to Diff 194658. kadircet added a comment. - Update file comment for PrintASTTests.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59639/new/ https://reviews.llvm.org/D59639 Files: clang-tools-extra/clangd/AST.cpp

[PATCH] D59639: [clangd] Print template arguments helper

kadircet updated this revision to Diff 194686. kadircet marked 6 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59639/new/ https://reviews.llvm.org/D59639 Files: clang-tools-ex

[PATCH] D59639: [clangd] Print template arguments helper

kadircet added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:139 + // location information. + printTemplateArgumentList(OS, Cls->getTemplateArgs().asArray(), Policy); +} ioeric wrote: > Could you also add a test case for this with th

[PATCH] D59605: [clangd] Introduce background-indexer

kadircet marked an inline comment as done. kadircet added a comment. Looking at the current state of `BackgroundIndex`, it has the following implementation details: - Loading of shards from storage - Storing of shards to storage - Collecting symbols from a TU - Sharding of symbol information col

[PATCH] D59641: [clangd] Show template argument list in workspacesymbols and documentsymbols responses

kadircet updated this revision to Diff 194810. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59641/new/ https://reviews.llvm.org/D59641 Files: clang-tools-extra/clangd/FindSymbols.cpp clang-tools-extra/clangd

[PATCH] D59640: [clangd] Add TemplateArgumentList into Symbol

kadircet added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59640/new/ https://reviews.llvm.org/D59640 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D59641: [clangd] Show template argument list in workspacesymbols and documentsymbols responses

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:112 if (!Out.str().empty()) { -// FIXME(ibiryukov): do not show args not explicitly written by the user. -if (auto *ArgList = getTemplateSpecializati

[PATCH] D59640: [clangd] Add TemplateArgumentList into Symbol

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:566 +printTemplateSpecializationArgs(ND); +S.TemplateSpecializationArgs = TemplateSpecializationArgs; if (Opts.StoreAllDocume

[PATCH] D59640: [clangd] Add TemplateArgumentList into Symbol

kadircet updated this revision to Diff 194820. kadircet marked 2 inline comments as done. kadircet added a comment. - Fill in the TemplateSpecializationArgs in all code paths. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59640/new/ https://reviews

[PATCH] D59599: [clangd] Fix a crash while printing Template Arguments

kadircet abandoned this revision. kadircet added a comment. It is incorporated into D59639 Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59599/new/ https://reviews.llvm.org/D59599

[PATCH] D59640: [clangd] Add TemplateArgumentList into Symbol

This revision was automatically updated to reflect the committed changes. Closed by commit rL358273: [clangd] Add TemplateArgumentList into Symbol (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://revie

[PATCH] D59639: [clangd] Print template arguments helper

This revision was automatically updated to reflect the committed changes. Closed by commit rL358272: [clangd] Print template arguments helper (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.ll

[PATCH] D59641: [clangd] Show template argument list in workspacesymbols and documentsymbols responses

This revision was automatically updated to reflect the committed changes. Closed by commit rL358274: [clangd] Show template argument list in workspacesymbols and documentsymbols… (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[PATCH] D60685: [clangd] Dont index Symbols with invalid source location

kadircet created this revision. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov. Herald added a project: clang. After rL358098 clangd started to index redecls of implicit symbols, but sometimes these decls lack the sour

[PATCH] D60689: [clangd] Fallback to OrigD when SLoc is invalid

kadircet created this revision. kadircet added a reviewer: sammccall. kadircet added a project: clang. Some implicit/built-in decls lack the source location information. Fallback to OrigD that we've seen in the source code instead of the canonical one in those cases. Repository: rG LLVM Github

[PATCH] D60689: [clangd] Fallback to OrigD when SLoc is invalid

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE358413: [clangd] Fallback to OrigD when SLoc is invalid (authored by kadircet, committed by ). Changed prior to commit: https://reviews.llvm.org/D60689?vs=195140&id=195179#toc Repository: rCTE Clan

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

kadircet added a comment. Ping Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59302/new/ https://reviews.llvm.org/D59302 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

kadircet updated this revision to Diff 195375. kadircet marked 14 inline comments as done. kadircet added a comment. - Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59302/new/ https://reviews.llvm.org/D59302 Files: clangd/Diagnos

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

kadircet added inline comments. Comment at: unittests/clangd/DiagnosticsTests.cpp:620 +ParsedAST build(const std::string &MainFile, +const llvm::StringMap &Files) { ilya-biryukov wrote: > We were discussing adding the extra files to `TestTU` ins

[PATCH] D58291: [clangd] Include textual diagnostic ID as Diagnostic.code.

kadircet accepted this revision. kadircet added a comment. In D58291#1469880 , @sammccall wrote: > It's a good question, it depends how this is surfaced, and we may want to > tweak the behavior or suppress entirely in some cases. > I think at least some

[PATCH] D60822: [clangd] Use shorter, more recognizable codes for diagnostics.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. I agree that these are more useful. What about also adding "-W" in front of warning flags to make them more explicit and stand out from "non-flag" error names? Repository: rCTE Clang T

[PATCH] D60819: [clangd] Strip the ' [some-check-name]' suffix from clang-tidy diagnostics. The check name is reported in Diagnostic.code.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, we might need to make sure clients that do not show "code" field become aware of this before LLVM9 releases Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https:/

[PATCH] D60827: [rename] Deduplicate symbol occurrences

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. Thanks Comment at: clang-tools-extra/unittests/clangd/ClangdTests.cpp:1160 +TEST_F(ClangdVFSTest, NoDuplicatedTextEditsOnRename) { + MockFSProvider FS; ---

[PATCH] D60267: [clangd] Support relatedInformation in diagnostics.

kadircet added inline comments. Comment at: clangd/Diagnostics.cpp:271 + if (!Note.AbsFile) { +log("Dropping note from unknown file: {0}", Note); +continue; ilya-biryukov wrote: > Maybe `vlog`? This is what we use for dropped diagnostics, sho

[PATCH] D60865: [clangd] Use llvm::set_thread_priority in background-index

kadircet created this revision. kadircet added a reviewer: gribozavr. Herald added subscribers: cfe-commits, jfb, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov, krytarowski. Herald added a project: clang. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D60865 Files: clangd/T

[PATCH] D60867: [clang][CIndex] Use llvm::set_thread_priority

kadircet created this revision. kadircet added reviewers: jkorous, gribozavr. Herald added subscribers: cfe-commits, arphaman, dexonsmith. Herald added a project: clang. Repository: rC Clang https://reviews.llvm.org/D60867 Files: tools/libclang/CIndex.cpp Index: tools/libclang/CIndex.cpp =

[PATCH] D60865: [clangd] Use llvm::set_thread_priority in background-index

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE358664: [clangd] Use llvm::set_thread_priority in background-index (authored by kadircet, committed by ). Changed prior to commit: https://reviews.llvm.org/D60865?vs=195721&id=195730#toc Repository:

[PATCH] D60867: [clang][CIndex] Use llvm::set_thread_priority

This revision was automatically updated to reflect the committed changes. Closed by commit rC358665: [clang][CIndex] Use llvm::set_thread_priority (authored by kadircet, committed by ). Changed prior to commit: https://reviews.llvm.org/D60867?vs=195725&id=195731#toc Repository: rC Clang CHA

[PATCH] D60873: [clang][HeaderSuggestion] Handle the case of dotdot with an absolute path

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, arphaman, jkorous, ioeric, ilya-biryukov. Herald added a project: clang. Include insertion in clangd was inserting absolute paths when the include directory was an absolute path with a doub

[PATCH] D60995: [clangd] Make sure include path does not contain any backslashes on Windows

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. kadircet added a child revision: D60873: [clang][HeaderSuggestion] Handle the case of dotdot with an absolute path

[PATCH] D60873: [clang][HeaderSuggestion] Handle the case of dotdot with an absolute path

kadircet added a comment. Sent out D60995 , hopefully it should fix the issue. Will wait until that lands. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60873/new/ https://reviews.llvm.org/D60873 ___

[PATCH] D60995: [clangd] Make sure include path does not contain any backslashes on Windows

kadircet added a comment. I had just looked at the method name and thought it was the right place( `suggestPathToFileForDiagnostics` since it says "path to file".) But looking at the comments, /// Suggest a path by which the specified file could be found, for /// use in diagnostics to sugge

[PATCH] D60995: [clang][HeaderSearch] Make sure there are no backslashes in suggestedPath

kadircet updated this revision to Diff 196192. kadircet added a comment. - Fix bug in HeaderSearch instead of clangd Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60995/new/ https://reviews.llvm.org/D60995 Files: clang/lib/Lex/HeaderSearch.cpp

[PATCH] D60995: [clang][HeaderSearch] Make sure there are no backslashes in suggestedPath

kadircet updated this revision to Diff 196392. kadircet added a comment. - Update documentation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60995/new/ https://reviews.llvm.org/D60995 Files: clang/include/clang/Lex/HeaderSearch.h clang/lib/Le

[PATCH] D60995: [clang][HeaderSearch] Make sure there are no backslashes in suggestedPath

kadircet updated this revision to Diff 196393. kadircet added a comment. - Update comments to focus on forward slashes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60995/new/ https://reviews.llvm.org/D60995 Files: clang/include/clang/Lex/Header

[PATCH] D60995: [clang][HeaderSearch] Make sure there are no backslashes in suggestedPath

This revision was automatically updated to reflect the committed changes. Closed by commit rL359075: [clang][HeaderSearch] Make sure there are no backslashes in suggestedPath (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prio

[PATCH] D60873: [clang][HeaderSuggestion] Handle the case of dotdot with an absolute path

This revision was automatically updated to reflect the committed changes. Closed by commit rL359078: [clang][HeaderSuggestion] Handle the case of dotdot with an absolute path (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prio

[PATCH] D61126: [clangd] Also perform merging for symbol definitions

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. clangd currently prefers declarations from codegen files. This patch implements that behavior for definition locat

[PATCH] D61077: [clangd] Query index in code completion no-compile mode.

kadircet added inline comments. Comment at: clangd/SourceCode.cpp:503 +case tok::l_brace: + if (State == NamespaceName) { +// Parsed: namespace { I believe it is safe to ignore(just mark the opening brace) anonymous namespaces here. Since there

[PATCH] D61126: [clangd] Also perform merging for symbol definitions

kadircet updated this revision to Diff 196654. kadircet added a comment. - Get rid of debug comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61126/new/ https://reviews.llvm.org/D61126 Files: clang-tools-extra/clangd/XRefs.cpp clang-tools

[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex

kadircet marked 2 inline comments as done. kadircet added a comment. It has been a long time since I've proposed that change and even I forgot some of the high level details. Therefore, I wanted to sum up the state again so that we can decide on how to move forward. Currently we have two differ

[PATCH] D61077: [clangd] Query index in code completion no-compile mode.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: unittests/clangd/SourceCodeTests.cpp:325 +TEST(SourceCodeTests, VisibleNamespaces) { + std::vector>> Cases = { sammccall wro

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

kadircet added inline comments. Comment at: clangd/Diagnostics.cpp:78 +// offsets when displaying that information to users. +Position toOneBased(Position P) { + ++P.line; ilya-biryukov wrote: > Could we avoid introducing a function that breaks the invariant of

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

kadircet updated this revision to Diff 196822. kadircet marked 5 inline comments as done. kadircet added a comment. - Address comments - Drop include stack Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59302/new/ https://reviews.llvm.org/D59302 Fil

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

kadircet updated this revision to Diff 197068. kadircet marked 2 inline comments as done. kadircet added a comment. - Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59302/new/ https://reviews.llvm.org/D59302 Files: clangd/Diagnost

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

kadircet updated this revision to Diff 197073. kadircet marked 8 inline comments as done. kadircet added a comment. - Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59302/new/ https://reviews.llvm.org/D59302 Files: clangd/Diagnost

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

kadircet updated this revision to Diff 197077. kadircet added a comment. - Rebase Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59302/new/ https://reviews.llvm.org/D59302 Files: clangd/Diagnostics.cpp clangd/Diagnostics.h clangd/unittests/Dia

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

This revision was automatically updated to reflect the committed changes. Closed by commit rL359432: [clangd] Surface diagnostics from headers inside main file (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM CHAN

[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex

kadircet updated this revision to Diff 197288. kadircet added a comment. - Change logic to count references from only main files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59481/new/ https://reviews.llvm.org/D59481 Files: clang-tools-extra/cl

[PATCH] D61316: [clangd] Improvements to header mapping: more precise parsing of cppreference symbol pages.

kadircet added inline comments. Comment at: clangd/include-mapping/gen_std.py:90 +# Decl may not be for the symbol name we're looking for. +if not re.search("\\b%s\\b" % symbol_name, text): + continue does this also work with functions? F

[PATCH] D61316: [clangd] Improvements to header mapping: more precise parsing of cppreference symbol pages.

kadircet added inline comments. Comment at: clangd/include-mapping/gen_std.py:60 + +def ParseSymbolPage(symbol_page_html, symbol_name): """Parse symbol page and retrieve the include header defined in this page. I think this requires some corresponding changes

[PATCH] D61316: [clangd] Improvements to header mapping: more precise parsing of cppreference symbol pages.

kadircet added inline comments. Comment at: clangd/include-mapping/gen_std.py:95 +if was_decl: + current_headers = [] +was_decl = False kadircet wrote: > why not perform clean-up unconditionally? ok I see the reason for that one, https:/

[PATCH] D61316: [clangd] Improvements to header mapping: more precise parsing of cppreference symbol pages.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. Thanks! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61316/new/ https://reviews.llvm.org/D61316 _

[PATCH] D61349: [clangd] Standard library mapping: prefer "primary" versions of functions over variants.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61349/new/ https://reviews.llvm.org/D61349 __

[PATCH] D61126: [clangd] Also perform merging for symbol definitions

kadircet updated this revision to Diff 197725. kadircet added a comment. - Update diff Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61126/new/ https://reviews.llvm.org/D61126 Files: clang-tools-extra/clangd/XRefs.cpp clang-tools-extra/clangd/

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.h:186 void findHover(PathRef File, Position Pos, - Callback> CB); + Callback> CB); sammccall wrote: > Not sure about switching from `Hover` to `

[PATCH] D61442: [clangd] Fix header-guard check for include insertion, and don't index header guards.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. Thanks! mostly LGTM, only confused about changing symbolcollector to produce a unique header per symbol(where as it was(could) produce multiple headers before). What is the rationale behin

[PATCH] D61126: [clangd] Also perform merging for symbol definitions

This revision was automatically updated to reflect the committed changes. kadircet marked 2 inline comments as done. Closed by commit rL359874: [clangd] Also perform merging for symbol definitions (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-

[PATCH] D61442: [clangd] Fix header-guard check for include insertion, and don't index header guards.

kadircet added inline comments. Comment at: clangd/index/SymbolCollector.h:130 + // The final spelling is calculated in finish(). + llvm::DenseMap IncludeFiles; + // Indexed macros, to be erased if they turned out to be include guards. sammccall wrote: > kadir

[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Change ClangdServer layer to output a structured response for Hover, which can be rendered by client according to

[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet planned changes to this revision. kadircet added a comment. This is still WIP, just looking for feedbacks on `HoverInfo` struct and overall layering Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61497/new/ https://reviews.llvm.org/D61497

[PATCH] D61519: [clangd] Support -fallback-style, similar to clang-format.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM Putting http://lists.llvm.org/pipermail/clangd-dev/2019-May/000411.html here for context. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D61588: [clangd] Expose whether no-compile completion was used.

kadircet added a comment. We already have `CodeCompletion::Origin` which is (at least should be)set to `Identifier` in those cases. Is there a reason for not using it? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61588/new/ https://reviews.llvm.or

[PATCH] D61588: [clangd] Expose whether no-compile completion was used.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. Thanks for the info. LGTM Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61588/new/ https://reviews.llvm.org/D61588 ___

[PATCH] D61596: [clangd] Move Rename into its own file, and add unit test. NFC

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM Comment at: clangd/ClangdServer.cpp:264 Callback> CB) { - auto Action = [Pos](Path File, std::string NewName, + auto Action = [Pos](std:

[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet updated this revision to Diff 198295. kadircet marked 8 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61497/new/ https://reviews.llvm.org/D61497 Files: clang-tools-ex

[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet added inline comments. Comment at: clang-tools-extra/clangd/XRefs.h:52 +struct HoverInfo { + LocatedSymbol Sym; + /// Name of the context containing the symbol. sammccall wrote: > I'm not sure about reuse of LocatedSymbol - do we want to commit to retu

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

kadircet added a comment. I think it makes more sense to expose semantical information in HoverInfo(like Name, Scope, Definition, etc), rather than formatted strings, and let this be serialized by the users of ClangdServer (as in D61497 ). This is what I'll perf

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

kadircet added a comment. > I'm happy to restructure the `HoverInfo` to your liking if the current one > seems problematic for any reason. It will just cause more code move on my side, no problems apart from that Comment at: clang-tools-extra/clangd/Protocol.cpp:707 + else +

[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet marked 2 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/XRefs.h:73 + llvm::Optional> Parameters; + llvm::Optional> TemplateParameters; + ilya-biryukov wrote: > What does `Type` mean for non-type and templa

[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet updated this revision to Diff 198420. kadircet marked an inline comment as done. kadircet added a comment. - Add comments for non-type params Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61497/new/ https://reviews.llvm.org/D61497 Files:

[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet updated this revision to Diff 198457. kadircet marked 2 inline comments as done. kadircet added a comment. I will start adding test cases(and most likely change the way I populate the struct at some places) if everyone is happy with current representation for `HoverInfo`. Repository:

[PATCH] D61497: [clangd] Introduce a structured hover response

kadircet added inline comments. Comment at: clang-tools-extra/clangd/XRefs.h:57 +// template class. +Type T; +std::string Name; ilya-biryukov wrote: > Same about type template parameters. > Probably just holds `class` or `typename`? Exactly, adding co

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