[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/

[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] D63559: [clangd] Added functionality for getting semantic highlights for variable and function declarations

jvikstrom updated this revision to Diff 206630. jvikstrom marked 6 inline comments as done. jvikstrom added a comment. Renamed types to follow LSP. Also renamed files. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63559/new/

[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

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

jvikstrom marked an inline comment as done. jvikstrom added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlight.h:17 + +enum class SemanticHighlightKind { + Variable, hokein wrote: > LSP proposal is using `Highlighting` rather than

[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

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

jvikstrom added inline comments. Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightTests.cpp:47 +TEST(SemanticTokenCollector, GetsCorrectTokens) { + const char *TestCases[] = { + R"cpp( hokein wrote: > the code seems not clang-format, could

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

jvikstrom updated this revision to Diff 206618. jvikstrom marked 11 inline comments as done. jvikstrom added a comment. - [clangd] Added functionality for getting semantic highlights for variable and function declarations Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[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] D63559: [clangd] Added functionality for getting semantic highlights for variable and function declarations

jvikstrom added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlight.cpp:28 + : Ctx(AST.getASTContext()), SM(AST.getSourceManager()) { +Ctx.setTraversalScope(AST.getLocalTopLevelDecls()); + } hokein wrote: > I'd move this line to

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

jvikstrom updated this revision to Diff 206593. jvikstrom marked 24 inline comments as done. jvikstrom added a comment. Fixed a bunch of small comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63559/new/ https://reviews.llvm.org/D63559

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

jvikstrom updated this revision to Diff 206450. jvikstrom added a comment. Made SemanticTokenCollector skip Decls not in the main file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63559/new/ https://reviews.llvm.org/D63559 Files:

[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;

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

jvikstrom added inline comments. Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightTests.cpp:57 + + checkTokensExists(Tokens, Variables, SemanticHighlightKind::Variable); + checkTokensExists(Tokens, Function, SemanticHighlightKind::Function);

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

jvikstrom updated this revision to Diff 206393. jvikstrom marked 4 inline comments as done. jvikstrom added a comment. Changed tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63559/new/ https://reviews.llvm.org/D63559 Files:

[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),

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

jvikstrom updated this revision to Diff 206376. jvikstrom marked 3 inline comments as done. jvikstrom added a comment. Added header and empty line Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63559/new/ https://reviews.llvm.org/D63559 Files:

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

jvikstrom updated this revision to Diff 206375. jvikstrom added a comment. Made SemanticTokenCollector visible and removed getSemanticHighlights function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63559/new/ https://reviews.llvm.org/D63559