[PATCH] D133339: [clangd] Isolate logic for setting LSPServer options

kadircet updated this revision to Diff 458769. kadircet added a comment. I agree this is creating confusing state for constructors of ClangdLSPServer. It would be interesting to create LSPServer after the initialize request one day. - Make client-capability related options private again.

[PATCH] D133440: [clangd] Allow programmatically disabling rename of virtual method hierarchies.

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/refactor/Rename.cpp:217 return ReasonToReject::NonIndexable; + if (!Opts.RenameVirtual) { +if (const auto *S

[PATCH] D130265: [clangd] resolve forwarded parameters in Hover

kadircet added a comment. This looks like a nice idea, but i think we definitely need annotations to prevent confusion. I'd prefer something like: assume `foo(int x, Args... args);` function foo -> void Parameters: - int x - int a : forwarded to [bar:a](it'd be great if we could have

[PATCH] D133479: [clangd] Set Incompleteness for spec fuzzyfind requests

kadircet created this revision. kadircet added reviewers: sammccall, dgoldman. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra.

[PATCH] D132962: [clangd][ObjC] Improve completions for protocols + category names

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1731 SymbolSlab::Builder ResultsBuilder; -if (Opts.Index->fuzzyFind(Req, [&](const Symbol ) { -

[PATCH] D133423: [clangd] Improve Selection testcase, pin to C++17

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. can you wait for premerge checks to finish (especially for windows)? Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:729-730

[PATCH] D132918: [clang] Fix a crash in constant evaluation

kadircet added a comment. > This is helpful information but I am not sure this convinces me that > EvaluateVarDecl is the correct place to check. Why not in EvaluateDecl or > EvaluateStmt? Both locations we could also check. I have done the check inside `EvaluateVarDecl` because the invalid

[PATCH] D133339: [clangd] Isolate logic for setting LSPServer options

kadircet updated this revision to Diff 458128. kadircet added a comment. rename helper & add comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D19/new/ https://reviews.llvm.org/D19 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp

[PATCH] D130863: [clangd] Make git ignore index directories

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp:58 +if (CreatingNewDirectory) { + llvm::Error error = addGitignore(DiskShardRoot); + if (error) { nit: ``` if(llvm::Error E = addGitignore(..)) {

[PATCH] D133339: [clangd] Isolate logic for setting LSPServer options

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Moves

[PATCH] D130863: [clangd] Make git ignore index directories

kadircet added a comment. In D130863#3696419 , @sums wrote: > Thank you for the reviews and discussion, everyone! > > TIL about `.git/info/exclude`. It'll let me exclude stuff without updating > the checked in `.gitignore`. I was achieving the same

[PATCH] D132830: [clangd] Avoid crash when printing call to string literal operator template in hover

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

[PATCH] D132918: [clang] Fix a crash in constant evaluation

kadircet updated this revision to Diff 457566. kadircet added a comment. Add reproducer. I think the issue is about keeping constexpr functions valid even when their bodies contain invalid decls under certain instantiations, which I believe is the right behaviour. As the function body might be

[PATCH] D132962: [clangd][ObjC] Improve completions for protocols + category names

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:3228 + + Results = completions(R"objc( + Fo^ nit: just `completions("Fo^", ...)` Comment at:

[PATCH] D132962: [clangd][ObjC] Improve completions for protocols + category names

kadircet added a comment. can you also add test cases for the other two (filtering both for speculative index queries/regular ones, and making sure we don't suggest symbols from index for category names), so that we don't regress in the future? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D132918: [clang] Fix a crash in constant evaluation

kadircet added a comment. sorry, i missed the other comments around having a lit test. reverting the change. Comment at: clang/lib/AST/ExprConstant.cpp:4797 } +// Can't access properties of an incomplete type. +if (!RD->hasDefinition()) { shafik

[PATCH] D132918: [clang] Fix a crash in constant evaluation

This revision was automatically updated to reflect the committed changes. Closed by commit rGa5ab650714d0: [clang] Fix a crash in constant evaluation (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132918/new/

[PATCH] D132919: [clangd] Enable folding ranges by default.

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

[PATCH] D132919: [clangd] Enable folding ranges by default.

kadircet added a comment. In D132919#3757685 , @Trass3r wrote: > Whenever I tried this option in the past it crashed clangd. > I recommend doing some more testing before flipping the switch. we've a new implementation of folding ranges based on pseudo

[PATCH] D132919: [clangd] Enable folding ranges by default.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:340 desc("Enable preview of FoldingRanges feature"), -init(false), +init(true), Hidden, i think we should just retire the flag altogether, ATM this is only

[PATCH] D132918: [clang] Fix a crash in constant evaluation

kadircet created this revision. kadircet added reviewers: usaxena95, ilya-biryukov. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This was showing up in our internal crash collector. I have no idea

[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

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

[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:254 +// Remove the ending sentinal "*/" from the block comment range. +if (Code.substr(EndOffset(*LastComment) - 2, 2) == "*/") { + End.character -= 2; this

[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:202 FR.endLine = End.line; +FR.startCharacter = Start.character; FR.endCharacter = End.character; can you put it back to its previous location (i.e. right

[PATCH] D132830: [clangd] Avoid crash when printing call to string literal operator template in hover

kadircet added a comment. can you move the test into `llvm/llvm-project/clang/unittests/AST/StmtPrinterTest.cpp` ? while there, also a test on pack-extended user defined literals would be nice. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D132110: [IncludeCleaner] Handle more C++ constructs

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:55 + bool VisitMemberExpr(MemberExpr *E) { +auto *MD = E->getMemberDecl(); +report(E->getMemberLoc(), MD); sammccall wrote: > sammccall wrote: > > nit: inline?

[PATCH] D132110: [IncludeCleaner] Handle more C++ constructs

kadircet updated this revision to Diff 453638. kadircet marked 5 inline comments as done. kadircet added a comment. - Adress comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132110/new/ https://reviews.llvm.org/D132110 Files:

[PATCH] D132110: [IncludeCleaner] Handle more C++ constructs

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang-tools-extra. This brings IncludeCleaner's reference discovery from AST

[PATCH] D131088: [clang] Apply FixIts to members declared via `using` in derived classes

This revision was automatically updated to reflect the committed changes. Closed by commit rG8837ef4d373b: [clang] Apply FixIts to members declared via `using` in derived classes (authored by denis-fatkulin, committed by kadircet). Changed prior to commit:

[PATCH] D130363: [clang] Give priority to Class context while parsing declarations

This revision was automatically updated to reflect the committed changes. Closed by commit rG4dd71b3cb947: [clang] Give priority to Class context while parsing declarations (authored by furkanusta, committed by kadircet). Changed prior to commit:

[PATCH] D131385: [clangd] Support for standard type hierarchy

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rG83411bf06f34: [clangd] Support for standard type hierarchy (authored by kadircet). Repository: rG LLVM

[PATCH] D131385: [clangd] Support for standard type hierarchy

kadircet marked 2 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:1713 + fillSuperTypes(*ParentDecl, TUPath, *ParentSym, RPSet); + Item.data.parents->emplace_back(ParentSym->data); +

[PATCH] D131385: [clangd] Support for standard type hierarchy

kadircet updated this revision to Diff 452927. 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/D131385/new/ https://reviews.llvm.org/D131385 Files:

[PATCH] D131385: [clangd] Support for standard type hierarchy

kadircet marked 3 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/Protocol.cpp:1228 +Result["parents"] = RP.parents; + return std::move(Result); +} usaxena95 wrote: > Nit: Allow RVO. there's an issue with one of

[PATCH] D131696: [clangd] Fix an inlay-hint crash on a broken designator.

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/InlayHints.cpp:144 }); -if (llvm::isa(Init)) +if (!Init || llvm::isa(Init)) continue; // a "hole" for a

[PATCH] D131724: [pseudo] Eliminate an ambiguity for the empty member declaration.

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

[PATCH] D131696: [clangd] Fix an inlay-hint crash on a broken designator.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:140 for (const Expr *Init : Sem->inits()) { +if (!Init) + continue; we should have this bail out after introducing the scope_exit below to make sure we skip the

[PATCH] D130363: [clang] Give priority to Class context while parsing declarations

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! let me know if i should land this for you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130363/new/

[PATCH] D131569: [clangd] Allow updates to be canceled after compile flags retrieval

kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: usaxena95, arphaman, javed.absar. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang-tools-extra.

[PATCH] D130015: [clangd] Add "usedAsMutablePointer" highlighting modifier

kadircet added a comment. I agree with Nathan to some extent that this doesn't carry as much weight, but if we can get it without introducing much complexity to the user (i.e. a new modifier they need to remember), it's worth having since we don't need too much effort to support it.

[PATCH] D131088: [clang] Apply FixIts to members declared via `using` in derived classes

kadircet added a comment. looks like you've uploaded the diff without context, can you upload it again with full context? also the changes to `MaybeAddResult` seem to be missing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131088/new/ https://reviews.llvm.org/D131088

[PATCH] D131088: [clang] Apply FixIts to members declared via `using` in derived classes

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/CodeCompleteTests.cpp:2187 +TEST(CompletionTest, FixItForMembersUsing) { + const Annotations Code(R"cpp(

[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:225 + Position End) { +// For LineFoldingsOnly clients, do not fold the last line if it +// contains tokens after `End`.

[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

kadircet added a comment. thanks, i think we should change the behaviour to not fold the last line in any case. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:943 Callback> Reply) { - Server->foldingRanges(Params.textDocument.uri.file(), std::move(Reply)); +

[PATCH] D131385: [clangd] Support for standard type hierarchy

kadircet created this revision. kadircet added a reviewer: usaxena95. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. This is

[PATCH] D129973: [clang] Pass FoundDecl to DeclRefExpr creator for operator overloads

kadircet added a comment. Reverted in df48e3fbcc8be1f4c04bd97517d12e662f54de75 , @tstellar it needs to be cherry-picked into release branch as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D129973: [clang] Pass FoundDecl to DeclRefExpr creator for operator overloads

kadircet added a comment. Reverting for @bgraur as he's having some issues with his commit access ATM. To sum up the reproducer attached by @joanahalili triggers a crash https://reviews.llvm.org/rG4e94f6653150511de434fa7e29b684ae7f0e52b6 with stack trace and error: $

[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

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

[PATCH] D130041: [clangd] Add decl/def support to SymbolDetails

kadircet added a comment. no worries, as discussed offline this LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130041/new/ https://reviews.llvm.org/D130041 ___ cfe-commits mailing list

[PATCH] D130826: [clang-tools-extra] Fixed a number of typos

This revision was automatically updated to reflect the committed changes. Closed by commit rG0ed2bd9311fd: [clang-tools-extra] Fixed a number of typos (authored by GabrielRavier, committed by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D130826: [clang-tools-extra] Fixed a number of typos

kadircet added a comment. also LMK if I should land this for you Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130826/new/ https://reviews.llvm.org/D130826 ___ cfe-commits mailing list

[PATCH] D130826: [clang-tools-extra] Fixed a number of typos

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

[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

kadircet added a comment. I agree with Sam on this one, especially on big files it just floods and hides non-stylistic diagnostics (you even hit max diagnostics exceeded errors). I believe unless someone takes the extra mile to fix all the value types in LLVM to be const-correct, this is

[PATCH] D130363: [clang] Give priority to Class context while parsing declarations

kadircet added a comment. > Is my invocation correct or is this case handled by clangd and not clang? yeah your invocation is correct. this is because clang's printer does a prefix based filtering (while clangd does a fuzzy match)

[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

kadircet added a comment. sorry this has slipped through. since we're going to backport it, let's make it minimal and always point to in-progress release notes (so sorry for reverting back to original version of the patch). i think having clangd point to versioned docs would be much nicer, but

[PATCH] D130863: [clangd] Make git ignore index directories

kadircet added a comment. I am not sure if clangd is the right tool the create those `.gitignore` files, e.g. what if the project's VCS isn't git? I believe the right thing to do is for projects that want to make use of such tools to ignore the relevant directory explicitly (e.g. `.cache).

[PATCH] D130363: [clang] Give priority to Class context while parsing declarations

kadircet added a comment. Can you also add a test, preferably in clang/test/CodeCompletion/overrides.cpp ? Comment at: clang/lib/Parse/ParseDecl.cpp:3269 - if (getCurScope()->getFnParent() || getCurScope()->getBlockParent()) -CCC =

[PATCH] D130690: [clangd][NFCI] Store TUPath inside ParsedAST

This revision was automatically updated to reflect the committed changes. kadircet marked 2 inline comments as done. Closed by commit rG3b8fb471cbbd: [clangd][NFCI] Store TUPath inside ParsedAST (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D130690: [clangd][NFCI] Store TUPath inside ParsedAST

kadircet marked 2 inline comments as done. kadircet added a comment. as discussed offline, this patch stores the filepath provided by the client (e.g. vscode) which is usually the filepath seen by the user, inside the ParsedAST and later on uses that when a hint is needed for translating

[PATCH] D130636: [clangd] Upgrade vlog() to log() for preamble build stats

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/Preamble.cpp:555 if (BuiltPreamble) { -vlog("Built preamble of size {0} for file {1} version {2} in {3} seconds", +

[PATCH] D130041: [clangd] Add decl/def support to SymbolDetails

kadircet added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:1528 } - + auto MainFilePath = + getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM); dgoldman wrote: > dgoldman wrote: > > sammccall wrote: > > > kadircet wrote:

[PATCH] D130690: [clangd][NFCI] Store TUPath inside ParsedAST

kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: usaxena95, arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang-tools-extra. Lots of

[PATCH] D130081: Add foldings for multi-line comment.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Protocol.h:1782 unsigned endCharacter; - std::string kind; + FoldingRangeKind kind; }; hokein wrote: > sorry for not being clear on my previous comment, I think the current `string >

[PATCH] D130095: [clangd] Improve XRefs support for ObjCMethodDecl

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/XRefs.cpp:89 + return MD; +auto *DeclCtx = cast(MD->getDeclContext()); +if (DeclCtx->isInvalidDecl())

[PATCH] D130260: [clangd] Make forwarding parameter detection logic resilient

This revision was automatically updated to reflect the committed changes. Closed by commit rG4839929bed6e: [clangd] Make forwarding parameter detection logic resilient (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D130260?vs=446796=446805#toc Repository: rG LLVM

[PATCH] D130260: [clangd] Make forwarding parameter detection logic resilient

kadircet added a comment. In D130260#3671290 , @sammccall wrote: > driveby thoughts, but please don't block on them. > > (if this fix is a heuristic that fixes a common crash but isn't completely > correct, that may still be worth landing but warrants a

[PATCH] D130260: [clangd] Make forwarding parameter detection logic resilient

kadircet updated this revision to Diff 446796. 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/D130260/new/ https://reviews.llvm.org/D130260 Files:

[PATCH] D130260: [clangd] Make forwarding parameter detection logic resilient

kadircet updated this revision to Diff 446770. kadircet added a comment. - Update test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130260/new/ https://reviews.llvm.org/D130260 Files: clang-tools-extra/clangd/AST.cpp

[PATCH] D130260: [clangd] Make forwarding parameter detection logic resilient

kadircet updated this revision to Diff 446769. kadircet marked an inline comment as done. kadircet added a comment. - Rather than asserting limit the traversal - Have more comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130260/new/

[PATCH] D130260: [clangd] Make forwarding parameter detection logic resilient

kadircet added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:790 +// Skip functions with less parameters, they can't be the target. +if (Callee->parameters().size() < Parameters.size()) + return; upsj wrote: > This is not a

[PATCH] D130260: [clangd] Make forwarding parameter detection logic resilient

kadircet updated this revision to Diff 446763. kadircet added a comment. - Add OOB check as an asssertion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130260/new/ https://reviews.llvm.org/D130260 Files: clang-tools-extra/clangd/AST.cpp

[PATCH] D130260: [clangd] Make forwarding parameter detection logic resilient

kadircet updated this revision to Diff 446759. kadircet added a comment. - Check to ensure function call receives all the arguments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130260/new/ https://reviews.llvm.org/D130260 Files:

[PATCH] D130259: [clangd] fix crash and handle implicit conversions in inlay hints for forwarding functions

kadircet added a comment. In D130259#3671022 , @nridge wrote: > Which bug/issue is this? https://github.com/llvm/llvm-project/issues/56620 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130259/new/

[PATCH] D130259: [clangd] fix crash and handle implicit conversions in inlay hints for forwarding functions

kadircet added a comment. sorry for not mentioning in the bug that i was also working on a patch, D130260 seems to be a little bit more contained and focused on making the check not crash (as we're getting close to release cut). WDYT about landing it now,

[PATCH] D130260: [clangd] Make forwarding parameter detection logic resilient

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

[PATCH] D130261: [clangd] Refactor forwarding call detection logic

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb5871dfaf318: [clangd] Refactor forwarding call detection logic (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D130261: [clangd] Refactor forwarding call detection logic

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:1446 +void bax(Args... args) { foo({args...}, args...); } + +void foo() { ilya-biryukov wrote: > NIT: maybe test for the case with a single expansion here:

[PATCH] D130261: [clangd] Refactor forwarding call detection logic

kadircet updated this revision to Diff 446529. 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/D130261/new/ https://reviews.llvm.org/D130261 Files:

[PATCH] D130228: [clangd] Mention whether compile flags were inferred in check mode

This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rG1515490c80fa: [clangd] Mention whether compile flags were inferred in check mode (authored by kadircet). Changed prior to commit:

[PATCH] D130261: [clangd] Refactor forwarding call detection logic

kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: usaxena95, arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang-tools-extra. Repository:

[PATCH] D130260: [clangd] Make forwarding parameter detection logic resilient

kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: usaxena95, arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang-tools-extra. This could

[PATCH] D130228: [clangd] Mention whether compile flags were inferred in check mode

kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: usaxena95, arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang-tools-extra. That way

[PATCH] D130041: [clangd] Add decl/def support to SymbolDetails

kadircet added a comment. thanks, this LG per symboldetails change but as discussed offline i think we actually need a new request or a capability to indicate this (and having a new request is just easier for both clients and clangd) otherwise it can't be reliably used by clients. in addition

[PATCH] D130011: Use pseudoparser-based folding ranges in ClangdServer.

kadircet added a comment. as discussed offline i don't see much value in having an extra flag to choose between ast-based and pseudo-based implementation, as the pseudo-based one is a super-set of the ast-based implementation. hence it shouldn't be regressing change in any way, therefore i am

[PATCH] D128621: [clangd] Do not try to use $0 as a placeholder in completion snippets

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, LG, just some extra precautions. Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:195 ++SnippetArg; - *Snippet += - "${" + -

[PATCH] D130003: [clangd] Use empty string to represent None semantics in FoldingRange::kind

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0496cf2f6a2e: [clangd] Use empty string to represent None semantics in FoldingRange::kind (authored by kadircet). Repository: rG LLVM Github

[PATCH] D130003: [clangd] Use empty string to represent None semantics in FoldingRange::kind

kadircet created this revision. kadircet added a reviewer: usaxena95. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository:

[PATCH] D129642: [Sema] Tweak diagnostic logic so suppress-in-hedaer logic works in tools too.

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaDecl.cpp:1784 if (D->isInvalidDecl() || D->isUsed() || D->hasAttr()) return false; sammccall wrote: >

[PATCH] D129642: [Sema] Tweak diagnostic logic so suppress-in-hedaer logic works in tools too.

kadircet added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:1784 if (D->isInvalidDecl() || D->isUsed() || D->hasAttr()) return false; i think we can just bail out early here when the main file is a header file (i.e. lang opts have

[PATCH] D129579: [clangd] Remove `allCommitCharacters`

kadircet added a comment. sorry for being late to the party, but reading the spec it sounds like vscode behaviour is actually the

[PATCH] D124447: [clang-tidy] Add infrastructure support for running on project-level information

kadircet added a comment. The idea looks great in general, I didn't get a chance to look at all the details but this also creates some concerns for integrations of clang-tidy checks in other environments (like clangd or tidy running in distributed systems) as the workflow actually needs to be

[PATCH] D129100: [clangd] Support external throttler for preamble builds

kadircet added inline comments. Comment at: clang-tools-extra/clangd/TUScheduler.cpp:499 // Build the preamble and let the waiters know about it. build(std::move(*CurrentReq)); } regarding the flakiness, what about resetting the

[PATCH] D128204: [clangd] Add fix-it for inserting IWYU pragma: keep

kadircet added a comment. > A specific example i encountered is clang/Tooling/DiagnosticsYaml.h Which > defines template specializations for inputting/outputting yaml io. That file > must be included if you ever want to emit diagnostics as YAML, but the > typical use case is to just use the

[PATCH] D129100: [clangd] Support external throttler for preamble builds

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/ClangdServer.h:108 +/// This throttler controls which preambles may be built at a given time. +

[PATCH] D129100: [clangd] Support external throttler for preamble builds

kadircet added a comment. we've already discussed most of these so no action needed, but here are some of them before they become completely irrelevant :P Comment at: clang-tools-extra/clangd/TUScheduler.cpp:457 +// Give up on this request, releasing resources if any. +

[PATCH] D127856: [clangd] Support multiline semantic tokens

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG333620d37a26: [clangd] Support multiline semantic tokens (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D128621: [clangd] Do not try to use $0 as a placeholder in completion snippets

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:197 - "${" + - std::to_string(SnippetArg == CursorSnippetArg ? 0 : SnippetArg) + ':'; appendEscapeSnippet(Chunk.Text, Snippet); as you've

[PATCH] D122677: [prototype] include-cleaner library

kadircet added a comment. hi Serge, sorry for the silence here. we'll be moving this towards a production ready version over the next months (also releasing some comments i've forgotten to send, we've already discussed these offline so no need for action). we'll probably be leaving some

[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

kadircet added a comment. luckily this document links were introduced only recently, hence they didn't make it to clangd-14, but they'll start being around from clangd-15 and such changes will be breaking all the links in existing clangd's. i wonder if we should actually point these at

[PATCH] D128204: [clangd] Add fix-it for inserting IWYU pragma: keep

kadircet added a comment. > I had another idea about offering the export pragma when in a header file but > I don't know if that's going too far There are definitely cases where this is conceptually "right" fix, but it's hard to guess that in clangd (well at least without having some codebase

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