[PATCH] D121103: [clang] Adjust LookupTest for UsingTypeLocs

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd65952b9bd80: [clang] Adjust LookupTest for UsingTypeLocs (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D119040: Fix LookupTest where it was missing an assertion

kadircet added a comment. Thanks for the change, but the test is actually checking for rename in presence of using-decls. I beleive https://reviews.llvm.org/D121103 is the proper fix here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119040/new/ https://reviews.llvm.org/D119040

[PATCH] D121103: [clang] Adjust LookupTest for UsingTypeLocs

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We no longer traverse the underlying RecordTypeLoc directly, but rather visit the

[PATCH] D120569: [clang] Improve laziness of resolving module map headers.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. Thanks, this LGTM. but I am afraid all users of modules/modulemaps might not be aware of the fact that a module might be "partially" resolved (or still unresolved after a resolve

[PATCH] D120375: Trim unnecessary component/library dependencies.

kadircet added a comment. Mostly a review of the clangd changes, I am not familiar with all the other parts (and not necessarily everyone will be) hence some explicit testing results would be great. Also please upload the patch with full context. Comment at:

[PATCH] D120115: [clangd] Tweak --query-driver to ignore slash direction on windows

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/QueryDriverDatabase.cpp:278 +} else if (llvm::sys::path::is_separator(Glob[I]) && +

[PATCH] D120081: [clang][ASTReader] Fix memory leak while reading FriendTemplateDecls

This revision was automatically updated to reflect the committed changes. Closed by commit rG977b1f574fa1: [clang][ASTReader] Fix memory leak while reading FriendTemplateDecls (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D120081: [clang][ASTReader] Fix memory leak while reading FriendTemplateDecls

kadircet created this revision. kadircet added a reviewer: sammccall. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Allocate on ASTContext, rather than just on heap, so that template parameter lists are freed up. Repository:

[PATCH] D120065: [clang][SemaTemplate] Fix a stack use after scope

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc79c13cae615: [clang][SemaTemplate] Fix a stack use after scope (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D120065: [clang][SemaTemplate] Fix a stack use after scope

kadircet added inline comments. Comment at: clang/include/clang/AST/DeclTemplate.h:2464 FriendTemplateDecl(DeclContext *DC, SourceLocation Loc, - MutableArrayRef Params, + TemplateParameterList **Params, unsigned NumParams,

[PATCH] D120065: [clang][SemaTemplate] Fix a stack use after scope

kadircet updated this revision to Diff 409752. kadircet marked 2 inline comments as done. kadircet added a comment. - Inline `Params.size()` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120065/new/ https://reviews.llvm.org/D120065 Files:

[PATCH] D120065: [clang][SemaTemplate] Fix a stack use after scope

kadircet created this revision. kadircet added a reviewer: sammccall. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D120065 Files:

[PATCH] D119989: [clangd] Fix building SerializationTests unit test on OpenBSD

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks Comment at: clang-tools-extra/clangd/unittests/SerializationTests.cpp:313 // Sanitizers use a lot of address space, so we can't apply strict limits. -#if

[PATCH] D118976: clangd: Set a diagnostic on a code action resulting from a tweak

This revision was automatically updated to reflect the committed changes. Closed by commit rGbd6c6974f5ea: clangd: Set a diagnostic on a code action resulting from a tweak (authored by ckandeler, committed by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D119130: [clangd] NFC: Move stdlibg headers handling to Clang

kadircet added a comment. Regarding the include mapping generator, I think it would've been better if we had some sort of list directly from libc++ (as this is now being part of clang rather than just clangd), but having the current symbol mapping available for other tools too is definitely a

[PATCH] D118471: [clang][Lexer] Make raw and normal lexer behave the same for line comments

kadircet added inline comments. Comment at: clang/unittests/Lex/LexerTest.cpp:654 + while (!L.LexFromRawLexer(T)) { +ASSERT_TRUE(!ToksView.empty()); +EXPECT_EQ(T.getKind(), ToksView.front().getKind()); probinson wrote: > @kadircet @sammccall It turns

[PATCH] D113874: [clang] Propagate requires-clause from constructor template to implicit deduction guide

kadircet added a comment. As for testing strategy, after this change the AST should look different. You can probably modify `clang/test/SemaTemplate/deduction-guide.cpp` to check for existence of requires clauses in the implicitly generated function templates for guides. Repository: rG

[PATCH] D118782: clangd: Add a break for every case in the PopulateSwitch tweak

kadircet added a comment. I agree with Sam on this one and remember having some (possibly offline) discussions around not having those breaks explicitly (we should've documented the reasoning, sorry). As pointed out, I believe the main benefit is not spelling out each enum value (it's usually

[PATCH] D118913: [clang-tidy] Fix LLVM include order check policy

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0447ec2fb050: [clang-tidy] Fix LLVM include order check policy (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D118913: [clang-tidy] Fix LLVM include order check policy

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: carlosgalvezp, xazax.hun. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Clang-format LLVM style has a custom include

[PATCH] D118592: [cte] Add release notes for clangd-14

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG236458ef0298: [cte] Add release notes for clangd-14 (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D118592: [cte] Add release notes for clangd-14

kadircet updated this revision to Diff 404515. kadircet added a comment. - Fix spelling of `highlighting` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118592/new/ https://reviews.llvm.org/D118592 Files: clang-tools-extra/docs/ReleaseNotes.rst

[PATCH] D118471: [clang][Lexer] Make raw and normal lexer behave the same for line comments

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. kadircet marked 2 inline comments as done. Closed by commit rGff77071a4d67: [clang][Lexer] Make raw and normal lexer behave the same for line comments (authored by

[PATCH] D118592: [cte] Add release notes for clangd-14

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

[PATCH] D118592: [cte] Add release notes for clangd-14

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

[PATCH] D118471: [clang][Lexer] Make raw and normal lexer behave the same for line comments

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a subscriber: usaxena95. kadircet requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang. Normally there are heruistics in lexer to treat `//*`

[PATCH] D115232: [clangd] Indexing of standard library

kadircet added a comment. thanks LG, mostly nits and a couple of questions Comment at: clang-tools-extra/clangd/ClangdServer.cpp:91 +if (Tasks) + Tasks->runAsync("IndexStdlib", std::move(Task)); +else I suppose this should be rare hence won't bite

[PATCH] D118260: [clangd][Hover] Suppress initializers with many tokens

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc4e68953f644: [clangd][Hover] Suppress initializers with many tokens (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D118260: [clangd][Hover] Suppress initializers with many tokens

kadircet updated this revision to Diff 403583. kadircet added a comment. - Add comments about why we suppress initializer printing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118260/new/ https://reviews.llvm.org/D118260 Files:

[PATCH] D118260: [clangd][Hover] Suppress initializers with many tokens

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

[PATCH] D118245: [clang][DeclPrinter] Fix printing for noexcept expressions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb777d354f670: [clang][DeclPrinter] Fix printing for noexcept expressions (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D117864: [clangd] Enable hover on character literal.

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/HoverTests.cpp:1254 + [](HoverInfo ) { + HI.Name = "expression", HI.Type = "char"; +

[PATCH] D118245: [clang][DeclPrinter] Fix printing for noexcept expressions

kadircet created this revision. kadircet added a reviewer: kbobyrev. Herald added a subscriber: usaxena95. kadircet requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang. We are already building into the final result, no need to

[PATCH] D116085: [clang-tidy] Performance improvements for NOLINTBEGIN/END blocks

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp:187 +static SmallVector +formNoLintBlocks(SmallVector , +

[PATCH] D98110: [NFC][clangd] Use table to collect option aliases

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

[PATCH] D116085: [clang-tidy] Performance improvements for NOLINTBEGIN/END blocks

kadircet added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:214 + std::string CheckName = getCheckName(Info.getID()); + return NoLintHandler.shouldSuppress(DiagLevel, Info, CheckName, NoLintErrors); +} we don't

[PATCH] D116786: [clangd] Add designator inlay hints for initializer lists.

kadircet added a comment. > Agree. I guess then we ignore the issue for this patch? As you say that's > affects other hints too, and a mitigation (hopefully) won't be specific to > the kind of hint. Yup. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D116786: [clangd] Add designator inlay hints for initializer lists.

kadircet added a comment. I was also thinking about a cache-based solution in which we can update results as we please, and as you noted the idea definitely generalizes to other requests as well, with possibly little nuances around the definition of "as we please", which makes things a lot

[PATCH] D117776: [clangd] Fail inlayHints requests on content changes

This revision was automatically updated to reflect the committed changes. Closed by commit rG825a3cd6b697: [clangd] Fail inlayHints requests on content changes (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117776/new/

[PATCH] D117792: [clangd][Background] Make index validation logs verbose

This revision was automatically updated to reflect the committed changes. Closed by commit rG597eae998a87: [clangd][Background] Make index validation logs verbose (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117792/new/

[PATCH] D116085: [clang-tidy] Performance improvements for NOLINTBEGIN/END blocks

kadircet added inline comments. Comment at: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/1st-translation-unit.cpp:1 +// NOLINTBEGIN +class A { A(int i); }; kadircet wrote: > no run lines or anything here (and the following file) oops,

[PATCH] D116085: [clang-tidy] Performance improvements for NOLINTBEGIN/END blocks

kadircet added a comment. here are some more comments, can't say I've been able to go through all of the patch yet, unfortunately it's considerably big in size. it would be great to get rid of some of those extra code by just dropping accessors/classes when not needed.

[PATCH] D117792: [clangd][Background] Make index validation logs verbose

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman, kristof.beyls. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. These errors

[PATCH] D117776: [clangd] Fail inlayHints requests on content changes

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

[PATCH] D116786: [clangd] Add designator inlay hints for initializer lists.

kadircet added a comment. > Any ideas? :-( I've got a feeling that, this should also be annoying in the case of function calls (especially when there's only one overload)? Maybe we should just debounce the inlayhint requests (by saying contents modified to the clients)? Repository: rG LLVM

[PATCH] D117549: [clangd] Sort targets before printing for tests

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcae932b6c6d3: [clangd] Sort targets before printing for tests (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D117549: [clangd] Sort targets before printing for tests

kadircet updated this revision to Diff 401178. kadircet marked an inline comment as done. kadircet added a comment. - Store in vector and sort Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117549/new/ https://reviews.llvm.org/D117549 Files:

[PATCH] D117549: [clangd] Sort targets before printing for tests

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman, mgrang. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Targets are not

[PATCH] D116786: [clangd] Add designator inlay hints for initializer lists.

kadircet added a comment. a drive by concern around possible flakiness of the interaction. have you checked how does it look like when the user is still forming the initializer? it might be annoying if their cursor kept jumping around while they're editing the (possibly half-formed)

[PATCH] D117036: [clangd] Remove --inlay-hints flag

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, as discussed offline, i was mostly worried about the user confusion we would get when people can't turn it off with `--inlay-hints=false` but this has both made it into clangd-13

[PATCH] D117036: [clangd] Remove --inlay-hints flag

kadircet added inline comments. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:296 RetiredFlag ClangTidyChecks("clang-tidy-checks"); +RetiredFlag InlayHints("inlay-hints"); oops, missed this one in the previous revision. any reason for not directly

[PATCH] D114724: [clangd][StdSymbolMap] Prefer std::remove from algorithm

This revision was automatically updated to reflect the committed changes. Closed by commit rGc490f8feb71e: [clangd][StdSymbolMap] Prefer std::remove from algorithm (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114724/new/

[PATCH] D114724: [clangd][StdSymbolMap] Prefer std::remove from algorithm

kadircet updated this revision to Diff 399300. kadircet marked 5 inline comments as done. kadircet added a comment. - Change the variant acception mechanism to work with specific variants. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114724/new/

[PATCH] D117037: [clang][CodeComplete] Perform approximate member search in bases

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG612f5ed88231: [clang][CodeComplete] Perform approximate member search in bases (authored by kadircet). Repository: rG LLVM Github Monorepo

[PATCH] D116294: [CodeCompletion] (mostly) fix completion in incomplete C++ ctor initializers.

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

[PATCH] D117037: [clang][CodeComplete] Perform approximate member search in bases

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:3459 + const std::string Code = R"cpp( +struct Base { Base foo(); }; + sammccall wrote: > does this also work if the base is dependent (Base)? yes

[PATCH] D117037: [clang][CodeComplete] Perform approximate member search in bases

kadircet updated this revision to Diff 399252. kadircet marked 5 inline comments as done. kadircet added a comment. - Replace clangd unittest with clang lit - get rid of non-cxxrecorddecl handling Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D117037: [clang][CodeComplete] Perform approximate member search in bases

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added projects: clang, clang-tools-extra. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D117036: [clangd] Retire --inlay-hints flag

kadircet added a comment. nit: I'd rename the patch to `remove` rather than `retire`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117036/new/ https://reviews.llvm.org/D117036 ___ cfe-commits mailing

[PATCH] D116294: [CodeCompletion] (mostly) fix completion in incomplete C++ ctor initializers.

kadircet added inline comments. Comment at: clang/lib/Parse/ParseCXXInlineMethods.cpp:153 + // probably truncated, so don't eat more tokens. + if (!Toks.back().is(tok::code_completion)) +SkipMalformedDecl(); i don't follow the logic here. maybe

[PATCH] D115490: [clangd] Include fixer for missing functions in C

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaDecl.cpp:14995 +DeclFilterCCC CCC{}; +Corrected = CorrectTypo(DeclarationNameInfo(, Loc), LookupOrdinaryName, +

[PATCH] D116721: [Tooling] When transferring compile commands between files, always use '--'

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

[PATCH] D116699: [clangd] Polish clangd/inlayHints and expose them by default.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks this LGTM just a couple nits. my biggest concern is old endpoint will vanish all of a sudden and it might create some confusion, especially for editors that turn on inlayhints

[PATCH] D115301: [clangd] Don't index __reserved_names in headers.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/SourceCode.h:334 +/// Returns true if Name is reserved, like _Foo or __Vector_base. +inline bool

[PATCH] D116326: [CodeCompletion] Signature help for aggregate initialization.

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

[PATCH] D116196: [clangd] Add CompileFlags.Compiler option to override argv0

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/ConfigFragment.h:140 +/// The name can affect how flags are parsed (clang++ vs clang). +/// If the executable name

[PATCH] D116167: [clangd] Adjust compile flags so they work when applied to other file(type)s.

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/CompileCommands.cpp:287 + if (TransferFrom) { +tooling::CompileCommand TransferCmd; maybe

[PATCH] D116326: [CodeCompletion] Signature help for aggregate initialization.

kadircet added inline comments. Comment at: clang/include/clang/Sema/CodeCompleteConsumer.h:1085 +const RecordDecl *getAggregate() const { + return getKind() == CK_Aggregate ? AggregateType : nullptr; +} either assert the kind and return

[PATCH] D116317: [CodeCompletion] Signature help for braced constructor calls

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:995 /// \param NumCandidates the number of overload candidates void

[PATCH] D116147: [clangd] Respect .clang-tidy ExtraArgs (-Wfoo only) when producing diagnostics

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/ParsedAST.cpp:270 +// We don't want this, so keep track of them to fix afterwards. +llvm::DenseSet

[PATCH] D116352: [CodeCompletion] Signature help for template argument lists

kadircet accepted this revision. kadircet added a comment. thanks! as discussed offline i think it's better to ignore function parameters for now. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3781 +// If the template is for a function `bool foo(int)`, then returns +//

[PATCH] D116502: [clangd] Helper for determining member insertion point.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. Thanks, LGTM! In D116502#3217270 , @sammccall wrote: > In D116502#3217084 , @kadircet > wrote: > >> We

[PATCH] D116326: [CodeCompletion] Signature help for aggregate initialization.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1184 - auto Name = Ident->getName(); - if (!Name.empty()) -ParamNames.insert(Name.str()); we were never recording unnamed params before, now they'll be

[PATCH] D116352: [CodeCompletion] Signature help for template argument lists

kadircet added a comment. thanks this looks great! just a question around rendering of the result type chunk. feel free to leave a fixme if there's no easy way of doing that. Comment at: clang/lib/Parse/ParseTemplate.cpp:1237 +/// +/// \param NameHint is not required, and

[PATCH] D116502: [clangd] Helper for determining member insertion point.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/InsertionPoint.cpp:51 +} else { + assert(A.Direction == Anchor::Below); + if (LastMatched && !Matches) nit: use a switch? Comment at:

[PATCH] D116502: [clangd] Helper for determining member insertion point.

kadircet added a comment. We have some logic in AddUsing tweak to determine insertion point based on AST. i think it makes sense to migrate it to these helpers too. There's some more logic in extract variable/function too. Extract variable seems too elaborate as it actually looks at

[PATCH] D116085: [clang-tidy] Performance improvements for NOLINTBEGIN/END blocks

kadircet added a comment. some drive-by comments, i'll be AFK for a while though. so please don't block this on my approval if people disagree. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:197 + SourceLocation Loc = FileStartLoc.getLocWithOffset(

[PATCH] D116043: [clangd] Don't trim xrefs references if we overran the limit

This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rG81967b4fa77a: [clangd] Dont trim xrefs references if we overran the limit (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D116043: [clangd] Don't trim xrefs references if we overran the limit

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

[PATCH] D115799: [clangd] Add ) to signature-help triggers

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG517f1d9e5cd0: [clangd] Add ) to signature-help triggers (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D115799: [clangd] Add ) to signature-help triggers

kadircet created this revision. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. It is important for nested function calls. Repository: rG LLVM

[PATCH] D115650: [clangd] Disable support for clang-tidy suppression blocks (NOLINTBEGIN)

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. I am a little worried about the discrepancy we'll get here, but (un)fortunately it won't be the first with clang-tidy. Also gave the author a headsup in

[PATCH] D108560: [clang-tidy] Add support for `NOLINTBEGIN` ... `NOLINTEND` comments

kadircet added a comment. Hi @salman-javed-nz ! Thanks for the great work and working on the performance improvements. Just FYI, we are also disabling this in clangd's clang-tidy integration as it is increasing the latency of diagnostic generation. It would be great if you also provided some

[PATCH] D115604: [Support] Expand `<@>` as the base directory in response files.

kadircet added a comment. IIUC, the new behavior being introduced by this patch is not the ability of having a way to refer to other files in a config/response file relative way, but rather extending that ability from only the options that start with `@` to options that contain `@` as a

[PATCH] D115484: [clangd] Include-fixer: handle more "incomplete type" diags, clean up tests

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeFixer.cpp:75 + /* + There are many "incomplete type" diagnostics! + They are almost all Sema diagnostics with "incomplete" in the name. have you considered updating these

[PATCH] D115243: [clangd] Extend SymbolOrigin, stop serializing it

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. Thanks, LGTM! --- > The problem is these methods yield SymbolSlabs, and the symbols within them > are frozen/const. There's no provision to "just tweak some bitfield" - we'd > have to

[PATCH] D115442: [clangd] Provide documentation as MarkupContent in signaturehelp

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd3606a3375d2: [clangd] Provide documentation as MarkupContent in signaturehelp (authored by kadircet). Repository: rG LLVM Github Monorepo

[PATCH] D115442: [clangd] Provide documentation as MarkupContent in signaturehelp

kadircet updated this revision to Diff 393438. kadircet marked 2 inline comments as done. kadircet added a comment. - Drop extra tests - Move capability closer to others Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115442/new/

[PATCH] D115442: [clangd] Provide documentation as MarkupContent in signaturehelp

kadircet added a comment. Also some related discussions in https://github.com/clangd/clangd/issues/945 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115442/new/ https://reviews.llvm.org/D115442 ___

[PATCH] D107275: [Sema] a[x] has type T when a has type T* or T[], even when T is dependent

kadircet added a comment. Sorry for leaving this without any replies, I think the summary you have is already of our offline discussion. Let me raise my final concerns, if you think we're covered for those and others don't chime in this week I suppose we can consider this as good to go. ---

[PATCH] D115442: [clangd] Provide documentation as MarkupContent in signaturehelp

kadircet added a comment. I am thinking about a follow-up which will actually change `formatDocumentation` to return a `Markup::Document` instead to make sure we will have a single way of representing documents throughout features in future. But that's also what we store in the index, hence

[PATCH] D115442: [clangd] Provide documentation as MarkupContent in signaturehelp

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

[PATCH] D115345: [clangd] Suppress IncludeCleaner warnings for headers behind pragma keep

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/IncludeCleanerTests.cpp:414 + auto = AST.getIncludeStructure(); + EXPECT_THAT(getUnused(AST,

[PATCH] D114072: [clangd] Record IWYU pragma keep in the IncludeStructure

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, LGTM! sorry for taking so long. Comment at: clang-tools-extra/clangd/Headers.cpp:142 private: + // Level will be increased every time we enter the file and

[PATCH] D114072: [clangd] Record IWYU pragma keep in the IncludeStructure

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Headers.cpp:126 + bool HandleComment(Preprocessor , SourceRange Range) override { +if (!isInsideMainFile(Range.getBegin(), SM) || Range.getBegin().isMacroID()) + return false; we're

[PATCH] D114665: [clangd] Make a.k.a printing configurable.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ConfigFragment.h:271 + /// Describes hover preferences. + struct HoverBlock { +/// Whether hover show a.k.a type. lh123 wrote: > sammccall wrote: > > One question is whether the setting

[PATCH] D115243: [clangd] Extend SymbolOrigin, stop serializing it

kadircet added a comment. I think we also need to update `index/remote/Server.cpp` && `FileSymbols` (and `FileIndex` too). Regarding updates to `loadIndex`, I actually think it makes sense for that index the always retrieve symbols as `Static` origin, then whoever makes use of that (we always

[PATCH] D114949: [clangd] IncludeCleaner: Do not require forward declarations of RecordDecls when definition is available

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:127 +// declared multiple times. The most common two cases are: +// - Definition available in TU,

[PATCH] D113896: [clangd] cleanup of header guard names

kadircet added a comment. > This looks to be a deliberate decision: > https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h#L32 > > Looking at the code under `clang/include`, it looks like most headers don't > have the `#endif // COMMENT`, so this

[PATCH] D114949: [clangd] IncludeCleaner: Do not require forward declarations of RecordDecls when definition is available

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:151 + if (SM.isInMainFile(RD->getMostRecentDecl()->getLocation())) { +if (Definition) Result.insert(Definition->getLocation()); if we made it here, we

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