[PATCH] D137040: [clangd] Add heuristic for dropping snippet when completing member function pointer

tom-anders added a comment. Hmm I added the test for the flag to Sema, but now we kinda have the same test case duplicated in sema and clangd tests - I guess for clangd we now actually only have to test that the SnippetSuffix is cleared when FunctionCanBeCall is true, but I don't see an easy wa

[PATCH] D137650: [clangd] Implement hover for string literals

tom-anders added a comment. In D137650#3919089 , @kadircet wrote: > sure i am happy to land, @tom-anders do you have anything else to add ? LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137650/new/

[PATCH] D137817: [clangd] Improve action `RemoveUsingNamespace` for user-defined literals

tom-anders added a comment. This looks really cool :) Comment at: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:133 + std::string Directives; + for (auto &Using : UsingToAdd) +Directives += llvm::formatv("using namespace {0};\n", Using); -

[PATCH] D137040: [clangd] Add heuristic for dropping snippet when completing member function pointer

tom-anders updated this revision to Diff 474818. tom-anders added a comment. Fix test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137040/new/ https://reviews.llvm.org/D137040 Files: clang-tools-extra/clangd/CodeComplete.cpp clang-tools-extra

[PATCH] D137040: [clangd] Add heuristic for dropping snippet when completing member function pointer

tom-anders updated this revision to Diff 474990. tom-anders added a comment. Clean up test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137040/new/ https://reviews.llvm.org/D137040 Files: clang-tools-extra/clangd/CodeComplete.cpp clang-tools-

[PATCH] D137040: [clangd] Add heuristic for dropping snippet when completing member function pointer

tom-anders added a comment. In D137040#3924115 , @nridge wrote: > Thanks! Thanks for the review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137040/new/ https://reviews.llvm.org/D137040 ___

[PATCH] D137817: [clangd] Improve action `RemoveUsingNamespace` for user-defined literals

tom-anders added a comment. LGTM, but I'll give @kadircet a chance to review as well Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137817/new/ https://reviews.llvm.org/D137817 ___ cfe-commits mailing lis

[PATCH] D138028: [clangd] Fix action `RemoveUsingNamespace` for inline namespace

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:212 // Produce replacements to add the qualifiers. std::string Qualifier = printUsingNamespaceName(Ctx, *TargetDirective) + "::"; for (auto Loc : IdentsToQua

[PATCH] D137909: [Support] Allow complex names for annotation points and ranges via ${}

tom-anders updated this revision to Diff 475922. tom-anders added a comment. Herald added subscribers: cfe-commits, arphaman. Herald added a project: clang-tools-extra. Keep name syntax, add optional payload instead Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D137909: [Support] Allow complex names for annotation points and ranges via ${}

tom-anders added a comment. In D137909#3930043 , @sammccall wrote: > What do you think about `$name(payload)^` for the general case, with > `$(payload)^`, `$name`, and `^` as abbreviated forms? I don't think the lack > of brackets on **names** hurts a l

[PATCH] D137040: [clangd] Add heuristic for dropping snippet when completing member function pointer

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0e00611cbc2b: [clangd] Add heuristic for dropping snippet when completing member function… (authored by tom-anders). Repository: rG LLVM Github Mo

[PATCH] D137909: [Support] Allow complex names for annotation points and ranges via ${}

tom-anders updated this revision to Diff 475939. tom-anders added a comment. Fix clangd::Annotations interface, now also has pointWithPayload etc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137909/new/ https://reviews.llvm.org/D137909 Files:

[PATCH] D137909: [Support] Allow complex names for annotation points and ranges via ${}

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/unittests/Annotations.h:30 + struct Position : clangd::Position { +llvm::Optional Payload; tom-anders wrote: > I thought about just using `std::pair llvm::Optional> instead, but that is

[PATCH] D137909: [Support] Allow complex names for annotation points and ranges via ${}

tom-anders updated this revision to Diff 476230. tom-anders marked 8 inline comments as done. tom-anders added a comment. Use std::pair for returning payloads along with points/ranges. Also, refactor internal logic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D137909: [Support] Allow complex names for annotation points and ranges via ${}

tom-anders added a comment. In D137909#3933730 , @sammccall wrote: > Thanks! sorry about continuing to drift this patch, but this looks like a > great improvement. No problem! I wasn't quite happy yet with the previous version of this patch either, rea

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

tom-anders updated this revision to Diff 493096. tom-anders marked 8 inline comments as done. tom-anders added a comment. Address review comments, add regression test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140915/new/ https://reviews.llvm.or

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

tom-anders marked an inline comment as done. tom-anders added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1714 +} +llvm::copy_if(SpecifiedScopes.scopesForIndexQuery(), + std::back_inserter(QueryScopes), Should I

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

tom-anders updated this revision to Diff 494074. tom-anders marked 4 inline comments as done. tom-anders added a comment. Move EnclosingNamespace logic to scopesForIndexQuery, add FIXME Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140915/new/ http

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1709 + +// The enclosing namespace must be first, it gets a quality boost. +if (auto Enclosing = SpecifiedScopes.EnclosingNamespace) { kadircet wrote: > i was actually

[PATCH] D143112: [clang] Support parsing comments without ASTContext

tom-anders created this revision. tom-anders added a reviewer: kadircet. Herald added a project: All. tom-anders requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang. This is in preparation for implementing doxygen parsing, see disc

[PATCH] D143112: [clang] Support parsing comments without ASTContext

tom-anders added inline comments. Comment at: clang/include/clang/AST/RawCommentList.h:159 + /// Parse the \p Comment, storing the result in \p Allocator + static comments::FullComment *parse(llvm::StringRef Comment, + llvm::BumpPtrAllocator

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

tom-anders updated this revision to Diff 489284. tom-anders added a comment. Address review comment: Introduce new AccessibleScopes variable Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140915/new/ https://reviews.llvm.org/D140915 Files: clang-

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:674 +else if (const auto *ND = dyn_cast(Context)) { + if (ND->isInlineNamespace()) +Scopes.AccessibleScopes.push_back(printQualifiedName(*ND) + "::"); kadirce

[PATCH] D140775: [clangd] Hover: show CalleeArgInfo for literals

This revision was automatically updated to reflect the committed changes. Closed by commit rG343b1ae3622a: [clangd] Hover: show CalleeArgInfo for literals and expressions (authored by tom-anders). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140775/

[PATCH] D141800: [clangd] Fix qualifier not being dropped for using declaration referring to scoped enum

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

[PATCH] D141800: [clangd] Fix qualifier not being dropped for using declaration referring to scoped enum

tom-anders updated this revision to Diff 489391. tom-anders added a comment. Factor out logic into helper function Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141800/new/ https://reviews.llvm.org/D141800 Files: clang-tools-extra/clangd/CodeCom

[PATCH] D141800: [clangd] Fix qualifier not being dropped for using declaration referring to scoped enum

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1993 Builder.emplace(Recorder ? &Recorder->CCSema->getASTContext() : nullptr, -Item, SemaCCS, QueryScopes, *Inserter, FileName, +Recor

[PATCH] D141800: [clangd] Fix qualifier not being dropped for using declaration referring to scoped enum

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1993 Builder.emplace(Recorder ? &Recorder->CCSema->getASTContext() : nullptr, -Item, SemaCCS, QueryScopes, *Inserter, FileName, +Recor

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1035 } else if (const auto *MTE = CastNode->ASTNode.get()) { } else { // Unknown implicit node, assume type conversion. This branch is now empty, do we

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1035 } else if (const auto *MTE = CastNode->ASTNode.get()) { } else { // Unknown implicit node, assume type conversion. v1nh1shungry wrote: > tom-anders

[PATCH] D128202: [clangd] Include "final" when printing class declaration

tom-anders created this revision. Herald added subscribers: usaxena95, kadircet, arphaman. Herald added a project: All. tom-anders requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added projects: clang, clang-tools-extra. Fixes https://github

[PATCH] D128202: [clangd] Include "final" when printing class declaration

tom-anders added inline comments. Comment at: clang/lib/AST/DeclPrinter.cpp:1010 + if (D->isEffectivelyFinal()) { + Out << " final"; sammccall wrote: > isEffectivelyFinal returns true for > `struct X { ~X() final; }` > > I don't think we want to print `s

[PATCH] D141800: [clangd] Fix qualifier not being dropped for using declaration referring to scoped enum

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

[PATCH] D137909: [Support] Allow complex names for annotation points and ranges via ${}

This revision was automatically updated to reflect the committed changes. tom-anders marked 6 inline comments as done. Closed by commit rG3cf14a7bdce0: [Support] Add support for attaching payloads to points and ranges (authored by tom-anders). Changed prior to commit: https://reviews.llvm.org/D

[PATCH] D137909: [Support] Allow complex names for annotation points and ranges via ${}

tom-anders added a comment. Thanks again! Landed this with your suggestions incorporated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137909/new/ https://reviews.llvm.org/D137909 ___ cfe-commits mailin

[PATCH] D137894: [clangd] Add extension for adding context (enclosing function or class) in references results

tom-anders created this revision. Herald added subscribers: kadircet, arphaman. Herald added a project: All. tom-anders updated this revision to Diff 476438. tom-anders added a comment. tom-anders retitled this revision from "[clangd] Prototype for adding enclosing function in references results"

[PATCH] D137894: [clangd] Add extension for adding context (enclosing function or class) in references results

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1939 int main() { - int $def[[foo]]; - [[^foo]] = 2; - int test1 = [[foo]]; + int $def(main)[[foo]]; + $(main)[[^foo]] = 2; --

[PATCH] D137894: [clangd] Add extension for adding context (enclosing function or class) in references results

tom-anders updated this revision to Diff 476918. tom-anders marked 7 inline comments as done. tom-anders added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137894/new/ https://reviews.llvm.org/D137894 Files: clang-to

[PATCH] D137894: [clangd] Add extension for adding context (enclosing function or class) in references results

tom-anders added a comment. In D137894#3940600 , @sammccall wrote: > Since this is a protocol extension, you might want to add an end-to-end test: > variant of `clang-tools-extra/clangd/test/xrefs.test` > (`xrefs-container.test`? to avoid complicating a

[PATCH] D138425: [clangd] Parameter hints for template specialization

tom-anders added a comment. In D138425#3941206 , @sammccall wrote: > In D138425#3941183 , @v1nh1shungry > wrote: > >> Yes, there is a common situation where people use a meaningless template >> parameter name, b

[PATCH] D137894: [clangd] Add extension for adding context (enclosing function or class) in references results

tom-anders updated this revision to Diff 477566. tom-anders added a comment. Add end-to-end test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137894/new/ https://reviews.llvm.org/D137894 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp cla

[PATCH] D138028: [clangd] Fix action `RemoveUsingNamespace` for inline namespace

tom-anders added a comment. In D138028#3957139 , @v1nh1shungry wrote: > A gentle ping? Sorry if it bothers you. Sorry! I thought you’d probably first wanted to address the remaining comments in https://reviews.llvm.org/D137817 because they might also a

[PATCH] D138028: [clangd] Fix action `RemoveUsingNamespace` for inline namespace

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:93 +// Return true if `LHS` is declared in `RHS` +bool declareIn(const NamedDecl *LHS, const DeclContext *RHS) { + const auto *D = LHS->getDeclContext(); -

[PATCH] D147846: [clangd] Hover: resolve forwarding parameters for CalleeArgInfo

tom-anders created this revision. tom-anders added reviewers: nridge, upsj. Herald added subscribers: kadircet, arphaman. Herald added a project: All. tom-anders requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-ext

[PATCH] D147847: [clangd] Hover: Add CalleeArgInfo for constructor expressions

tom-anders created this revision. tom-anders added reviewers: nridge, sammccall. Herald added subscribers: kadircet, arphaman. Herald added a project: All. tom-anders requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tool

[PATCH] D147847: [clangd] Hover: Add CalleeArgInfo for constructor expressions

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:986 + const FunctionDecl *FD = nullptr; + llvm::SmallVector Args; + Unfortunately, while CallExpr and CXXConstructExpr basically have the same API for getting Args, they're not re

[PATCH] D147847: [clangd] Hover: Add CalleeArgInfo for constructor expressions

tom-anders added a comment. (Note that this diff is stacked on D147846 ) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147847/new/ https://reviews.llvm.org/D147847 ___ c

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

tom-anders added a comment. In D142014#4106088 , @v1nh1shungry wrote: > Thanks for the review! @kadircet > > Would you mind having a look at if there're any concerns about the current > code change, @nridge, @tom-anders, and @adamcz? Thanks a lot! Noth

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

tom-anders added a comment. Thanks for reviewing! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140915/new/ https://reviews.llvm.org/D140915 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGce87b0314370: [clangd] Fix getQueryScopes for using-directive with inline namespace (authored by tom-anders). Changed prior to commit: https://rev

[PATCH] D143112: [clang] Support parsing comments without ASTContext

tom-anders updated this revision to Diff 498586. tom-anders added a comment. Herald added a subscriber: arphaman. Herald added a project: clang-tools-extra. - Move to free function in CodeCompletionStrings.h - Add back comment markers before parsing (Index stores comments without markers, but the

[PATCH] D143112: [clang] Support parsing comments without ASTContext

tom-anders marked an inline comment as done. tom-anders added inline comments. Comment at: clang/lib/AST/RawCommentList.cpp:227 + SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID()), + Allocator, SourceMgr.getDiagnostics(), Traits)

[PATCH] D147846: [clangd] Hover: resolve forwarding parameters for CalleeArgInfo

This revision was automatically updated to reflect the committed changes. Closed by commit rGa7b4fd953f44: [clangd] Hover: resolve forwarding parameters for CalleeArgInfo (authored by tom-anders). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147846/

[PATCH] D147847: [clangd] Hover: Add CalleeArgInfo for constructor expressions

tom-anders updated this revision to Diff 517994. tom-anders marked an inline comment as done. tom-anders added a comment. Use ArrayRef instead of SmallVector to avoid copy (also fix 2 typos in the comment below) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D147847: [clangd] Hover: Add CalleeArgInfo for constructor expressions

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:986 + const FunctionDecl *FD = nullptr; + llvm::SmallVector Args; + nridge wrote: > tom-anders wrote: > > Unfortunately, while CallExpr and CXXConstructExpr basically have the same

[PATCH] D143112: [clang] Support parsing comments without ASTContext

tom-anders added a comment. (ping) Does this make sense or are more adjustments needed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143112/new/ https://reviews.llvm.org/D143112 ___ cfe-commits mailing

[PATCH] D147847: [clangd] Hover: Add CalleeArgInfo for constructor expressions

This revision was automatically updated to reflect the committed changes. Closed by commit rGcb133a4629a5: [clangd] Hover: Add CalleeArgInfo for constructor expressions (authored by tom-anders). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147847/ne

[PATCH] D134130: [clangd] Add doxygen parsing for Hover [1/3]

tom-anders added a comment. Thanks for the detailed feedback! Unfortunately, I’m sick right now, so I probably won’t be able to give a detailed answer until after Christmas. In D134130#3992116 , @kadircet wrote: > Happy to move the discussion to some ot

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders marked 12 inline comments as done. tom-anders added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:314 // (these tend to be vague, like Type or Unknown) +// - Resolved tokens (i.e. without the "dependent-name" modifier) with kind +//

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders updated this revision to Diff 368177. tom-anders marked 3 inline comments as done. tom-anders added a comment. - Apply suggested renaming and fix nits - Use a map for extra modifiers instead of abusing conflict resolution - Use ArrayRef instead of callback - Add FIXME for handling CXXOp

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders updated this revision to Diff 368179. tom-anders added a comment. - Remove accidentally added empty line Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108320/new/ https://reviews.llvm.org/D108320 Files: clang-tools-extra/clangd/Semant

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders updated this revision to Diff 368181. tom-anders added a comment. - clang-format once again Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108320/new/ https://reviews.llvm.org/D108320 Files: clang-tools-extra/clangd/SemanticHighlightin

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders added a comment. @sammccall @nridge Thanks for the positive feedback, let me know if I missed any of your suggestions! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108320/new/ https://reviews.llvm.org/D108320 _

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders marked 5 inline comments as done. tom-anders added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:470 std::vector Tokens; + std::map> ExtraModifiers; const HeuristicResolver *Resolver; nridge wrote: > Looking at e

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders updated this revision to Diff 369249. tom-anders marked an inline comment as done. tom-anders added a comment. - Use llvm::SmallVector, add more FIXME, remove old commented out test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108320/new

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders added a comment. Yeah, I‘d need someone to commit this for me. About the std::map thing: let’s proceed as suggested and have @sammccall take look at it himself after :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108320/new/ https://

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

tom-anders added a comment. I’d say commit as is and have a look at the SmallVector -> unit32_t thing together with the std::map issue afterwards Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108320/new/ https://reviews.llvm.org/D108320 _

<    1   2