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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.


================
Comment at: clang-tools-extra/clangd/SemanticHighlight.cpp:1
+#include "SemanticHighlight.h"
+
----------------
Header is missing.


================
Comment at: clang-tools-extra/clangd/SemanticHighlight.cpp:32
+  void addSymbol(Decl *D, SemanticScope Scope) {
+    auto Loc = D->getLocation();
+    SemanticSymbol S;
----------------
Please don't use auto if type is not spelled explicitly.


================
Comment at: clang-tools-extra/clangd/SemanticHighlight.cpp:34
+    SemanticSymbol S;
+    auto LSPLoc = sourceLocToPosition(SM, Loc);
+
----------------
Please don't use auto if type is not spelled explicitly.


================
Comment at: clang-tools-extra/clangd/SemanticHighlight.cpp:99
+void write32be(uint32_t I, llvm::raw_ostream &OS) {
+  char Buf[4];
+  llvm::support::endian::write32be(Buf, I);
----------------
May be std::array is better way to do this? Same below.


================
Comment at: clang-tools-extra/clangd/SemanticHighlight.cpp:135
+  Collector.TraverseAST(AST);
+  auto Symbols = Collector.getSymbols();
+  std::vector<LineHighlight> Lines;
----------------
Please don't use auto if type is not spelled explicitly.


================
Comment at: clang-tools-extra/clangd/SemanticHighlight.h:2
+//===--- SemanticHighlight.h - Generating highlights from the AST
+//-----*- C++ -*-===//
+//
----------------
Please merge with previous line. If it doesn't fit, remove some - and =.


================
Comment at: clang-tools-extra/clangd/SemanticHighlight.h:14
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SEMANTICSYMBOLASTCOLLECTOR_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SEMANTICSYMBOLASTCOLLECTOR_H
----------------
Please separate with empty line.


================
Comment at: clang-tools-extra/clangd/SemanticHighlight.h:35
+struct SemanticSymbol {
+  SemanticSymbol() {}
+  SemanticSymbol(SemanticScope Scope, Position StartPosition, unsigned int Len)
----------------
Please use = default;


================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightTests.cpp:2
+//===-- SemanticSymbolASTCollectorTests.cpp - SemanticSymbolASTCollector tests
+//------------------*- C++ -*-===//
+//
----------------
Please merge with previous line. If it doesn't fit, remove some - and =.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63559/new/

https://reviews.llvm.org/D63559