[PATCH] D67496: [clangd] Collect macros in the preamble region of the main file

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 23 05:12:50 PDT 2019


ilya-biryukov added a comment.

Sorry for the delay



================
Comment at: clang-tools-extra/clangd/Macro.h:59
+
+    if (auto Range = getTokenRange(SM, LangOpts, MacroNameTok.getLocation())) {
+      MainFileMacros.push_back(
----------------
Converting here is too early, could we keep this conversion in syntax highlighting code?
Keeping source locations here is enough.


================
Comment at: clang-tools-extra/clangd/Macro.h:67
+  bool InMainFile = true;
+  std::vector<MainFileMacro> &MainFileMacros;
+};
----------------
Could we model in a way that avoids duplicating macro names on each occurrence?
We had `StringSet Names` and `vector<SourceLocation> Locations`, let's keep it in the same way.

We could group this into a struct to reduce boilerplate of transferring it around, obviously
```
struct MainFileMacros {
  StringSet Names;
  vector<SourceLocation> Locations;
};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67496





More information about the cfe-commits mailing list