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

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 12 05:30:51 PDT 2019


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.isWrittenInMainFile(Loc))
----------------
nit: the `)` is not match.


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:237
+    // file for a class) from another file)
+    if (!SM.isWrittenInMainFile(Loc))
+      return;
----------------
nit: clangd has its own `isInsideMainFile`, please use it.


================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:57
+void checkHighlightings(llvm::StringRef Code,
+                        std::vector<std::pair<llvm::StringRef, llvm::StringRef>>
+                            AdditionalFiles = {}) {
----------------
nit: maybe just `std::vector<std::pair</*FileName*/llvm::StringRef, /*FileContent*/llvm::StringRef>>`, it is clearer and would save you a long comment above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66083





More information about the cfe-commits mailing list