[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

hokein added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:266 elog("No reply to message {0}({1})", Method, ID); assert(false && "must reply to all calls!"); (*this)(llvm::make_error("server failed to reply",

[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

hokein updated this revision to Diff 212794. hokein marked 2 inline comments as done. hokein edited the summary of this revision. hokein added a comment. Herald added a subscriber: jfb. - adjust ReplyOnce assertion logic - add more tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

hokein updated this revision to Diff 212796. hokein added a comment. some tweaks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65387/new/ https://reviews.llvm.org/D65387 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp clang-tools-extra/cla

[PATCH] D65637: [clangd] [WIP] Semantic highlighting prototype for the vscode extension.

hokein added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/TextMate.ts:20 +const split = scope.split('.'); +while(split.length > 0) { +split.splice(-1) jvikstrom wrote: > I'm pretty sure th

[PATCH] D65657: [clangd][vscode] clang-format the the extension code.

hokein created this revision. hokein added reviewers: ilya-biryukov, sammccall, jvikstrom. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. As we are going to grow the extension in the near future, it is time to formalize the TS code format/style of ou

[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

hokein added a comment. the change looks good to me, leave the final approval to Ilya as he knows more context. Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:116 + auto AST = TU.build(); + EXPECT_THAT(AST.getLocalTopLevelDecls(), ElementsAre(DeclNamed("f

[PATCH] D65657: [clangd][vscode] clang-format the the extension code.

This revision was automatically updated to reflect the committed changes. Closed by commit rL367684: [clangd][vscode] clang-format the the extension code. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https:/

[PATCH] D65735: [AST] Fix RecursiveASTVisitor visiting implicit constructor initializers.

hokein added a comment. I think this is a reasonable fix, just a few comments on the test. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp:26 + bool TraverseConstructorInitializer(CXXCtorInitializer *Init) { +if (Init->getSourceLocati

[PATCH] D65637: [clangd] [WIP] Semantic highlighting prototype for the vscode extension.

hokein added a comment. In D65637#1613692 , @nridge wrote: > Do you plan to support text decoration options other than color, e.g. bold / > underline / italic? I think we'd just support color, and we don't have further plan to support richer renderings

[PATCH] D65735: [AST] Fix RecursiveASTVisitor visiting implicit constructor initializers.

hokein added inline comments. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/ImplicitCtorInitializer.cpp:2 +//===- unittest/Tooling/RecursiveASTVisitorTests/ImplicitCtorInitializer.cpp +//-===// +// we should make them one line even it exceeds 80 c

[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

hokein added a comment. thanks for the detailed comments! Comment at: clang-tools-extra/clangd/test/request-reply.test:22 +--- +{"jsonrpc":"2.0","id":0,"result":{"applied":false}} +# CHECK: "code": -32001, sammccall wrote: > please use increasing IDs and

[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

hokein updated this revision to Diff 213304. hokein marked 24 inline comments as done. hokein added a comment. address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65387/new/ https://reviews.llvm.org/D65387 Files: clang-tools-extra/cl

[PATCH] D65735: [AST] Fix RecursiveASTVisitor visiting implicit constructor initializers.

hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/ImplicitCtorInitializer.cpp:26 + VisitedImplicitInitializer = true; +Match("initializer", Init->get

[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

hokein updated this revision to Diff 213334. hokein marked 8 inline comments as done. hokein added a comment. address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65387/new/ https://reviews.llvm.org/D65387 Files: clang-tools-extra/cla

[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

hokein added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:205 + // Return a call id of the request. + int bindReply(Callback Reply) { +llvm::Optional>> OldestCB; sammccall wrote: > nit: I think this function could return json::V

[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

This revision was automatically updated to reflect the committed changes. Closed by commit rL367845: [clangd] Add a callback mechanism for handling responses from client. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to c

[PATCH] D65738: [clangd] Added a TextMate theme parser that updates when the current theme changes.

hokein added a comment. Haven't looked at the patch in details. Looks like the patch is doing different things, and the test just covers a small set of the code. 1. find and parse the theme definition files `json` ; 2. define related data structures to save the TM scopes and do the transformat

[PATCH] D65738: [clangd] Added a TextMate theme parser that updates when the current theme changes.

hokein added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/TextMate.ts:6 + +export namespace SemanticHighlighting { +interface ScopeColorRule { I think we should not use the namespace in typescript. The `namespace` in TS refers

[PATCH] D65738: [clangd] Added a TextMate theme parser that updates when the current theme changes.

hokein added a comment. thanks, looks simpler now, just a few more nits. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:7 +interface TokenColorRule { + scope: string, textColor: string, +} nit: we need documentation fo

[PATCH] D65738: [clangd] Added a TextMate theme parser that updates when the current theme changes.

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. mostly good, please update the patch description. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:6 + +interface TokenColorRule { +

[PATCH] D65928: [clangd] Added an early return from VisitMemberExpr in SemanticHighlighting if the MemberLoc is invalid.

hokein added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:56 +if (ME->getMemberLoc().isInvalid()) + // If the member is a conversion operator the loc will be invalid. This + // causes the addToken function to emit a lot of error log

[PATCH] D65936: [clangd] Use raw rename functions to implement the rename.

hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: kadircet, jfb, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. The API provided by refactoring lib doesn't provide enough flexibility to get clangd's rename to behave as we expect

[PATCH] D65928: [clangd] Added an early return from VisitMemberExpr in SemanticHighlighting if the MemberLoc is invalid.

hokein added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:56 +if (isa(MD)) + // If the member is a conversion operator the loc will be invalid. This + // causes the addToken function to emit a lot of error logs about trying

[PATCH] D65936: [clangd] Use raw rename functions to implement the rename.

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

[PATCH] D65936: [clangd] Use raw rename functions to implement the rename.

hokein added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:175 +// Currently, we only support normal rename (one range) for C/C++. +// FIXME: support multiple-range rename for objective-c methods. +if (Rename.getNameRanges().size() > 1) ---

[PATCH] D65928: [clangd] Added an early return from VisitMemberExpr in SemanticHighlighting if the MemberLoc is invalid.

hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:56 +if (isa(MD)) + // If the member is a conversion operator the loc will be invalid. This + /

[PATCH] D65928: [clangd] Added an early return from VisitMemberExpr in SemanticHighlighting if the MemberLoc is invalid.

hokein added a comment. please update the patch description. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65928/new/ https://reviews.llvm.org/D65928 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D65926: [clangd] Fixed printTemplateSpecializationArgs not printing partial variable specialization arguments.

hokein added inline comments. Comment at: clang-tools-extra/clangd/unittests/ASTTests.cpp:40 +TEST(PrintTemplateSpecializationArgs, PrintsTemplateArgs) { + TestTU TU; looks like the related tests are in `PrintASTTests.cpp`, could you move the test there? Re

[PATCH] D65938: [AST] No longer visiting CXXMethodDecl bodies created by compiler when method was default created.

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. looks good from my side. Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:262 + struct $Class[[A]] { + $Class[[B]] $Field[[BB]]; +

[PATCH] D65943: [clangd] Added semantic highlighting support for primitives.

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. nice, just a nit. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:138 +// Builtins must be special cased as they do not have a TagDecl. +if (const auto

[PATCH] D65936: [clangd] Use raw rename functions to implement the rename.

hokein marked 3 inline comments as done. hokein added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:175 +// Currently, we only support normal rename (one range) for C/C++. +// FIXME: support multiple-range rename for objective-c methods. +i

[PATCH] D65936: [clangd] Use raw rename functions to implement the rename.

This revision was automatically updated to reflect the committed changes. Closed by commit rL368429: [clangd] Use raw rename functions to implement the rename. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: ht

[PATCH] D65936: [clangd] Use raw rename functions to implement the rename.

hokein updated this revision to Diff 214349. hokein marked an inline comment as done. hokein added a comment. Add a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65936/new/ https://reviews.llvm.org/D65936 Files: clang-tools-extra/clangd

[PATCH] D65998: [clangd] Added the vscode SemanticHighlighting feature code but did not enable it in the client.

hokein added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:37 +// experimental semantic highlighting. +export class Feature implements vscodelc.StaticFeature { + scopes: string[]; nit: name it `Semantic

[PATCH] D65996: [clangd] Highlighting auto variables as the deduced type.

hokein added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:153 +// "auto" keyword other than using an offset. +Loc = Loc.getLocWithOffset(9); + Deduced.getTypePtr()->dump(); The heuristic using here is fragile

[PATCH] D65996: [clangd] Highlighting auto variables as the deduced type.

hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:143 + const auto Deduced = AT->getDeducedType(); + if (Deduced.isNull()) +return true; ---

[PATCH] D66074: [clangd] Drop diags from non-written #include.

hokein created this revision. hokein added a reviewer: ilya-biryukov. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. This would fix that we show weird diagnostics on random lines of the main file. Repository: rG LLVM Github Monorepo https://revi

[PATCH] D66074: [clangd] Drop diags from non-written #include.

hokein updated this revision to Diff 214590. hokein marked an inline comment as done. hokein added a comment. Add a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66074/new/ https://reviews.llvm.org/D66074 Files: clang-tools-extra/clangd

[PATCH] D66074: [clangd] Drop diags from non-written #include.

This revision was automatically updated to reflect the committed changes. Closed by commit rL368549: [clangd] Drop diags from non-written #include. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://review

[PATCH] D66074: [clangd] Drop diags from non-written #include.

hokein marked an inline comment as done. hokein added a subscriber: hans. hokein added a comment. @hans is there still any chance to merge this patch into the release? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66074/new/ https://reviews.llvm.org/D66074

[PATCH] D65998: [clangd] Added the vscode SemanticHighlighting feature code but did not enable it in the client.

hokein added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:8 +// The information clangd sends when highlightings should be updated. +interface SemanticHighlightingParams { + // The information about the text document wh

[PATCH] D66083: [clangd] Remove highlightings coming from non topLevelDecls from included files.

hokein accepted this revision. hokein added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:236 +// topLevelDecls. (example: method declarations being included from another +// file for a class) from another file) +if (!SM.isWrittenInMai

[PATCH] D65998: [clangd] Added the vscode SemanticHighlighting feature code but did not enable it in the client.

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. mostly good with a few nits. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:9 +// Parameters for the semantic highlighting (server-s

[PATCH] D65856: [clangd] Added class for mapping TokenColorRules to their associated clangd TextMate scope

hokein added a comment. I think we could make the layering clearer: - we now have a list of theme color rules, and scope names provided by clangd; and we we want to find the best match theme rule for a particular clangd scope; - we could define a function like `getBestThemeRule(clangd_scope_name

[PATCH] D66074: [clangd] Drop diags from non-written #include.

hokein added a comment. In D66074#1626856 , @hans wrote: > In D66074#1624885 , @hokein wrote: > > > @hans is there still any chance to merge this patch into the release? > > > I tried to merge this to release_90, bu

[PATCH] D66074: [clangd] Drop diags from non-written #include.

hokein added a comment. In D66074#1626932 , @hans wrote: > > thanks for the information, I suspect that this patch may depend on > > rL367303 (which is not merged in the > > release), I will double check. > > Yes, that

[PATCH] D65856: [clangd] Added class for mapping TokenColorRules to their associated clangd TextMate scope

hokein added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:65 +const name = +vscode.workspace.getConfiguration('workbench').get('colorTheme'); +if (typeof name != 'string') { maybe just `

[PATCH] D65856: [clangd] Added class for mapping TokenColorRules to their associated clangd TextMate scope

hokein added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:131 +this.rules.forEach((rule) => { + if (rule.scope.length <= scope.length && + scope.substr(0, rule.scope.length) === rule.scope && -

[PATCH] D65856: [clangd] Added class for mapping TokenColorRules to their associated clangd TextMate scope

hokein added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:128 + // Find the rule wich is the longest prefix of scope. + if (rule.scope.length <= scope.length && + scope.substr(0, rule.scope.length) ==

[PATCH] D66209: Improved the doc comment for getCommentsInFile

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

[PATCH] D66211: [clangd][vscode] Surface the error when applying tweaks fails

hokein created this revision. hokein added a reviewer: ilya-biryukov. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. The current behavior for a failed request is just to log it in the output panel. When applyTweak fails for whatever reason, users usu

[PATCH] D65856: [clangd] Added class for mapping TokenColorRules to their associated clangd TextMate scope

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. thanks, looks good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65856/new/ https://reviews.llvm.org/D65856

[PATCH] D66215: [clangd] Print qualifiers of out-of-line definitions in document outline

hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/AST.cpp:106 + +/// Returns a nested name specifier if \p ND refers to a an out-of-line +/// definition. `if .. out-

[PATCH] D66211: [clangd][vscode] Surface the error when applying tweaks fails

hokein added a comment. In D66211#1629112 , @ilya-biryukov wrote: > How do they look in practice? In particular, should we add more context > information (either in clangd or in the VSCode extension) to those errors, > now that we know they are shown to

[PATCH] D66211: [clangd][vscode] Surface the error when applying tweaks fails

hokein updated this revision to Diff 215107. hokein marked an inline comment as done. hokein added a comment. avoid re-implementing of the method. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66211/new/ https://reviews.llvm.org/D66211 Files: cl

[PATCH] D66211: [clangd][vscode] Surface the error when applying tweaks fails

hokein added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:109 +// Keep the default behavior. +if (error instanceof vscodelc.ResponseError && +error.code === vscodelc.ErrorCodes.RequestCancelled) -

[PATCH] D66211: [clangd][vscode] Surface the error when applying tweaks fails

This revision was automatically updated to reflect the committed changes. Closed by commit rL368851: [clangd][vscode] Surface the error when applying tweaks fails (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D66343: [clangd] Simplify code of ClangdLSPServer::onCommand

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

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

hokein created this revision. hokein added a reviewer: ilya-biryukov. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. The test didn't test anything actually -- it used "[]" as annotation which should be "[[]]". This patch also fixes a bug in XRef wh

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

hokein added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:389 llvm::sort(References, [](const Reference &L, const Reference &R) { - return std::tie(L.Loc, L.CanonicalTarget, L.Role) < - std::tie(R.Loc, R.CanonicalTarget, R.Role); + re

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

hokein marked an inline comment as done. hokein added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:389 llvm::sort(References, [](const Reference &L, const Reference &R) { - return std::tie(L.Loc, L.CanonicalTarget, L.Role) < - std::tie(R.L

[PATCH] D66219: [clangd] Added a colorizer to the vscode extension.

hokein added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:144 +// sends. +export class Colorizer { + private files: Map> = new Map(); The generic `T` doesnt seem to be used? Regarding the name, the w

[PATCH] D66406: [clangd] Update themeRuleMatcher when color theme changes in vscode extension.

hokein added a comment. the change looks good, but I'd wait until D66219 is finished. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66406/new/ https://reviews.llvm.org/D66406 ___

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

hokein marked an inline comment as done. hokein added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:389 llvm::sort(References, [](const Reference &L, const Reference &R) { - return std::tie(L.Loc, L.CanonicalTarget, L.Role) < - std::tie(R.L

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

hokein marked an inline comment as done. hokein added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:389 llvm::sort(References, [](const Reference &L, const Reference &R) { - return std::tie(L.Loc, L.CanonicalTarget, L.Role) < - std::tie(R.L

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

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

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

This revision was automatically updated to reflect the committed changes. Closed by commit rL369387: [clangd] Fix one testcase in XRefsTests. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm

[PATCH] D66478: [clangd] Ignore implicit conversion-operator nodes in find refs.

hokein created this revision. hokein added a reviewer: ilya-biryukov. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66478 Files: clang-tools-extra/clangd/XRefs.cpp clang-tools-ext

[PATCH] D63559: [clang-tidy] Added functionality for getting semantic highlights for variable and function declarations

hokein added a comment. Thanks, I think we can simplify the interface further, see my comments inline. The boundary of this patch seems unclear, currently it contains C++ APIs, and some implementations for semantic highlighting LSP. I think we should merely focus on the C++ APIs in this patch.

[PATCH] D63426: [clangd] Narrow rename to local symbols.

hokein updated this revision to Diff 205963. hokein added a comment. Improve the tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63426/new/ https://reviews.llvm.org/D63426 Files: clang-tools-extra/clangd/ClangdServer.cpp clang-tools-extra

[PATCH] D63270: [clangd] Add include-mapping for C symbols.

This revision was automatically updated to reflect the committed changes. Closed by commit rL364044: [clangd] Add include-mapping for C symbols. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.l

[PATCH] D63426: [clangd] Narrow rename to local symbols.

hokein updated this revision to Diff 206185. hokein added a comment. More cleanups. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63426/new/ https://reviews.llvm.org/D63426 Files: clang-tools-extra/clangd/ClangdServer.cpp clang-tools-extra/cla

[PATCH] D63426: [clangd] Narrow rename to local symbols.

hokein updated this revision to Diff 206184. hokein marked 11 inline comments as done. hokein added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63426/new/ https://reviews.llvm.org/D63426 Files: clang-tools-e

[PATCH] D63714: [clangd] Cleanup the duplicated getTokenRange.

hokein created this revision. hokein added a reviewer: kadircet. Herald added subscribers: arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Also lift it to SourceCode.h, so that it can be used in other places (semantic code highlighting). Repository: rG LLVM Github Mon

[PATCH] D63559: [clang-tidy] Added functionality for getting semantic highlights for variable and function declarations

hokein added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlight.cpp:11 + +// Collects all semantic symbols in an ASTContext. Symbols on line i are always +// in front of symbols on line i+1 The comment is out of date now. C

[PATCH] D63714: [clangd] Cleanup the duplicated getTokenRange.

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

[PATCH] D63714: [clangd] Cleanup the duplicated getTokenRange.

This revision was automatically updated to reflect the committed changes. Closed by commit rL364280: [clangd] Cleanup the duplicated getTokenRange. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://review

[PATCH] D63559: [clangd] Added functionality for getting semantic highlights for variable and function declarations

hokein added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlight.cpp:43 + + unsigned int Len = + clang::Lexer::MeasureTokenLength(D->getLocation(), SM, Ctx.getLangOpts()); we can reuse the getTokenRange() function (in SourceCode.h), you

[PATCH] D63426: [clangd] Narrow rename to local symbols.

This revision was automatically updated to reflect the committed changes. Closed by commit rL364283: [clangd] Narrow rename to local symbols. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm

[PATCH] D63759: [clangd] Don't rename the namespace.

hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Also fix a small bug -- the extra argument "-xc++" doesn't overwrite the language if the argument is present after the fil

[PATCH] D63759: [clangd] Don't rename the namespace.

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

[PATCH] D63759: [clangd] Don't rename the namespace.

hokein added inline comments. Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:137 +// Parsing the .h file as C++ include. +TU.ExtraArgs.push_back("-xobjective-c++-header"); auto AST = TU.build(); sammccall wrote: > (why this change?) f

[PATCH] D63759: [clangd] Don't rename the namespace.

hokein updated this revision to Diff 206437. hokein marked an inline comment as done. hokein added a comment. Verify the error message in the test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63759/new/ https://reviews.llvm.org/D63759 Files: c

[PATCH] D63759: [clangd] Don't rename the namespace.

hokein added inline comments. Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:137 +// Parsing the .h file as C++ include. +TU.ExtraArgs.push_back("-xobjective-c++-header"); auto AST = TU.build(); sammccall wrote: > hokein wrote: > > sa

[PATCH] D63559: [clangd] Added functionality for getting semantic highlights for variable and function declarations

hokein added a comment. the test looks better now, another round of reviews, most are nits. Comment at: clang-tools-extra/clangd/SemanticHighlight.cpp:19 +class SemanticTokenCollector +: private RecursiveASTVisitor { + friend class RecursiveASTVisitor; nit

[PATCH] D63759: [clangd] Don't rename the namespace.

This revision was automatically updated to reflect the committed changes. Closed by commit rL364392: [clangd] Don't rename the namespace. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org

[PATCH] D63559: [clangd] Added functionality for getting semantic highlights for variable and function declarations

hokein added a comment. much clearer now, a few more nits. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:69 bool HandleTopLevelDecl(DeclGroupRef DG) override { +//FIXME: There is an edge case where this will still get decls from include files. for (Decl *D :

[PATCH] D63784: [clang-tidy] Fix ClangTidyTest to initialize context before checks.

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. looks good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63784/new/ https://reviews.llvm.org/D63784

[PATCH] D63559: [clangd] Added functionality for getting semantic highlights for variable and function declarations

hokein added a comment. thanks mostly good. Thinking more about the name, we should align with the LSP proposal, see my comments inline. Comment at: clang-tools-extra/clangd/SemanticHighlight.cpp:20 +// Collects all semantic tokens in an ASTContext. +class SemanticTokenCollect

[PATCH] D63559: [clangd] Added functionality for getting semantic highlights for variable and function declarations

hokein added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlight.h:17 + +enum class SemanticHighlightKind { + Variable, jvikstrom wrote: > hokein wrote: > > LSP proposal is using `Highlighting` rather than `Highlight`, let's align > > with t

[PATCH] D63559: [clangd] Added functionality for getting semantic highlights for variable and function declarations

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Thanks, looks good. Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:1 +//==- SemanticHighlightingTest.cpp - SemanticHighlightTest tests-*- C++ -*

[PATCH] D63818: [clangd] Collect the refs when the main file is header.

hokein created this revision. hokein added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Previously, we only collect refs of the symbols which are declared in the preamble and referenced in the main file, it w

[PATCH] D63817: [clangd] No longer getting template instantiations from header files in Main AST.

hokein added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:74 + if (!SM->isInMainFile(D->getLocation())) +// This decl comes from another file and should not be included in the you could get SourceManager from `D->getASTConte

[PATCH] D63821: [clangd] Added C++ API code for semantic highlighting

hokein added a comment. thanks, it is much clearer now. Could you please make the commit message be more specific what this patch does? C++ APIs is too generous, (we already have C++ APIs and data structures for semantic highlightings which are in `SemanticHighlighting.h`).

[PATCH] D63872: [clangd] Fix a case where we fail to detect a header-declared symbol in rename.

hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Failing case: #include "foo.h" void fo^o() {} getRenameDecl() returns the decl of the symbol under the cursor (which

[PATCH] D63874: [clangd] No need to setTraversalScope in SemanticHighlighting.

hokein created this revision. hokein added a reviewer: jvikstrom. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. We have already set it when the AST is being built, and setting TraversalScope is not free (it will clear the cache, which

[PATCH] D63874: [clangd] No need to setTraversalScope in SemanticHighlighting.

This revision was automatically updated to reflect the committed changes. Closed by commit rL364528: [clangd] No need to setTraversalScope in SemanticHighlighting. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D63821: [clangd] Added C++ API code for semantic highlighting

hokein added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:60 + void + onHighlightingsReady(PathRef File, + std::vector Highlightings) override; nit: you can remove this override, since we have provided an empty d

[PATCH] D63559: [clangd] Added functionality for getting semantic highlights for variable and function declarations

hokein closed this revision. hokein added a comment. the patch was landed in rL364421 , not sure why it was not associated with this review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63559/new/ https://review

[PATCH] D63872: [clangd] Fix a case where we fail to detect a header-declared symbol in rename.

hokein updated this revision to Diff 206844. hokein marked 2 inline comments as done. hokein added a comment. Address comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63872/new/ https://reviews.llvm.org/D63872 Files: clang-tools-extra/clan

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