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

kadircet marked an inline comment as done. kadircet added a comment. thanks! that looks really useful, especially knowing when a parameter is being passed as a ref/val. Comment at: clang-tools-extra/clangd/Hover.cpp:292 +void fillParam(const ParmVarDecl *PVD, HoverInfo::Para

[PATCH] D81173: [clangd] Change ParseInputs to store FSProvider rather than VFS

kadircet updated this revision to Diff 269161. kadircet added a comment. - Rebase - Move GetFSProvider to a lambda inside scanPreamble with a FIXME. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81173/new/ https://reviews.llvm.org/D81173 Files:

[PATCH] D81173: [clangd] Change ParseInputs to store FSProvider rather than VFS

kadircet updated this revision to Diff 269168. kadircet marked 2 inline comments as done. kadircet added a comment. - Inline VFS uses in ClangdServer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81173/new/ https://reviews.llvm.org/D81173 Files:

[PATCH] D81173: [clangd] Change ParseInputs to store FSProvider rather than VFS

This revision was automatically updated to reflect the committed changes. Closed by commit rGf693ce4aa97e: [clangd] Change ParseInputs to store FSProvider rather than VFS (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81173/new

[PATCH] D80784: [clangd][NFC] Explode ReceivedPreamble into a CV

kadircet updated this revision to Diff 269487. kadircet marked 2 inline comments as done. kadircet added a comment. - Make distinction between PreambleCV and RequestsCV clearer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80784/new/ https://revie

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

kadircet created this revision. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, javed.absar, ilya-biryukov. Herald added a project: clang. kadircet added a reviewer: sammccall. This is an extension clangd provides in LSP layer and to embedders of ClangdServer. Curren

[PATCH] D80900: [clangd] Use different FS in PreambleThread

kadircet abandoned this revision. kadircet added a comment. we've went with D81173 instead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80900/new/ https://reviews.llvm.org/D80900 __

[PATCH] D80784: [clangd][NFC] Explode ReceivedPreamble into a CV

This revision was automatically updated to reflect the committed changes. Closed by commit rG406ac49fb05e: [clangd][NFC] Explode ReceivedPreamble into a CV (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80784/new/ https://revi

[PATCH] D81530: [clangd] Log rather than assert on bad UTF-8.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! (i bet you rushed it already for the cherry-pick, but just wanted to remind again that we should :D) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D81474: Handle delayed-template-parsing functions imported into a non-dtp TU

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. Looking at this some more, it looks like whenever clang is building a TU prefix(preamble), the assumption is that any delayed templates will be parsed at the end of the TU. That is unfortu

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

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:695 + const PrintingPolicy &Policy) { + if (!N) +return; nit: this is already checked before entering the func

[PATCH] D81691: [clangd] Set CWD in semaCodeComplete

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/D81691 Files: clang-tools-extra/clangd/CodeComplete.cpp Index: clang-too

[PATCH] D81691: [clangd] Set CWD in semaCodeComplete

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 rGf44d2424f8d7: [clangd] Set CWD in semaCodeComplete (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D81719: [clangd] Drop usage of PreambleStatCache in scanPreamble

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. It was used inside buildCompilerInvocation to speed up stats. But preambleStatCache doesn't contain sta

[PATCH] D81720: [clangd] Change prepareCompilerInstance to take an FSProvider

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 API easier to use by moving setcwd and fscache setup into it. Also ensures no side effects a

[PATCH] D81720: [clangd] Change prepareCompilerInstance to take an FSProvider

kadircet updated this revision to Diff 270333. kadircet added a comment. - Use Cmd.Directory when creating includefixer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81720/new/ https://reviews.llvm.org/D81720 Files: clang-tools-extra/clangd/Cod

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

kadircet added a comment. I am not the best person to review this, but dig a little bit into your issues and history of this file. Your diagnosis and fix seems reasonable, AFAICT the existing caching behavior was chosen to optimize files with many macro invocations that are expanding into relat

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

kadircet added a comment. Also forgot to say, thanks a lot for taking your time to investigate this issue and coming up with a patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80681/new/ https://reviews.llvm.org/D80681 _

[PATCH] D81719: [clangd] Drop usage of PreambleStatCache in scanPreamble

kadircet added a comment. Looked at this more in details, today we call `buildCompilerInvocation` in 3 different places: - Once in TUScheduler whenever we receive an update, later on it propagates into `buildPreamble` and `buildAST`. - Once during signatureHelp/codeCompletion. In theory that on

[PATCH] D81719: [clangd] Drop usage of PreambleStatCache in scanPreamble

kadircet added a comment. Moreover, the most time consuming(well this might be subject to ltrace not counting IO wait times, as an openat above was only 5microsecs/call and a tolower call is 56 microsecs/call :D) bit actually seems like the string comparisons/manipulations done while parsing th

[PATCH] D81920: [clangd] Change FSProvider::getFileSystem to take CurrentWorkingDirectory

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. We've faced a couple of problems when the returned FS didn't have the proper working directory. New sig

[PATCH] D81719: [clangd] Drop usage of PreambleStatCache in scanPreamble

kadircet updated this revision to Diff 271083. kadircet marked 2 inline comments as done. kadircet added a comment. - Drop VFS usage in scanPreamble completely to avoid IO. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81719/new/ https://reviews.ll

[PATCH] D81719: [clangd] Drop usage of PreambleStatCache in scanPreamble

kadircet added a comment. In D81719#2092589 , @sammccall wrote: > Thanks for all this investigation! > > > 80.710.002330 5 394 374 openat > > I'm curious what the 400 attempts and 20 successes are (I've seen this before > but d

[PATCH] D81964: [clangd] Make use of preamble bounds from the patch inside ReplayPreamble

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. Clangd was using bounds from the stale preamble, which might result in crashes. For example: #includ

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

kadircet added a comment. In D80681#2096886 , @nickdesaulniers wrote: > Heh, those were painstakingly written in markdown by hand, using > `CC="/usr/bin/time -v clang"` to test. The weren't statistically significant > (N=1), nor were they autogenerated

[PATCH] D81920: [clangd] Change FSProvider::getFileSystem to take CurrentWorkingDirectory

kadircet updated this revision to Diff 271312. kadircet marked 2 inline comments as done. kadircet added a comment. - Change signature to llvm::Optional to accomodate call sites that don't want to cd Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81

[PATCH] D81998: [clangd][NFC] Rename FSProvider and getFileSystem

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. Clangd uses FSProvider to get threadsafe views into file systems. This patch changes naming to make tha

[PATCH] D82002: [clangd] Drop FS usage in ClangTidyOpts

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 D81998 Repository: rG LLVM Github Monorepo https://re

[PATCH] D81998: [clangd][NFC] Rename FSProvider and getFileSystem

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

[PATCH] D81920: [clangd] Change FSProvider::getFileSystem to take CurrentWorkingDirectory

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:187 +Opts.ClangTidyOpts = +GetClangTidyOptions(*FSProvider.getFileSystem("."), File); Opts.SuggestMissingIncludes = SuggestMissingIncludes; sammccall wrote: > Not

[PATCH] D81920: [clangd] Change FSProvider::getFileSystem to take CurrentWorkingDirectory

kadircet updated this revision to Diff 271328. kadircet added a comment. - Provide two overloads to make implicit string -> StringRef conversion possible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81920/new/ https://reviews.llvm.org/D81920 Fi

[PATCH] D82002: [clangd] Drop FS usage in ClangTidyOpts

kadircet updated this revision to Diff 271342. kadircet added a comment. - Accept an inner opt provider instead to enable wrapping other types of providers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82002/new/ https://reviews.llvm.org/D82002

[PATCH] D81920: [clangd] Change FSProvider::getFileSystem to take CurrentWorkingDirectory

kadircet marked 9 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:555 // different code layout. if (auto CorrespondingFile = getCorrespondingHeaderOrSource( + std::string(Path), FSProvider.getFileSy

[PATCH] D82002: [clangd] Drop FS usage in ClangTidyOpts

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

[PATCH] D81998: [clangd][NFC] Rename FSProvider and getFileSystem

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

[PATCH] D81920: [clangd] Change FSProvider::getFileSystem to take CurrentWorkingDirectory

kadircet updated this revision to Diff 271354. 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/D81920/new/ https://reviews.llvm.org/D81920 Files: clang-tools-ex

[PATCH] D82011: [clangd] Don't mangle workdir-relevant driver path in compile commands

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/CompileCommands.cpp:141 +// Let's hope it's not a symlink. +if (llvm::any_of(Driver, + [](c

[PATCH] D82024: [clangd] Rename FSProvider to TFS in case of ThreadsafeFS

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. Depends on D81998 Repository: rG LLVM Github Monorep

[PATCH] D81998: [clangd][NFC] Rename FSProvider and getFileSystem

kadircet marked 6 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:722 /* Override */ OverrideClangTidyOptions, -FSProvider.getFileSystem(/*CWD=*/llvm::None)); +FSProvider.view(/*CWD=*/llvm:

[PATCH] D81964: [clangd] Make use of preamble bounds from the patch inside ReplayPreamble

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Preamble.h:136 std::vector PreambleIncludes; + PreambleBounds PatchBounds = {0, false}; }; sammccall wrote: > nit: = {} seems less confusing. > > Call this ModifiedBounds (or ModifiedPream

[PATCH] D81964: [clangd] Make use of preamble bounds from the patch inside ReplayPreamble

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

[PATCH] D81998: [clangd][NFC] Rename FSProvider and getFileSystem

kadircet updated this revision to Diff 271386. kadircet marked an inline comment as done. kadircet added a comment. Herald added subscribers: javed.absar, mgorny. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81998/new/ https://r

[PATCH] D81964: [clangd] Make use of preamble bounds from the patch inside ReplayPreamble

This revision was automatically updated to reflect the committed changes. Closed by commit rG4317ee27bd64: [clangd] Make use of preamble bounds from the patch inside ReplayPreamble (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

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

kadircet abandoned this revision. kadircet added a comment. abandoning in favor of D82944 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82729/new/ https://reviews.llvm.org/D82729

[PATCH] D82964: [clangd] Config: loading and caching config from disk.

kadircet added a comment. thanks looks really cool! Comment at: clang-tools-extra/clangd/ConfigProvider.cpp:39 +std::vector &Out) { +assert(llvm::sys::path::is_absolute(Path)); +auto FS = TFS.view(/*CWD=*/llvm::None); I believe this function

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

kadircet planned changes to this revision. kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:670 auto Version = DraftMgr.addDraft(File, Params.textDocument.version, Contents); - Server->addDocu

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

kadircet updated this revision to Diff 275069. kadircet marked an inline comment as done. kadircet added a comment. - Get rid of the fixtures Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82944/new/ https://reviews.llvm.org/D82944 Files: clang-t

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG37cc3ee8c555: [clangd] Switch FindSymbolsTests to use TestTU (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82944/new/ https://review

[PATCH] D83099: Revert "[clangd] Store index in '.clangd/index' instead of '.clangd-index'"

kadircet added inline comments. Comment at: .gitignore:56 .vs # clangd index +.clangd-index/ the comment above says `do not add trailing /`, we've got no choice for `.clangd/` but maybe drop that for `.clangd-index` ? Repository: rG LLVM Github Monorepo C

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG5c46fefdba3b: [clangd] Improve hover on arguments to function call (authored by adamcz, committed by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D82964: [clangd] Config: loading and caching config from disk.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ConfigProvider.cpp:39 +auto Buf = FS.getBufferForFile(Path); +// If stat() succeeds but we failed to read, retry once and cache failure. +if (!Buf) why do we want to cache failure ca

[PATCH] D83095: [clangd] Config: compute config in TUScheduler and BackgroundIndex

kadircet added a comment. LG, mostly nits apart from a question about moving the context generation logic into a different place. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:748 +Context ClangdServer::createProcessingContext(PathRef File) const { + if (!ConfigProv

[PATCH] D83143: [clangd] Fix hover crash on invalid decls

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 also changes the way we display Size and Offset to be independent. Repository: rG LLVM Github

[PATCH] D83095: [clangd] Config: compute config in TUScheduler and BackgroundIndex

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/D83095/new/ https://reviews.llvm.org/D83095 __

[PATCH] D82964: [clangd] Config: loading and caching config from disk.

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/D82964/new/ https://reviews.llvm.org/D82964 ___

[PATCH] D83095: [clangd] Config: compute config in TUScheduler and BackgroundIndex

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:748 +Context ClangdServer::createProcessingContext(PathRef File) const { + if (!ConfigProvider) sammccall wrote: > kadircet wrote: > > why not make this a free function in `C

[PATCH] D83143: [clangd] Fix hover crash on invalid decls

kadircet updated this revision to Diff 275448. kadircet added a comment. Inline OffsetBits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83143/new/ https://reviews.llvm.org/D83143 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-extra/cla

[PATCH] D83143: [clangd] Fix hover crash on invalid decls

This revision was automatically updated to reflect the committed changes. Closed by commit rG50ba9f994c6f: [clangd] Fix hover crash on invalid decls (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83143/new/ https://reviews.llv

[PATCH] D83157: [clangd] Extract BackgroundIndex::Options struct. NFC

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 for doing this! Comment at: clang-tools-extra/clangd/ClangdServer.cpp:184 +TFS, CDB, BackgroundIndexSto

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

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. Some clang-tidy checkers, e.g. llvm-include-order can emit diagnostics at this callback (as mentioned i

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

kadircet added a comment. In D83178#2131914 , @sammccall wrote: > > can emit diagnostics at this callback (as mentioned in the comments). > > I'm a bit puzzled about this. The case that the code handles and the comment > refers to is if a check installs P

[PATCH] D83189: [clangd] More complete fix for hover crashes on invalid record.

kadircet added a comment. thanks for doing this ! Comment at: clang-tools-extra/clangd/Hover.cpp:665 if (auto *RD = llvm::dyn_cast(&ND)) { if (auto Size = Ctx.getTypeSizeInCharsIfKnown(RD->getTypeForDecl())) HI.Size = Size->getQuantity(); i think

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

kadircet updated this revision to Diff 275627. kadircet added a comment. - Add unittest to DiagnosticsTest via llvm-include-order Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83178/new/ https://reviews.llvm.org/D83178 Files: clang-tools-extra/c

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

kadircet added a comment. Yeah concerns around header-guard checks are real (and likely to be annoying), so will wait for D78038 . I suppose we can split that into multiple patches, and at least land the bit around preserving conditional stack at the end of prea

[PATCH] D83157: [clangd] Extract BackgroundIndex::Options struct. NFC

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:184 +TFS, CDB, BackgroundIndexStorage::createDiskBackedStorageFactory( [&CDB](llvm::StringRef File) { return CDB.getProje

[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, aaron.ballman, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. This enables sharing the logic between standalone clangd and embedders of it. The new ap

[PATCH] D83189: [clangd] More complete fix for hover crashes on invalid record.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:677 HI.Size = Size->getQuantity(); - if (!FD->isInvalidDecl()) HI.Offset = Ctx.getFieldOffset(FD) / 8; + } hokein wrote: > kadircet wrote: > > could you mov

[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

kadircet updated this revision to Diff 275709. kadircet added a comment. - Add tests and also disable bugprone-use-after-move Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83224/new/ https://reviews.llvm.org/D83224 Files: clang-tools-extra/clang

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

kadircet added a comment. As discussed offline D78038 is not going to fix header-guard-check issue, as it relies on not only the PP state, but receiving appropriate callbacks for all of the ifndef/define/endif macros, which is something we don't do in current s

[PATCH] D83233: [clangd] Enable reading config from files by default.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:700 +ProviderStack.push_back( +config::Provider::fromAncestorRelativeYAMLFiles(".clangd", TFS)); +llvm::SmallString<256> UserConfig; should we first update th

[PATCH] D83233: [clangd] Enable reading config from files by default.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LG from my side Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83233/new/ https://reviews.llvm.org/D83233

[PATCH] D83290: [clangd] Enable async preambles by default

kadircet created this revision. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, javed.absar, ilya-biryukov. Herald added a project: clang. We've been testing this internally for a couple weeks now and it seems to be stable enough. Let's flip the flag before branch cu

[PATCH] D83099: [clangd] Store index in '.cache/clangd/index' instead of '.clangd/index'

kadircet added inline comments. Comment at: .gitignore:57 +# clangd index. (".clangd" is a config file now, thus trailing slash) +.clangd/ +.cache why do we still need this ? i thought index (and other caches) would reside in `.cache` ? Commen

[PATCH] D83290: [clangd] Enable async preambles by default

This revision was automatically updated to reflect the committed changes. Closed by commit rG1c7c5019a7ad: [clangd] Enable async preambles by default (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83290/new/ https://reviews.ll

[PATCH] D83536: [clangd] Index refs to main-file symbols as well

kadircet added a comment. Thanks a lot for doing this Nathan and sorry for not replying with a concrete decision on the issue tracker, we were still discussing it internally, as 7% increase for allowing multi-level call hierarchy support (compared to ~10% initial cost for single-level support)

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

kadircet added a comment. thanks for doing this! but i believe these look like bugs that should be addressed in other components, rather than worked around in hover. the first example is likely a bug in selection tree, file location corresponds to a macro body that's never expanded, which resid

[PATCH] D83546: [clangd] Fix hover crash on InitListExpr.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:345 +if (!ILE->isSemanticForm()) + E = ILE->getSemanticForm(); + } shouldn't we put this before `QualType T = E->getType();` ? Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D83548: [clangd] Fix tests build for GCC5

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/D83548/new/ https://reviews.llvm.org/D83548 __

[PATCH] D83546: [clangd] Fix hover crash on InitListExpr.

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/D83546/new/ https://reviews.llvm.org/D83546 __

[PATCH] D83548: [clangd] Fix tests build for GCC5

This revision was automatically updated to reflect the committed changes. Closed by commit rG07c4c7e7959b: [clangd] Fix tests build for GCC5 (authored by ArcsinX, committed by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83548/new/ https

[PATCH] D83536: [clangd] Index refs to main-file symbols as well

kadircet added a comment. In D83536#2144282 , @nridge wrote: > I can do that. > > Another thing we could consider, if the space increase is a concern, is to > limit which references we store, e.g. to functions only (not variables or > classes). Yes, th

[PATCH] D83755: [clangd] Cache config files for 5 seconds, without revalidating with stat.

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/ConfigProvider.cpp:24 +// + looks like an unwated artifact Comment at: clang-tools

[PATCH] D83768: [clangd] Config: Index.Background

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:98 + ValidValues.push_back(Name); + if (!Result && *Input == Name) +Result = Value; should we assert on multiple matches of the same name ?

[PATCH] D83768: [clangd] Config: Index.Background

kadircet accepted this revision. kadircet marked an inline comment as done. kadircet added a comment. This revision is now accepted and ready to land. let's ship it! Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:196 constexpr static llvm::SourceMgr::DiagKind Error =

[PATCH] D83802: [clangd] Config: also propagate in sync (testing) mode

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/TUScheduler.cpp:1241 + // Avoid null checks everywhere. + if (!Opts.ContextProvider) +this->Opts.ContextProvider

[PATCH] D83822: [clangd] Support config over LSP.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1220 CDB->setCompileCommand(File, std::move(New)); ModifiedFiles.insert(File); } nit: maybe just set `ReparseAllFiles` in here too, and change the conditio

[PATCH] D83855: [clang] Fix printing of lambdas with capture expressions

kadircet added inline comments. Comment at: clang/lib/AST/StmtPrinter.cpp:2011 + Expr *Init = D->getInit(); + if (D->getInitStyle() == VarDecl::CallInit && !isa(Init)) +OS << "("; what about having a `Pre` and `Post` print blocks, set to `"(" an

[PATCH] D83855: [clang] Fix printing of lambdas with capture expressions

kadircet added inline comments. Comment at: clang/lib/AST/StmtPrinter.cpp:2011 + Expr *Init = D->getInit(); + if (D->getInitStyle() == VarDecl::CallInit && !isa(Init)) +OS << "("; walrus wrote: > kadircet wrote: > > what about having a `Pre` and

[PATCH] D81998: [clangd][NFC] Rename FSProvider and getFileSystem

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

[PATCH] D81998: [clangd][NFC] Rename FSProvider and getFileSystem

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/TestFS.h:36 public: - // Prevent name hiding caused by the overload below. - using FileSystemProvider::getFileSystem; - IntrusiveRefCntPtr getFileSystem() const { return buildTestFS(Files, T

[PATCH] D82024: [clangd] Rename FSProvider to TFS in case of ThreadsafeFS

kadircet updated this revision to Diff 271985. 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/D82024/new/ https://reviews.llvm.org/D82024 Files: clang-tools-ex

[PATCH] D82024: [clangd] Rename FSProvider to TFS in case of ThreadsafeFS

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

[PATCH] D81719: [clangd] Drop usage of PreambleStatCache in scanPreamble

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

[PATCH] D81920: [clangd] Change FSProvider::getFileSystem to take CurrentWorkingDirectory

This revision was automatically updated to reflect the committed changes. Closed by commit rG2dc2e47e3cb7: [clangd] Change FSProvider::getFileSystem to take CurrentWorkingDirectory (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D81998: [clangd][NFC] Rename FSProvider and getFileSystem

This revision was automatically updated to reflect the committed changes. Closed by commit rG0628705efaf7: [clangd][NFC] Rename FSProvider and getFileSystem (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81998/new/ https://rev

[PATCH] D82024: [clangd] Rename FSProvider to TFS in case of ThreadsafeFS

This revision was automatically updated to reflect the committed changes. Closed by commit rG8d654df5b982: [clangd] Rename FSProvider to TFS in case of ThreadsafeFS (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D82024?vs=271988&id=271996#toc Repository: rG LLVM Gi

[PATCH] D81719: [clangd] Drop usage of PreambleStatCache in scanPreamble

This revision was automatically updated to reflect the committed changes. Closed by commit rGd2fcc586223c: [clangd] Drop usage of PreambleStatCache in scanPreamble (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81719/new/ http

[PATCH] D82011: [clangd] Don't mangle workdir-relevant driver path in compile commands

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CompileCommands.cpp:141 +// Let's hope it's not a symlink. +if (llvm::any_of(Driver, + [](char C) { return llvm::sys::path::is_separator(C); })) sammccall wrote: > ka

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

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 is both confusing and crashy. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D8

[PATCH] D81920: [clangd] Change FSProvider::getFileSystem to take CurrentWorkingDirectory

kadircet added a comment. thanks for the info @uabelho! this looks like a dormant warning though, as StringRef is not implicitly convertible to NoneType (and vice-versa) hence anyone trying to make use of the hidden overload would get a hard compile error anyways. Moreover this class is mostly

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