[PATCH] D82352: [clangd] Make background index thread count calculation clearer

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. Thanks LGTM! Comment at: clang-tools-extra/clangd/index/Background.h:140 + // In production an explicit value is passed. + size_t ThreadPoolSize = 4, std

[PATCH] D82373: [CodeComplete] Tweak code completion for `typename`

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, as previous version only saves a single character, i.e. pressing a single tab after qualifier vs hitting `:` twice, while it is annoying for the non-qualified case, now you need to d

[PATCH] D82386: [clangd] Config: Fragments and parsing from YAML

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ConfigFragment.h:9 +// +// Various clangd features have configurable behaviour (or can be disabled). +// The configuration system allows users to control this: i think this paragraph belongs to

[PATCH] D81456: [clangd] Get rid of WantDiagnostics::Yes

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

[PATCH] D82386: [clangd] Config: Fragments and parsing from YAML

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ConfigYAML.cpp:29 + llvm::SourceMgr &SM; + llvm::SmallString<256> Buf; + a comment for this buf, especially the reason why it's a member rather than a function-local when needed. (I suppose t

[PATCH] D82386: [clangd] Config: Fragments and parsing from YAML

kadircet added a comment. Thanks! Looks great, I've finally managed to finish all my iterations :D Apart from the comments(IIRC which are mostly minor), it felt like there were a few lines that aren't clang-format'd. Please make sure to run clang-format on the files at some point. ==

[PATCH] D82373: [CodeComplete] Tweak code completion for `typename` and `using`.

kadircet added inline comments. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:2058 Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace); - Builder.AddPlaceholderChunk("qualifier"); - Builder.AddTextChunk("::"); unfortunately the reasoning

[PATCH] D82326: [clangd] Disable printing of Value for tag-types on hover

kadircet updated this revision to Diff 273032. kadircet marked an inline comment as done. kadircet added a comment. - tag -> record in comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82326/new/ https://reviews.llvm.org/D82326 Files: clang

[PATCH] D82326: [clangd] Disable printing of Value for tag-types on hover

This revision was automatically updated to reflect the committed changes. Closed by commit rG6a3cffce3e80: [clangd] Disable printing of Value for tag-types on hover (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82326/new/ htt

[PATCH] D82373: [CodeComplete] Tweak code completion for `typename` and `using`.

kadircet added inline comments. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:2058 Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace); - Builder.AddPlaceholderChunk("qualifier"); - Builder.AddTextChunk("::"); lh123 wrote: > lh123 wrote:

[PATCH] D82497: [Clang][SourceManager] optimize getFileIDLocal()

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Btw, I've recently learned about http://llvm-compile-time-tracker.com/ so you might want to check the effect afterwards. Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D82498: [SourceManager] don't check invalid param of getLocalSLocEntry()

kadircet added inline comments. Comment at: clang/include/clang/Basic/SourceManager.h:1649 const SrcMgr::SLocEntry &getLocalSLocEntry(unsigned Index, bool *Invalid = nullptr) const { assert(Index < LocalSLocEntryTable.size() &

[PATCH] D82532: [libclang] Get rid of relience on SourceManager member signature

kadircet created this revision. kadircet added reviewers: sammccall, bkramer. Herald added subscribers: cfe-commits, arphaman. Herald added a project: clang. Libclang was enforcing a singature on SourceManager::getLocalSLocEntry which isn't possible to satisfy as pointed out in https://reviews.llv

[PATCH] D82532: [libclang] Get rid of relience on SourceManager member signature

This revision was automatically updated to reflect the committed changes. Closed by commit rGb5d3abea228b: [libclang] Get rid of relience on SourceManager member signature (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82532/ne

[PATCH] D82535: [CodeComplete] Add code completion for using alias.

kadircet added a comment. mostly LG, apart from the comments around the fallthrough, thanks! Comment at: clang/lib/Sema/SemaCodeComplete.cpp:2061 +if (SemaRef.getLangOpts().CPlusPlus11) + AddUsingAliasResult(Results); there's already a fallthrough do

[PATCH] D82498: [SourceManager] don't check invalid param of getLocalSLocEntry()

kadircet added inline comments. Comment at: clang/include/clang/Basic/SourceManager.h:1649 const SrcMgr::SLocEntry &getLocalSLocEntry(unsigned Index, bool *Invalid = nullptr) const { assert(Index < LocalSLocEntryTable.size() &

[PATCH] D82386: [clangd] Config: Fragments and parsing from YAML

kadircet marked 5 inline comments as done. kadircet added a comment. LGTM with couple of nits. Regarding clang-format, I was mostly confused by `template class Located {` being on a single line, but apparently that's the style :D Comment at: clang-tools-extra/clangd/ConfigFr

[PATCH] D82535: [CodeComplete] Add code completion for using alias.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Will land this one too. In the meantime feel free to apply for commit access, as explained in https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access (IIRC, you

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! This was also my final plan after investigating in https://reviews.llvm.org/D81920, but then I forgot ... Comment at: clang/CMakeLists.txt:398 + if (CMAK

[PATCH] D82606: [clangd] Config: config struct propagated through Context

kadircet added a comment. Thanks, LGTM. Just a question around the order of config vs other mangling. Comment at: clang-tools-extra/clangd/CompileCommands.cpp:187 + // FIXME: remove const_cast once unique_function is const-compatible. + for (auto &Edit : const_cast(Config::cu

[PATCH] D82498: [SourceManager] don't check invalid param of getLocalSLocEntry()

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82498/new/ https://reviews.llvm.org/D82498 __

[PATCH] D82497: [Clang][SourceManager] optimize getFileIDLocal()

kadircet added a comment. For the record: http://llvm-compile-time-tracker.com/compare.php?from=ed8184b7814df4310dbad065a9a1c3bb8f3bfa86&to=408efffbe4a52bae05f1677a47eb3ccfd5cdc1d3&stat=instructions https://photos.app.goo.gl/fJUS5jrMaJjvvmo1A Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D82535: [CodeComplete] Add code completion for using alias.

kadircet requested changes to this revision. kadircet added a comment. This revision now requires changes to proceed. this seems to be failing to merge, could you rebase on top of head? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82535/new/ https

[PATCH] D82373: [CodeComplete] Tweak code completion for `typename`.

This revision was automatically updated to reflect the committed changes. Closed by commit rG834c71829cc2: [CodeComplete] Tweak code completion for `typename`. (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82373/new/ https://

[PATCH] D82535: [CodeComplete] Add code completion for using alias.

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG5547a83c0b68: [CodeComplete] Add code completion for using alias. (authored by kadircet). Repository: rG LLVM Github Mo

[PATCH] D82606: [clangd] Config: config struct propagated through Context

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. oops, thought I've stamped it last time. Comment at: clang-tools-extra/clangd/CompileCommands.cpp:187 + // FIXME: remove const_cast once unique_function is const-compati

[PATCH] D80681: [clang][SourceManager] cache Macro Expansions

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM! Thanks for bearing with me on splitting the patches and such! Let's land it and see if this is going to regress any projects. Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D81169: [clangd] Improve hover on arguments to function call

kadircet added a comment. thanks a lot! haven't checked the tests yet. Comment at: clang-tools-extra/clangd/Hover.cpp:716 + for (unsigned I = 0; I < CE->getNumArgs() && I < FD->getNumParams(); ++I) { +auto *Arg = CE->getArg(I); +if (Arg != OuterNode.ASTNode.get())

[PATCH] D82690: [clang][SourceManager] cache Macro Expansions pt.2

kadircet added a comment. yes let's revert dffc1420451f674 and reland with this change included, also please keep the `Differential Revision: https://reviews.llvm.org/D80681` line on the commit message, so that it gets assoc

[PATCH] D82701: [clangd][Hover] Dont crash on null types

kadircet created this revision. kadircet added reviewers: hokein, sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D82701 Files: clang-tools-ext

[PATCH] D82729: [clangd] Make sure headers are available in FS inside FindSymbolsTests

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D82729 Files: clang-tools-extra/clan

[PATCH] D82736: [clangd] Rename FS.view(None) to FS.viewWithDefaultCWD()

kadircet added a comment. Thanks, renaming was also another option we had in mind, see https://reviews.llvm.org/D81920#2109901 and possibly the following comments. I thought it was discussed in the disable-the-warning thread, but to elaborate a little more: Naming is hard in general, this case

[PATCH] D82701: [clangd][Hover] Dont crash on null types

kadircet updated this revision to Diff 274000. kadircet marked 2 inline comments as done. kadircet added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82701/new/ https://reviews.llvm.org/D82701 Files: clang-tools-extr

[PATCH] D82701: [clangd][Hover] Dont crash on null types

This revision was automatically updated to reflect the committed changes. Closed by commit rG117b9230a74c: [clangd][Hover] Dont crash on null types (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82701/new/ https://reviews.llvm

[PATCH] D82701: [clangd][Hover] Dont crash on null types

kadircet marked 4 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:302 P.Type = printType(PVD->getType(), Policy); } else { std::string Param; hokein wrote: > nit: now we can remove this if

[PATCH] D82793: [clangd] Suppress GCC -Woverloaded-virtual by renaming ThreadsafeFS extension point

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, LGTM! Comment at: clang-tools-extra/clangd/Preamble.cpp:232 class EmptyFS : public ThreadsafeFS { public: +llvm::IntrusiveRefCntPtr viewImpl() const ove

[PATCH] D82729: [clangd] Make sure headers are available in FS inside FindSymbolsTests

kadircet added a comment. In D82729#2120647 , @sammccall wrote: > Why? workspaceSymbols depends on the index only. > ISTM the dynamic index + blockUntilIdle should be enough - what does the FS > have to do with it? Yes it does, but tests that I updated

[PATCH] D82891: [clangd] Fix race in FileIndex that sometimes temporarily lost updates.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.h:97 + unsigned Version = 0; llvm::StringMap> SymbolsSnapshot; oh and also maybe use `size_t` ? as there's only one (well three actually, if you count both preamble and m

[PATCH] D82891: [clangd] Fix race in FileIndex that sometimes temporarily lost updates.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:269 + +++this->Version; +if (Version) Bumping the version in `update` and

[PATCH] D81169: [clangd] Improve hover on arguments to function call

kadircet accepted this revision. kadircet marked an inline comment as done. kadircet added a comment. This revision is now accepted and ready to land. Thanks, LGTM! Comment at: clang-tools-extra/clangd/Hover.cpp:929 +if (CallPassType->PassBy != HoverInfo::PassType::Value) {

[PATCH] D82944: [clangd] Switch FindSymbolsTests to use TestTU

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. This gets rid of the dependency on ClangdServer and a bunch of extra infrastructure coming with it. Als

[PATCH] D77392: [WIP][clangd] Make signatureHelp work with stale preambles

kadircet updated this revision to Diff 258064. kadircet added a comment. - Use preprocessor instead of raw lexer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77392/new/ https://reviews.llvm.org/D77392 Files: clang-tools-extra/clangd/ClangdServe

[PATCH] D77392: [WIP][clangd] Make signatureHelp work with stale preambles

kadircet added a comment. This is ready for another round. To summarize the current state: - `PreamblePatch` can be create from a `ParseInput` and a `PreambleData`, - It will preprocessor preamble section of current file contents using a dummy FS to reduce cost of stat/realpaths - It won't p

[PATCH] D78359: [clangd] Do not store BaseOf relations for missing subjects

kadircet created this revision. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D78359 Files: clang-tools-extra/clangd/index/SymbolCollector.cpp Index:

[PATCH] D78359: [clangd] Do not store BaseOf relations for missing subjects

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG4503cf5f2313: [clangd] Drop dangling relations while sharding (authored by kadircet). Changed prior to commit: https://

[PATCH] D78366: [Preamble] Allow recursive inclusion of header-guarded mainfile.

kadircet added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:2070 + diag::err_pp_including_mainfile_in_preamble); +return {ImportAction::None}; + } Instead of returning here I think we should set the Action to `Skip`. So that relevant

[PATCH] D78429: [clangd] Trace ASTCache accesses

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, javed.absar, ilya-biryukov. Herald added a project: clang. Expose AST cache access statistics. Instead of doing this we can also expose EventTracer::

[PATCH] D78366: [Preamble] Allow recursive inclusion of header-guarded mainfile.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang/lib/Lex/PPDirectives.cpp:2070 + diag::err_pp_including_mainfile_in_preamble); +return {ImportAction::None}; + } -

[PATCH] D78429: [clangd] Trace ASTCache accesses

kadircet updated this revision to Diff 258559. kadircet added a comment. Extend tracer api to enable exporting metrics. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78429/new/ https://reviews.llvm.org/D78429 Files: clang-tools-extra/clangd/TUSc

[PATCH] D78436: [clangd] Record metrics for code action and rename usage

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Depends on D78429 . Repository: rG LLVM Github Monorepo https://r

[PATCH] D78429: [clangd] Metric tracking through Tracer

kadircet updated this revision to Diff 258591. kadircet added a comment. - Introduce latency tracking through context. - Add metrics for code actions and rename. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78429/new/ https://reviews.llvm.org/D784

[PATCH] D78429: [clangd] Metric tracking through Tracer

kadircet updated this revision to Diff 258590. kadircet added a comment. - Report metrics on destruction. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78429/new/ https://reviews.llvm.org/D78429 Files: clang-tools-extra/clangd/TUScheduler.cpp

[PATCH] D78436: [clangd] Record metrics for code action and rename usage

kadircet abandoned this revision. kadircet added a comment. Merged into D78429 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78436/new/ https://reviews.llvm.org/D78436 ___

[PATCH] D78429: [clangd] Metric tracking through Tracer

kadircet updated this revision to Diff 258595. kadircet added a comment. Herald added a subscriber: ormris. - Add tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78429/new/ https://reviews.llvm.org/D78429 Files: clang-tools-extra/clangd/Clan

[PATCH] D77392: [WIP][clangd] Make signatureHelp work with stale preambles

kadircet marked 15 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:278 + // This shouldn't coincide with any real file name. + PP.PatchFileName = llvm::formatv("{0}_preamble_patch.h", FileName); + sammc

[PATCH] D77392: [WIP][clangd] Make signatureHelp work with stale preambles

kadircet updated this revision to Diff 258833. kadircet marked 2 inline comments as done. kadircet added a comment. - Address comments - Don't store LexedInclude in PreambleData, expose the contents in PrecompiledPreamble instead. - Introduce DenseMapInfo for PPKeywordKind Repository: rG LLVM

[PATCH] D77456: [clangd] Parse `foo` in documentation comments and render as code.

kadircet added a comment. In D77456#1991484 , @sammccall wrote: > No, I'm complaining about the space before the period in > > Tests primality of `p` . > > > and the plaintext rendering too > > Tests primality of p . > Ah I see, yes this looks annoyi

[PATCH] D77392: [WIP][clangd] Make signatureHelp work with stale preambles

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:281 + // We are only interested in newly added includes. + llvm::StringSet<> ExistingIncludes; + for (const auto &Inc : Preamble.LexedIncludes)

[PATCH] D77392: [WIP][clangd] Make signatureHelp work with stale preambles

kadircet updated this revision to Diff 258915. kadircet marked 2 inline comments as done. kadircet added a comment. - Make scanPreambleIncludes return an Expected> - Bail out in case of errors, document the rational. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D78235: [clangd] Store ppdirective in Inclusion

This revision was automatically updated to reflect the committed changes. Closed by commit rG6e017188b760: [clangd] Store ppdirective in Inclusion (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D78235?vs=257812&id=258926#toc Repository: rG LLVM Github Monorepo CHA

[PATCH] D77392: [clangd] Make signatureHelp work with stale preambles

This revision was automatically updated to reflect the committed changes. Closed by commit rG2214b9076f1d: [clangd] Make signatureHelp work with stale preambles (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77392/new/ https:/

[PATCH] D78649: [clang] Make sure argument expansion locations are correct in presence of predefined buffer

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a project: clang. Herald added a subscriber: cfe-commits. Macro argument expansion logic relies on skipping file IDs that created as a result of an include. Unfortunately it fails to do that for predefined buffer si

[PATCH] D78626: [clangd] Fix a crash for accessing a null template decl returned by findExplicitReferences.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:1293 +struct Base {}; +namespace foo { +template ---

[PATCH] D78649: [clang] Make sure argument expansion locations are correct in presence of predefined buffer

kadircet marked 4 inline comments as done. kadircet added inline comments. Comment at: clang/lib/Lex/PPLexerChange.cpp:420 + // Predefines file doesn't have a valid include location. + CurPPLexer->FID == getPredefinesFileID())) { // Notify SourceManager to

[PATCH] D78649: [clang] Make sure argument expansion locations are correct in presence of predefined buffer

kadircet updated this revision to Diff 259332. 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/D78649/new/ https://reviews.llvm.org/D78649 Files: clang/lib/Basi

[PATCH] D78649: [clang] Make sure argument expansion locations are correct in presence of predefined buffer

This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rG411a254af3ff: [clang] Make sure argument expansion locations are correct in presence of… (authored by kadircet). Changed prior to commit: https://reviews.llvm.

[PATCH] D78740: [clangd] Handle PresumedLocations in IncludeCollector

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. This will enable extraction of correct line locations in preamble patch for includes. Repository: r

[PATCH] D78743: [clangd] Preserve line information while build PreamblePatch

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Depends on D78740 . Repository: rG LLVM Github Monorepo https://r

[PATCH] D78629: [clangd] Look for compilation database in `build` subdirectory of parents.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. sorry this fell of my radar. LGTM. As discussed offline i am a little bit worried about regressing the case when there's a `build` in a subdirectory, and a (merged) compilation database o

[PATCH] D78833: [clangd] Disable all dependency outputs

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Fixes https://github.com/clangd/clangd/issues/322 Repository: rG LLVM Github Monorepo https://revi

[PATCH] D78836: [clangd] Strip /showIncludes in clangd compile commands

kadircet added inline comments. Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:101 // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD. - if (!Arg.startswith("-M")) { + if (!Arg.startswith("-M") && Arg != "/showIncludes") { AdjustedArgs.push_back(Args[i]); -

[PATCH] D78833: [clangd] Disable all dependency outputs

This revision was automatically updated to reflect the committed changes. Closed by commit rG294b9d43cae7: [clangd] Disable all dependency outputs (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78833/new/ https://reviews.llvm.

[PATCH] D78848: [clangd] Disable delayed template parsing in the main file

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78848/new/ https://reviews.llvm.org/D78848 __

[PATCH] D78836: [clangd] Strip /showIncludes in clangd compile commands

kadircet added inline comments. Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:101 // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD. - if (!Arg.startswith("-M")) { + if (!Arg.startswith("-M") && Arg != "/showIncludes") { AdjustedArgs.push_back(Args[i]); -

[PATCH] D78836: [clangd] Strip /showIncludes in clangd compile commands

kadircet accepted this revision. kadircet added a comment. thanks, lgtm. do you have commit access? Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:101 // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD. - if (!Arg.startswith("-M")) { + if (!Arg.startswith("-M") &&

[PATCH] D77644: [clangd] Handle additional includes while parsing ASTs

kadircet updated this revision to Diff 260228. kadircet added a comment. - Use PreamblePatch - Add more tests - Handle deleted includes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77644/new/ https://reviews.llvm.org/D77644 Files: clang-tools-e

[PATCH] D77644: [clangd] Handle additional includes while parsing ASTs

kadircet added a comment. this is ready for review now, to explain the flow: While creating a patch we also record remaining includes from Baseline left in Modified, as the new ones will be put into the patch already. That way ParsedAST can keep track of latest state of the preamble + main file

[PATCH] D77644: [clangd] Handle additional includes while parsing ASTs

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/Preamble.h:115 + /// mutate include depths. + void patchPreambleIncludes(IncludeStructure &BaselineIncludes) const; + i am not happy about this inter

[PATCH] D79079: [clangd] Make use of URIs in FileShardedIndex

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. This makes FileShardedIndex more robust and gets rid of the need for a URIToFileCache, as it is done by

[PATCH] D79079: [clangd] Make use of URIs in FileShardedIndex

kadircet updated this revision to Diff 260878. kadircet added a comment. - Return None and change PathRef to auto since it is URI now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79079/new/ https://reviews.llvm.org/D79079 Files: clang-tools-ex

[PATCH] D79079: [clangd] Make use of URIs in FileShardedIndex

kadircet updated this revision to Diff 261061. kadircet marked 2 inline comments as done. kadircet added a comment. - Update FileSymbols to express keys are not necessarily file paths. - Use Uri for keys of PreambleSymbols Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D79079: [clangd] Make use of URIs in FileShardedIndex

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/Background.cpp:183 const auto &IGN = IndexIt.getValue(); // Note that sources do not contain any information regarding missing // headers, since we

[PATCH] D79079: [clangd] Make use of URIs in FileShardedIndex

This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rG97c407db7725: [clangd] Make use of URIs in FileShardedIndex (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D79079?vs=261061&id=26114

[PATCH] D78429: [clangd] Metric tracking through Tracer

kadircet marked 8 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/Trace.h:35 +/// itself to active tracer on destruction. +struct Metric { + enum Type { sammccall wrote: > Conceptually there's a difference between a

[PATCH] D78429: [clangd] Metric tracking through Tracer

kadircet updated this revision to Diff 261156. kadircet marked an inline comment as done. kadircet added a comment. - Separate concept of a metric and measurement - Tie latency tracking to trace::Spans. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D79157: [clangd] Render code complete documentation as plaintext/markdown.

kadircet marked an inline comment as done. kadircet added a comment. thanks, mostly LG apart from dropping of white-space only comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:377 + auto SetDoc = [&](llvm::StringRef Doc) { +if (!Doc.trim().empty()) {

[PATCH] D79142: [clangd] Render doc-comment code spans with `backticks` in plaintext mode

kadircet added inline comments. Comment at: clang-tools-extra/clangd/FormattedString.cpp:365 +llvm::StringRef Marker = ""; +if (C.Preserve && C.Kind == Chunk::InlineCode) + Marker = "`"; should we rather use `renderInlineBlock` here ? because in pres

[PATCH] D79139: [clangd] Fix whitespace between chunks in markdown paragraphs.

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/FormattedString.cpp:419 C.Contents = std::move(Norm); C.Kind = Chunk::InlineCode; return *this; i think we always want a space before code chu

[PATCH] D79142: [clangd] Render doc-comment code spans with `backticks` in plaintext mode

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM thanks! Comment at: clang-tools-extra/clangd/FormattedString.cpp:365 +llvm::StringRef Marker = ""; +if (C.Preserve && C.Kind == Chunk::InlineCode) + Mar

[PATCH] D78429: [clangd] Metric tracking through Tracer

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

[PATCH] D78429: [clangd] Metric tracking through Tracer

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:46 +// Tracks end-to-end latency of high level lsp calls. +constexpr trace::Metric LSPLatencies("lsp_latencies", sammccall wrote: > sammccall wrote: > > comment should ment

[PATCH] D79157: [clangd] Render code complete documentation as plaintext/markdown.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM thanks! Comment at: clang-tools-extra/clangd/CodeComplete.cpp:377 + auto SetDoc = [&](llvm::StringRef Doc) { +if (!Doc.trim().empty()) { + Comp

[PATCH] D78429: [clangd] Metric tracking through Tracer

kadircet updated this revision to Diff 261329. kadircet marked 4 inline comments as done. kadircet added a comment. Herald added a subscriber: mgorny. - Test real metrics Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78429/new/ https://reviews.llvm

[PATCH] D78429: [clangd] Metric tracking through Tracer

kadircet added a comment. In D78429#2012873 , @sammccall wrote: > Awesome, ship it! > > ... though, how do you feel about testing the actual metrics we export? > > Suggest a slightly generalized TestTracer that installs itself (RAII), and > has a > > //

[PATCH] D79139: [clangd] Fix whitespace between chunks in markdown paragraphs.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/FormattedString.cpp:439 + bool AdjacentCode = + !Chunks.empty() && Chunks.back().Kind == Chunk::InlineCode; std::string Norm = canonicalizeSpaces(std::move(Code)); I would say we should

[PATCH] D78429: [clangd] Metric tracking through Tracer

kadircet updated this revision to Diff 261441. kadircet marked 4 inline comments as done. kadircet added a comment. - Change IsEmpty with SizeIs - Make TestTracer thread-safe Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78429/new/ https://reviews.

[PATCH] D79139: [clangd] Fix whitespace between chunks in markdown paragraphs.

kadircet accepted this revision. kadircet marked an inline comment as done. kadircet added a comment. This revision is now accepted and ready to land. thanks lgtm! Comment at: clang-tools-extra/clangd/FormattedString.cpp:439 + bool AdjacentCode = + !Chunks.empty() && Chun

[PATCH] D78429: [clangd] Metric tracking through Tracer

kadircet updated this revision to Diff 261514. kadircet added a comment. - Track renamed files instead of occurences Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78429/new/ https://reviews.llvm.org/D78429 Files: clang-tools-extra/clangd/ClangdL

[PATCH] D78740: [clangd] Handle PresumedLocations in IncludeCollector

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Headers.cpp:40 + auto PreLoc = SM.getPresumedLoc(HashLoc); + if (auto FE = SM.getFileManager().getFile(PreLoc.getFilename())) { +if (SM.getFileEntryForID(SM.getMainFileID()) == *FE) { -

[PATCH] D78429: [clangd] Metric tracking through Tracer

This revision was automatically updated to reflect the committed changes. kadircet marked 2 inline comments as done. Closed by commit rGe64f99c51a8e: [clangd] Metric tracking through Tracer (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D78429?vs=261514&id=261686#toc

<    9   10   11   12   13   14   15   16   17   18   >