[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz added a comment. Thank you! Since the documentation shouldn't be available until LLVM 17, I'm slowly working on it. Let me update the PR in this weekend :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148489/new/

[PATCH] D151190: [clangd] Do not end inactiveRegions range at position 0 of line

daiyousei-qz added inline comments. Comment at: clang-tools-extra/clangd/unittests/ClangdTests.cpp:1343 #undef CMDMACRO $inactive3[[#ifdef CMDMACRO int inactiveInt2; hokein wrote: > While this patch is an improvement, I wonder we should move it further. >

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz marked an inline comment as done. daiyousei-qz added a comment. Addressed the remaining comments. Thanks everyone for helping review and improve this patch! Comment at: clang-tools-extra/clangd/InlayHints.cpp:802 +Position HintStart = sourceLocToPosition(SM,

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz updated this revision to Diff 525922. daiyousei-qz marked 8 inline comments as done. daiyousei-qz added a comment. - Address remaining comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150635/new/ https://reviews.llvm.org/D150635

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz updated this revision to Diff 525448. daiyousei-qz added a comment. - Fix reversed logic of including inactive region token Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148489/new/ https://reviews.llvm.org/D148489 Files:

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:578 +constexpr unsigned HintMinLineLimit = 2; +constexpr unsigned HintMaxLengthLimit = 50; + nridge wrote: > daiyousei-qz wrote: > > sammccall wrote: > > > We

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz added a comment. Sorry for the inactivity. I have replace the parameter with const ref. By the way, I also removed the existing `IncludeInactiveRegionTokens` flag and uses this filter instead. Please help review the change. Thanks! Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz updated this revision to Diff 525421. daiyousei-qz added a comment. - Address review comment and remove a unnecessary flag Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148489/new/ https://reviews.llvm.org/D148489 Files:

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz added a comment. Addressed review comments except for those we don't have an agreement yet. Comment at: clang-tools-extra/clangd/InlayHints.cpp:578 +constexpr unsigned HintMinLineLimit = 2; +constexpr unsigned HintMaxLengthLimit = 50; +

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz updated this revision to Diff 525418. daiyousei-qz marked 9 inline comments as done. daiyousei-qz added a comment. - Move computeBlockEndHintRange - Correct label length limit logic - Correct line number computation for '{' - Address other review comments Repository: rG LLVM

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz added a comment. Yeah, makes sense. Thank you for your insight! I missed the fact that this time include AST build time. I manually injected a time region into `inlayHints`, here's part of logs printed for scrolling "offsetToPosition" build: I[23:36:43.055] <--

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz added a comment. I'm not sure but 18ms is definitely wrong here, especially for a debug build. In your log, even semantic highlighting builds in 60ms, which looks incorrect. I tried again with TOT clangd and noticed the following pattern. - If I scroll with vscode, inlay hints

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:616 +Position HintStart = sourceLocToPosition(SM, BraceRange.getEnd()); +Position HintEnd = {HintStart.line, +HintStart.character + sammccall

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz added a comment. In D148489#4362990 , @nridge wrote: > (Do you plan to address the last comment before I merge?) Oh, okay. Let me address that. Didn't see that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz added a comment. Yes please. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148489/new/ https://reviews.llvm.org/D148489 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz marked an inline comment as done. daiyousei-qz added a comment. Addressed most review comments. I'm currently using the name "BlockEnd" as suggested. Though I'm proposing a name like "DeclBlockEnd" to make it clearer. What do you think? Comment at:

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz updated this revision to Diff 524183. daiyousei-qz marked 19 inline comments as done. daiyousei-qz added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150635/new/ https://reviews.llvm.org/D150635

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz added a comment. For a quick peek to the hint: F27483170: image.png Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150635/new/ https://reviews.llvm.org/D150635

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz added a comment. @zyounan np! Feel free to leave your review comments! @sammccall Thank you for you insightful and detailed review! I addressed many review comments in the updated patch and left things I'm not very sure. Regarding to **Naming**, I agree that `EndDefinitionComment`

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz updated this revision to Diff 522898. daiyousei-qz marked 16 inline comments as done. daiyousei-qz added a comment. - addressed many review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150635/new/

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz added a comment. Requesting for some advice Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:1666 + +$anon[[struct { + int x; This is unwanted behavior from my understanding. Do you guys have any insight how we could fix

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz updated this revision to Diff 522434. daiyousei-qz added a comment. minor naming fix2 (last fix breaks builds) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150635/new/ https://reviews.llvm.org/D150635 Files:

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

daiyousei-qz updated this revision to Diff 522432. daiyousei-qz edited the summary of this revision. daiyousei-qz added a comment. minor naming fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150635/new/ https://reviews.llvm.org/D150635 Files:

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

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

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz updated this revision to Diff 521977. daiyousei-qz added a comment. Fix format issue Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148489/new/ https://reviews.llvm.org/D148489 Files: clang-tools-extra/clangd/Config.h

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz updated this revision to Diff 521969. daiyousei-qz added a comment. Redo arc as some changes are missing from previous update. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148489/new/ https://reviews.llvm.org/D148489 Files:

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz updated this revision to Diff 521967. daiyousei-qz added a comment. 1. Updating D148489 : [clangd] Implement configs to stop clangd produce a certain semantic tokens # 2. Enter a brief description of the changes included in this update. 3. The

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz added a comment. Sorry, I'm a little occupied lately and don't have time to fix the test failure. I'll try to fix that in this week. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:400 + bool ActiveKindLookup[static_cast(HighlightingKind::LastKind)

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz added a comment. In D148489#4276634 , @kadircet wrote: > before you dive any deeper into the patch, could you give some reasoning > about why this is needed/useful? The discussion is at https://github.com/clangd/clangd/discussions/1598.

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz updated this revision to Diff 514530. daiyousei-qz added a comment. Add some comments to the config headers. Resubmit to retrigger build test since I cannot reproduce the failure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:1281 } +std::optional +highlightingKindFromString(llvm::StringRef Name) { I'm not sure if we should parse the enums here or in parsing configs using `EnumSwitch`.

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz updated this revision to Diff 514088. daiyousei-qz added a comment. Fix an issue where the previous change is lost. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148489/new/ https://reviews.llvm.org/D148489 Files:

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz updated this revision to Diff 514087. daiyousei-qz added a comment. Add a test in SemanticHighlightingTests.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148489/new/ https://reviews.llvm.org/D148489 Files:

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz updated this revision to Diff 514086. daiyousei-qz added a comment. Fix unittest Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148489/new/ https://reviews.llvm.org/D148489 Files: clang-tools-extra/clangd/Config.h

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

daiyousei-qz added a comment. Please hold off from reviewing this. I have been figuring out how arc tools work and the unittest of the current version is broken. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148489/new/

[PATCH] D148489: Implement configs to stop clangd produce a certain semantic tokens

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

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz added a comment. In D127082#4089377 , @nridge wrote: > @daiyousei-qz what is the current status of this patch? Is it ready to be > merged again? (If so, I can do that for you.) Honestly, I don't know. I looked at TOT llvm repo in github

[PATCH] D136033: Add an option to specify a workspece-level config file

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

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz added a comment. In D127082#3776173 , @vitalybuka wrote: > Looks like it breaks > https://lab.llvm.org/buildbot/#/builders/236/builds/206 > https://lab.llvm.org/buildbot/#/builders/237/builds/85 >

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz added a comment. In D127082#3770763 , @sammccall wrote: > Sorry for the delay :-( This looks great! > > It looks like this is your first patch, so you don't have commit access. I > can land the patch for you. > Can you provide an email

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz added a comment. @sammccall Hello, is there any update required by me? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127082/new/ https://reviews.llvm.org/D127082 ___ cfe-commits mailing

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz updated this revision to Diff 447962. daiyousei-qz added a comment. remove dedicated MacroExpansion variable (reuse Definition) added empty MACRO test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127082/new/

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1187 - if (!Definition.empty()) { + if (!Definition.empty() || !MacroExpansion.empty()) { Output.addRuler(); sammccall wrote: > Sorry, I think I wasn't clear about what I

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz marked 5 inline comments as done. daiyousei-qz added a comment. I have also moved the expansion out of the location checking branch. I think it's to find out if a macro actually has a definition which isn't always the case especially for special macros like `__FILE__`. Here's the

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz updated this revision to Diff 446041. daiyousei-qz added a comment. resolve review comment produce expansion even if definition isn't available Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127082/new/

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz updated this revision to Diff 444151. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127082/new/ https://reviews.llvm.org/D127082 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-extra/clangd/Hover.h

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz updated this revision to Diff 444150. daiyousei-qz added a comment. Herald added subscribers: Sanitizers, Enna1. Herald added a project: Sanitizers. fix patch context Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127082/new/

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz updated this revision to Diff 444147. daiyousei-qz added a comment. fix line end to LF Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127082/new/ https://reviews.llvm.org/D127082 Files: clang-tools-extra/clangd/Hover.cpp

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz added a comment. It looks like my inline comment wasn't submitted (didn't click the submit button in the bottom). Here's my old comment. Comment at: clang-tools-extra/clangd/Hover.cpp:1091 + + // Reformat Macro Expansion + if (!HI->MacroExpansion.empty()) {

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz updated this revision to Diff 442416. daiyousei-qz marked an inline comment as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127082/new/ https://reviews.llvm.org/D127082 Files: clang-tools-extra/clangd/Hover.cpp

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz updated this revision to Diff 441920. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127082/new/ https://reviews.llvm.org/D127082 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-extra/clangd/Hover.h

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz updated this revision to Diff 439592. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127082/new/ https://reviews.llvm.org/D127082 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-extra/clangd/Hover.h

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz updated this revision to Diff 439591. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127082/new/ https://reviews.llvm.org/D127082 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-extra/clangd/Hover.h

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz updated this revision to Diff 439590. daiyousei-qz added a comment. use a rebased patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127082/new/ https://reviews.llvm.org/D127082 Files: clang-tools-extra/clangd/Hover.cpp

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz added a comment. Sorry I'm a little occupied lately. Regarding to the order of `Definition` and `MacroExpansion`, I experimented putting expansion first and noticed the following: 1. Short definition and short expansion: order doesn't matter, it's always clear 2. Short definition

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz updated this revision to Diff 439585. daiyousei-qz added a comment. - add context to patch - use a fixed size buffer for expansion text Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127082/new/ https://reviews.llvm.org/D127082 Files:

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

daiyousei-qz added a comment. To elaborate on known situation where expansion isn't available: 1. expansion doesn't work on site of #define 2. expansion doesn't work for _Pragma(...) and alike 3. expansion doesn't work with builtin macro 4. expansion doesn't work for macros as arguments of

[PATCH] D125863: [clangd] Dont mark terminating PP-directives as skipped

daiyousei-qz added a comment. I have a similar change recently presented in https://github.com/clangd/clangd/issues/1181 that makes all conditional preprocessor lines active. Please refer to the change if that's helpful :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D126757: [AST] Fix clang RecursiveASTVisitor for definition of XXXTemplateSpecializationDecl

daiyousei-qz added a comment. Yes, please do so. I'm using the web interface since the arc tool isn't very friendly to Windows user :( Name: Qingyuan Zheng Email: qyzhe...@outlook.com Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126757/new/

[PATCH] D127082: Add Macro Expansion to Hover

daiyousei-qz created this revision. daiyousei-qz added reviewers: nridge, sammccall. Herald added subscribers: usaxena95, kadircet, arphaman. Herald added a project: All. daiyousei-qz requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber:

[PATCH] D126757: Fix clang RecursiveASTVisitor for definition of XXXTemplateSpecializationDecl

daiyousei-qz updated this revision to Diff 434196. daiyousei-qz marked an inline comment as done. daiyousei-qz added a comment. squashed commits. updated comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126757/new/

[PATCH] D126757: Fix clang RecursiveASTVisitor for definition of XXXTemplateSpecializationDecl

daiyousei-qz updated this revision to Diff 433590. daiyousei-qz added a comment. Herald added a subscriber: arphaman. Herald added a project: clang-tools-extra. added unit test moved redundant visit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D126757: Fix clang RecursiveASTVisitor for definition of XXXTemplateSpecializationDecl

daiyousei-qz created this revision. daiyousei-qz added reviewers: nridge, klimek. Herald added subscribers: usaxena95, kadircet. Herald added a project: All. daiyousei-qz requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang. The