sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Ah, this is a bit ugly... I'm not really happy with our representation of info
extracted from the preprocessor, but that's something to tackle another time.
Thanks for fixing this!
================
Comment at: clang-tools-extra/clangd/CollectMacros.h:31
// SourceManager from preamble is not available when we build the AST.
- llvm::DenseMap<SymbolID, std::vector<Range>> MacroRefs;
+ llvm::DenseMap<SymbolID, std::vector<std::pair<Range, bool>>> MacroRefs;
// Somtimes it is not possible to compute the SymbolID for the Macro, e.g. a
----------------
this pair should be a struct instead - MacroOccurrence?
================
Comment at: clang-tools-extra/clangd/CollectMacros.h:35
// highlighting.
std::vector<Range> UnknownMacros;
// Ranges skipped by the preprocessor due to being inactive.
----------------
this should probably also be MacroOccurrence?
================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:398
R.Location.FileURI = MainFileURI.c_str();
// FIXME: Add correct RefKind information to MainFileMacros.
+ R.Kind = IsDefinition ? RefKind::Definition : RefKind::Reference;
----------------
this fixme is now addressed
================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:405
+ auto StartLoc = sourceLocationInMainFile(SM, Range.start);
+ assert(StartLoc);
+ auto EndLoc = sourceLocationInMainFile(SM, Range.end);
----------------
nit: prefer SourceLocation StartLoc = cantFail(sourceLocationMainFile(...));
================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:408
+ assert(EndLoc);
+ S.Name = Lexer::getSourceText(
+ CharSourceRange::getCharRange(SourceRange(*StartLoc, *EndLoc)), SM,
----------------
nit: we have `toSourceCode(SourceRange(StartLoc, EndLoc))` for this
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94477/new/
https://reviews.llvm.org/D94477
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
