[PATCH] D70008: [clangd] Store xref for Macros in ParsedAST.
Haojian Wu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 11 03:03:39 PST 2019
hokein added inline comments.
================
Comment at: clang-tools-extra/clangd/CollectMacros.h:29
std::vector<Range> Ranges;
+ llvm::DenseMap<SymbolID, std::set<Location>> MacroRefs;
};
----------------
I think the `Ranges` and `MacrosRefs` have a lot of duplications, it is wasteful to store a same range twice. We don't need the MainFilePath, the callers should know the corresponding file path for the AST. Something like `llvm::DenseMap<SymbolID, std::vector<Range>> Refs;` should be enough.
symbol id for macro generation is required the definition location, but only defined macros have it, we may need an extra `vector` to store ranges for undefined macros.
```
#ifndef abc // there is no macro info* for abc, so getSymbolID will fail, but we still want it, e.g. in semantic highlighting.
#endif
```
There may be more tricky cases, it would be nice to have in the test.
```
#define abc 1
#undef abc
#define abc 2
#undef abc
```
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:898
+ // Handle macros.
+ if (auto Macro = locateMacroAt(Loc, AST.getPreprocessor())) {
+ if (auto MacroSID = getSymbolID(Macro->Name, Macro->Info, SM)) {
----------------
this is not completed, it only shows the macro refs in main file, we still need to query the index to get the rest (but our index doesn't collect macro at the moment). I'd prefer not doing this in this patch until our index is ready.
For testing, I think we can create a new test file for `CollectMainFileMacros`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70008/new/
https://reviews.llvm.org/D70008
More information about the cfe-commits
mailing list