[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

2023-10-22 Thread Nathan Ridge via Phabricator via cfe-commits
nridge marked 2 inline comments as done. nridge added a comment. @sammccall, how would you feel about proceeding with the patch in its current state, with the memory usage increase brought down from 8.2% to 2.5% thanks to the combination of the simple lookup optimization + RefKind filtering,

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

2023-10-22 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 557830. nridge added a comment. Address other review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93829/new/ https://reviews.llvm.org/D93829 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

2023-10-22 Thread Nathan Ridge via Phabricator via cfe-commits
nridge planned changes to this revision. nridge added a comment. The updated patch additionally implements the "simple lookup optimization" discussed in the review. With this version, memory usage on the test workload is: background_index 574MB (index 387MB, slabs 187MB). This is an increase

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

2023-10-22 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 557829. nridge added a comment. Implement the "simple lookup optimization" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93829/new/ https://reviews.llvm.org/D93829 Files:

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

2023-10-18 Thread Nathan Ridge via Phabricator via cfe-commits
nridge planned changes to this revision. nridge added a comment. The updated patch implements one of the optimizations discussed during review, namely filtering the Refs stored in the RevRefs data structure to just those that could be calls. To this end, the patch introduces a new `RefKind`,

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

2023-10-18 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 557745. nridge added a comment. Implement optimization to filter the refs stored in RevRefs to calls only Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93829/new/ https://reviews.llvm.org/D93829 Files:

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

2023-10-11 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. To be able to reason about the impact of various memory usage optimizations, I took some baseline measurements. I used a workload consisting of the LLVM codebase, with the compile_commands.json entries filtered to those containing `clang/lib` or

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

2023-10-11 Thread Nathan Ridge via Phabricator via cfe-commits
nridge commandeered this revision. nridge added a reviewer: qchateau. nridge added a comment. In D93829#4422090 , @qchateau wrote: > So if anyone wants to take it from there, feel free to reuse my commits (or > not). I'm going to pick up this work and

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

2023-10-02 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 557525. nridge added a comment. Rebased to apply to recent trunk Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93829/new/ https://reviews.llvm.org/D93829 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp

[PATCH] D145843: [clangd] Add option to always insert headers with <> instead of ""

2023-09-29 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D145843#4221826 , @nridge wrote: > Thanks for the clarification and suggested formulation. > > In D145843#4209750 , @sammccall > wrote: > >> I'd suggest something like: >> >> Style:

[PATCH] D156605: [clangd][CodeComplete] Improve FunctionCanBeCall

2023-09-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Thank you, the updates look good! Please go ahead and merge after addressing the last minor nits. Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:594 +

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

2023-09-11 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. This can be closed, as the change has been made in https://github.com/llvm/llvm-project/pull/65824 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138546/new/ https://reviews.llvm.org/D138546 ___ cfe-commits mailing

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

2023-09-11 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG94b14355e2ef: [clangd] allow extracting to variable for lambda

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

2023-09-10 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision. nridge added a comment. I'm fine with allowing partial selection for consistency with other expressions. I think this patch is in a good state. Thanks for your patience with the review :) Let me know if you need me to commit it for you. Repository: rG LLVM

[PATCH] D139277: [clangd] Use all query-driver arguments in cache key

2023-09-09 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. This was superseded by D146941 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139277/new/ https://reviews.llvm.org/D139277 ___ cfe-commits

[PATCH] D158804: [Clang] Fix crash in Parser::ParseDirectDeclarator by adding check that token is not an annotation token

2023-09-06 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision. nridge added a subscriber: rjmccall. nridge added a comment. This revision is now accepted and ready to land. I'm not well versed in the parser code, but the fix clearly avoids the assertion in `getIdentifierInfo()`, and both @aaron.ballman and @rjmccall indicated

[PATCH] D158926: [clangd] Show parameter hints for operator()

2023-09-05 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision. nridge added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/InlayHints.cpp:626 + Method && Method->isInstance()) +Args = Args.drop_front(1); +processCall(Callee, Args);

[PATCH] D156605: [clangd][CodeComplete] Improve FunctionCanBeCall

2023-09-05 Thread Nathan Ridge via Phabricator via cfe-commits
nridge requested changes to this revision. nridge added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:1297 +FunctionCanBeCall = +MaybeDerived == MaybeBase || MaybeDerived->isDerivedFrom(MaybeBase); +

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

2023-09-04 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. (Otherwise the updates look good!) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141757/new/ https://reviews.llvm.org/D141757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

2023-09-04 Thread Nathan Ridge via Phabricator via cfe-commits
nridge requested changes to this revision. nridge added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:187 +// Allow expressions, but only allow completely selected lambda +

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

2023-09-04 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:102 + + if (clang::Expr *const RequiresClause = + LExpr->getTrailingRequiresClause(); nit: just `if (clang::Expr *const RequiresClause =

[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

2023-08-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D153114#4630318 , @ChuanqiXu wrote: > However, I can't search the caller of `reparseOpenFilesIfNeeded` which > semantics matches the behavior. The two callers of > `reparseOpenFilesIfNeeded` I found are >

[PATCH] D156605: [clangd][CodeComplete] Improve FunctionCanBeCall

2023-08-26 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. (I'm away on travels, will get back to this within the next week) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156605/new/ https://reviews.llvm.org/D156605 ___ cfe-commits

[PATCH] D158873: [clangd] Do not ignore qualifier in heuristic resolution of dependent MemberExpr

2023-08-25 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf0f53cb4bfb2: [clangd] Do not ignore qualifier in heuristic resolution of dependent MemberExpr (authored by nridge). Repository: rG LLVM Github

[PATCH] D158873: [clangd] Do not ignore qualifier in heuristic resolution of dependent MemberExpr

2023-08-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 553607. nridge added a comment. Address review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158873/new/ https://reviews.llvm.org/D158873 Files: clang-tools-extra/clangd/HeuristicResolver.cpp

[PATCH] D158873: [clangd] Do not ignore qualifier in heuristic resolution of dependent MemberExpr

2023-08-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman. Herald added a project: All. nridge requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra.

[PATCH] D158851: [clangd] Avoid null result in FindRecordTypeAt()

2023-08-25 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1994e1173b64: [clangd] Avoid null result in FindRecordTypeAt() (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158851/new/

[PATCH] D158851: [clangd] Avoid null result in FindRecordTypeAt()

2023-08-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added reviewers: kadircet, hokein. Herald added a subscriber: arphaman. Herald added a project: All. nridge requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository:

[PATCH] D157990: [clangd] Add --query-driver flag to clangd-indexer

2023-08-24 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Committed, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157990/new/ https://reviews.llvm.org/D157990 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D157990: [clangd] Add --query-driver flag to clangd-indexer

2023-08-24 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG109bc024c8d7: [clangd] Add --query-driver flag to clangd-indexer (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157990/new/

[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

2023-08-21 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D153114#4603579 , @sammccall wrote: > Dep scanning - roles > > > IIUC we do this for two reasons: > > - to identify what module names we must have PCMs for in order to build a > given TU (either an open

[PATCH] D157990: [clangd] Add --query-driver flag to clangd-indexer

2023-08-20 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Thanks for the patch! My thoughts are: - As mentioned in the issue, I think this fills a logical gap: clangd-indexer is an alternative way of generating a project index to clangd's

[PATCH] D157956: [clangd] don't add inlay hint for dependent type in structured binding

2023-08-20 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Thanks! A future enhancement to consider could be, in the following testcase: template struct Pair { T t; U u; }; template void foobar(Pair arg) { auto [a, b] =

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

2023-08-18 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. nridge marked an inline comment as done. Closed by commit rG8ee710a40cc5: [clangd] Parameter hints for calls through function pointers (authored by nridge).

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

2023-08-18 Thread Nathan Ridge via Phabricator via cfe-commits
nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:508 + // Only one of Callee or ProtoTypeLoc is set. + const FunctionDecl *Callee = nullptr; + FunctionProtoTypeLoc ProtoTypeLoc;

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

2023-08-18 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 551696. nridge added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158249/new/ https://reviews.llvm.org/D158249 Files: clang-tools-extra/clangd/InlayHints.cpp

[PATCH] D149236: [clangd] Bail gracefully if given an assembly or IR source file

2023-08-18 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG744b111434b2: [clangd] Bail gracefully if given an assembly or IR source file (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D149236: [clangd] Bail gracefully if given an assembly or IR source file

2023-08-18 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 551694. nridge added a comment. Address review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149236/new/ https://reviews.llvm.org/D149236 Files: clang-tools-extra/clangd/ParsedAST.cpp

[PATCH] D158248: [clangd] Fix incorrect RecursiveASTVisitor usage in summarizeExpr()

2023-08-18 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG3e69886dd012: [clangd] Fix incorrect RecursiveASTVisitor usage in summarizeExpr() (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

2023-08-18 Thread Nathan Ridge via Phabricator via cfe-commits
nridge requested changes to this revision. nridge added a comment. This revision now requires changes to proceed. Ok, I've finished looking through the patch; sorry it took so long to get to. It generally looks pretty good to me, I just have some fairly minor comments. Thanks again for your

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

2023-08-18 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 551390. nridge added a comment. Fix naming nit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158249/new/ https://reviews.llvm.org/D158249 Files: clang-tools-extra/clangd/InlayHints.cpp

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

2023-08-18 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:466 +// so that we can recover argument names from it. +// FIXME: This function is mostly duplicated in SemaCodeComplete.cpp; unify. +static FunctionProtoTypeLoc getPrototypeLoc(Expr *Callee) {

[PATCH] D158248: [clangd] Fix incorrect RecursiveASTVisitor usage in summarizeExpr()

2023-08-18 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added reviewers: sammccall, hokein. Herald added subscribers: kadircet, arphaman. Herald added a project: All. nridge requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra.

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

2023-08-18 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:466 +// so that we can recover argument names from it. +// FIXME: This function is mostly duplicated in SemaCodeComplete.cpp; unify. +static FunctionProtoTypeLoc getPrototypeLoc(Expr *Callee) {

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

2023-08-17 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added reviewers: sammccall, hokein. Herald added subscribers: kadircet, arphaman. Herald added a project: All. nridge requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra.

[PATCH] D147905: [clangd] Avoid passing -xobjective-c++-header to the system include extractor

2023-08-17 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbd74186f1a08: [clangd] Avoid passing -xobjective-c++-header to the system include extractor (authored by nridge). Repository: rG LLVM Github

[PATCH] D147905: [clangd] Avoid passing -xobjective-c++-header to the system include extractor

2023-08-17 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 551377. nridge added a comment. Rebase and address review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147905/new/ https://reviews.llvm.org/D147905 Files:

[PATCH] D157952: [clang] Support function pointer types with attributes when extracting parameter names for signature help

2023-08-17 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd9cb76bc4d5e: [clang] Support function pointer types with attributes when extracting… (authored by nridge). Repository: rG LLVM Github Monorepo

[PATCH] D157952: [clang] Support function pointer types with attributes when extracting parameter names for signature help

2023-08-17 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 551374. nridge added a comment. Address review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157952/new/ https://reviews.llvm.org/D157952 Files:

[PATCH] D149236: [clangd] Bail gracefully if given an assembly or IR source file

2023-08-17 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a reviewer: sammccall. nridge added a comment. Adding Sam, since you're on a review roll ;) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149236/new/ https://reviews.llvm.org/D149236 ___

[PATCH] D157952: [clang] Support function pointer types with attributes when extracting parameter names for signature help

2023-08-15 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added reviewers: hokein, kadircet, Qwinci. Herald added a subscriber: arphaman. Herald added a project: All. nridge requested review of this revision. Herald added subscribers: cfe-commits, wangpc, ilya-biryukov. Herald added projects: clang, clang-tools-extra.

[PATCH] D157080: [clangd] Symbol search includes parameter types for (member) functions

2023-08-14 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. I understood https://github.com/clangd/clangd/issues/1344 as being about `workspace/symbol` rather than `textDocument/documentSymbol`, though I see now that both are affected. The analysis is a bit different for the two, though: unlike `DocumentSymbol`, the result

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

2023-08-14 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Haven't looked at everything yet, but wanted to mention one thing I noticed. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:91 + +// Local variables declared inside of the selected lambda cannot go out of +// scope. The

[PATCH] D156605: [clangd][CodeComplete] Improve FunctionCanBeCall

2023-08-13 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Thanks for the patch, these are nice improvements Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:258 case CodeCompletionString::CK_RightParen: + if (DropFunctionArguments && + ResultKind ==

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

2023-08-10 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. I promise I haven't forgotten about this. It's just been a challenge finding a large enough chunk of time to page in all the relevant context and think through the issues. Maybe this weekend... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D155370: [CodeComplete] Improve FunctionCanBeCall

2023-08-02 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D155370#4550042 , @zyounan wrote: > in the context where `CanBeCall=false`, parameters don't disambiguate against > the overloads, so we'd end up with the same function name after selecting the > candidate I was thinking of

[PATCH] D156300: [clangd] Avoid unexpected desugaring in isSugaredTemplateParameter

2023-08-01 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:1432 TEST(TypeHints, SubstTemplateParameterAliases) { + llvm::StringRef Header = R"cpp(

[PATCH] D150124: [index][clangd] Consider labels when indexing function bodies

2023-07-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150124/new/ https://reviews.llvm.org/D150124

[PATCH] D156300: [clangd] Avoid unexpected desugaring in isSugaredTemplateParameter

2023-07-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:198 bool isSugaredTemplateParameter(QualType QT) { static auto PeelWrappers = [](QualType QT) { // Neither `PointerType` nor `ReferenceType` is considered as sugared nit:

[PATCH] D155370: [CodeComplete] Improve FunctionCanBeCall

2023-07-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. I'm not sure how I feel about dropping the parameters from the signature in the `CanBeCall = false` case. I can see arguments in both directions: - On the one hand, dropping the parameters makes the text that is inserted more consistent with the text that is shown for

[PATCH] D156300: [clangd] Avoid unexpected desugaring in isSugaredTemplateParameter

2023-07-29 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Your patience is appreciated. I have a number of patches in my review queue, and 3 days is often not a realistic turnaround time for me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156300/new/

[PATCH] D150124: [index][clangd] Consider labels when indexing function bodies

2023-07-28 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D150124#4542032 , @kadircet wrote: > btw, thanks a lot Nathan for taking good care of clangd, I don't think I say > that enough I'm happy to help! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150124: [index][clangd] Consider labels when indexing function bodies

2023-07-28 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang/lib/Index/IndexBody.cpp:150 +ParentDC, +unsigned(SymbolRole::NameReference)); + } ckandeler wrote: > kadircet wrote: > > nridge wrote: > > >

[PATCH] D156513: [clang] [libIndex] Remove unused 'RefD' parameter of IndexingContext::handleReference()

2023-07-28 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5ddc37189032: [clang] [libIndex] Remove unused RefD parameter of IndexingContext… (authored by nridge). Repository: rG LLVM Github Monorepo

[PATCH] D156513: [clang] [libIndex] Remove unused 'RefD' parameter of IndexingContext::handleReference()

2023-07-28 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Noticed this while reviewing D150124 . No caller was providing a value for this parameter. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156513/new/ https://reviews.llvm.org/D156513

[PATCH] D156513: [clang] [libIndex] Remove unused 'RefD' parameter of IndexingContext::handleReference()

2023-07-28 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added a reviewer: hokein. Herald added a subscriber: arphaman. Herald added a project: All. nridge requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D147905: [clangd] Avoid passing -xobjective-c++-header to the system include extractor

2023-07-28 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a reviewer: sammccall. nridge added a comment. Adding Sam as well in case he has any thoughts on the discussion (another user ran into this recently and filed https://github.com/clangd/clangd/issues/1694) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150124: [index][clangd] Consider labels when indexing function bodies

2023-07-28 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D150124#4329163 , @ilya-biryukov wrote: > In `clangd` we also have `findExplicitReferences` and `targetDecl` functions > that seem to handle the `GoToStmt`. > In fact, I believe they are used in `findDocumentHighlights`, so

[PATCH] D154471: [clang] Add serialization support for the DynamicAllocLValue variant of APValue::LValueBase::Ptr

2023-07-21 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2e7d711a1061: [clang] Add serialization support for the DynamicAllocLValue variant of APValue… (authored by nridge). Repository: rG LLVM Github

[PATCH] D153946: [clangd] Add a flag to allow indexing of reserved identifiers

2023-07-21 Thread Nathan Ridge via Phabricator via cfe-commits
nridge abandoned this revision. nridge added a comment. Abandoning in favour of D155381 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153946/new/ https://reviews.llvm.org/D153946

[PATCH] D154471: [clang] Add serialization support for the DynamicAllocLValue variant of APValue::LValueBase::Ptr

2023-07-21 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 542793. nridge added a comment. Add testcase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154471/new/ https://reviews.llvm.org/D154471 Files: clang/include/clang/AST/PropertiesBase.td

[PATCH] D153946: [clangd] Add a flag to allow indexing of reserved identifiers

2023-07-21 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Yeah, I'm happy to go with D155381 instead. In D153946#4503585 , @sammccall wrote: > (Stupid over-flexible config system is too slow... I've learned my lesson > from that one!) The issues with

[PATCH] D155381: [clangd] Allow indexing of __reserved_names outside system headers

2023-07-21 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Thanks! I agree that this no-configuration approach is nicer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155381/new/

[PATCH] D140745: Generate Config {Fragment structure, json schema, docs, YAML parser} from schema spec

2023-07-17 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D140745#4505829 , @sammccall wrote: > ping on this one for when you have time (Just wanted to double-check, is this ping directed to me or Kadir (or both)? I haven't looked at this because Kadir has done a round of review and

[PATCH] D154853: [clangd][c++20]Consider the constraint of a constrained auto in FindTarget.

2023-07-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:581 + +[[Fooable]] auto i = 42; + )cpp"; nridge wrote: > sammccall wrote: > > this is going to have the same behavior on the `auto` token, right? > > > > This

[PATCH] D154853: [clangd][c++20]Consider the constraint of a constrained auto in FindTarget.

2023-07-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:581 + +[[Fooable]] auto i = 42; + )cpp"; sammccall wrote: > this is going to have the same behavior on the `auto` token, right? > > This is my main practical

[PATCH] D78038: [clangd] WIP: fix several bugs relating to include insertion

2023-07-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Also adding links to the open issues this fixes: - https://github.com/clangd/clangd/issues/333 - https://github.com/clangd/clangd/issues/337 Note the second issue is not specific to include insertion; it's a false positive diagnostic, which I think makes it a more

[PATCH] D78038: [clangd] WIP: fix several bugs relating to include insertion

2023-07-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added reviewers: hokein, kadircet, VitaNuo. nridge added a comment. Let's try adding some reviewers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78038/new/ https://reviews.llvm.org/D78038 ___

[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

2023-07-12 Thread Nathan Ridge via Phabricator via cfe-commits
nridge resigned from this revision. nridge added a comment. I'm sorry, I feel like I don't have a good enough level of insight into the requirements to usefully critique this patch, nor the bandwidth to take a detailed look through the implementation right now. I think it's best for me to

[PATCH] D154471: [clang] Add serialization support for the DynamicAllocLValue variant of APValue::LValueBase::Ptr

2023-07-12 Thread Nathan Ridge via Phabricator via cfe-commits
nridge planned changes to this revision. nridge added a comment. Thank you for the suggested testcase! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154471/new/ https://reviews.llvm.org/D154471 ___

[PATCH] D154903: clangd: Provide the resource dir via environment variable

2023-07-11 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a reviewer: sammccall. nridge added a comment. Looks reasonable to me but Sam should OK this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154903/new/ https://reviews.llvm.org/D154903 ___

[PATCH] D154471: [clang] Add serialization support for the DynamicAllocLValue variant of APValue::LValueBase::Ptr

2023-07-08 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. Herald added a project: All. nridge updated this revision to Diff 537970. nridge added a comment. nridge edited the summary of this revision. Herald added a subscriber: kadircet. nridge published this revision for review. Herald added subscribers: cfe-commits,

[PATCH] D149236: [clangd] Bail gracefully if given an assembly or IR source file

2023-07-08 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Review ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149236/new/ https://reviews.llvm.org/D149236 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D153946: [clangd] Add a flag to allow indexing of reserved identifiers

2023-06-28 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. No tests yet, posting for early feedback. Comment at: clang-tools-extra/clangd/index/StdLib.cpp:233 IndexOpts.StoreAllDocumentation = true; + // FIXME: Should we respect the Index.ReservedIdentifiers config here? // Sadly we can't use

[PATCH] D153946: [clangd] Add a flag to allow indexing of reserved identifiers

2023-06-28 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added reviewers: hokein, kadircet. Herald added a subscriber: arphaman. Herald added a project: All. nridge requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Some

[PATCH] D153248: [clangd] Use resolveTypeToRecordDecl() to resolve the type of a base specifier during heuristic resolution

2023-06-20 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6fe9cfe4137b: [clangd] Use resolveTypeToRecordDecl() to resolve the type of a base specifier… (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D153251: [clangd] Index the type of a non-type template parameter

2023-06-20 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc3075023850b: [clangd] Index the type of a non-type template parameter (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153251/new/

[PATCH] D153251: [clangd] Index the type of a non-type template parameter

2023-06-20 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 532798. nridge added a comment. Address review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153251/new/ https://reviews.llvm.org/D153251 Files: clang/lib/Index/IndexDecl.cpp

[PATCH] D153248: [clangd] Use resolveTypeToRecordDecl() to resolve the type of a base specifier during heuristic resolution

2023-06-19 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D153248#4432108 , @hokein wrote: > The other question might worth thinking, are these cases critical to fix now? > I'm not sure `this->find()` is a common case (`find();` already works today). I may have simplified the test

[PATCH] D153251: [clangd] Index the type of a non-type template parameter

2023-06-19 Thread Nathan Ridge via Phabricator via cfe-commits
nridge planned changes to this revision. nridge added inline comments. Comment at: clang/lib/Index/IndexDecl.cpp:708 } else if (const auto *NTTP = dyn_cast(TP)) { +IndexCtx.indexTypeSourceInfo(NTTP->getTypeSourceInfo(), Parent); if

[PATCH] D153251: [clangd] Index the type of a non-type template parameter

2023-06-19 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added a reviewer: hokein. Herald added subscribers: kadircet, arphaman. Herald added a project: All. nridge requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added projects: clang, clang-tools-extra. Fixes

[PATCH] D153248: [clangd] Use resolveTypeToRecordDecl() to resolve the type of a base specifier during heuristic resolution

2023-06-19 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. As this patch involves copying a bit of code from CXXRecordDecl::lookupDepenentName() to HeuristicResolver, I wanted to mention a couple of alternatives I considered: 1. Copy things in the other direction, i.e. implement some of the necessary parts of

[PATCH] D153248: [clangd] Use resolveTypeToRecordDecl() to resolve the type of a base specifier during heuristic resolution

2023-06-19 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added a reviewer: hokein. Herald added subscribers: kadircet, arphaman. Herald added a project: All. nridge requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. The code

[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

2023-06-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. > The major problem I see now is that we can't handle third party modules. > Third party modules refer to the modules whose source codes are not in > current project. The users are still able to see the hint from clangd if the > BMI of the third party modules are built

[PATCH] D152645: [clangd] Handle DependentNameType in HeuristicResolver::resolveTypeToRecordDecl()

2023-06-16 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7a709689bc17: [clangd] Handle DependentNameType in HeuristicResolver::resolveTypeToRecordDecl… (authored by nridge). Repository: rG LLVM Github

[PATCH] D152645: [clangd] Handle DependentNameType in HeuristicResolver::resolveTypeToRecordDecl()

2023-06-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 532263. nridge marked an inline comment as done. nridge added a comment. Address last comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152645/new/ https://reviews.llvm.org/D152645 Files:

[PATCH] D152645: [clangd] Handle DependentNameType in HeuristicResolver::resolveTypeToRecordDecl()

2023-06-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/HeuristicResolver.cpp:56 + if (const auto *DNT = T->getAs()) { +T = resolveDeclsToType(resolveDependentNameType(DNT)); +if (!T) hokein wrote: > Is the `resolveDeclsToType` call necessary

[PATCH] D152645: [clangd] Handle DependentNameType in HeuristicResolver::resolveTypeToRecordDecl()

2023-06-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 532008. nridge marked 2 inline comments as done. nridge added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152645/new/ https://reviews.llvm.org/D152645 Files:

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

2023-06-14 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Thank you Sam for the suggested performance improvements, and outlining a way forward. @qchateau, are you interested in updating the patch to implement some of the described optimizations / address other comments? Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D152500: [clangd] Unwrap type sugar in HeuristicResolver::resolveTypeToRecordDecl()

2023-06-14 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG19c9af81b1c5: [clangd] Unwrap type sugar in HeuristicResolver::resolveTypeToRecordDecl() (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

  1   2   3   4   5   6   7   8   9   10   >