sammccall added inline comments.
================
Comment at: clangd/IncludeFixer.cpp:130
+ // FIXME: consider batching the requests for all diagnostics.
llvm::Optional<Symbol> Matched;
Index.lookup(Req, [&](const Symbol &Sym) {
----------------
oops - this seems broken (in both the old and new form). Symbol is a shallow
reference, and it's not valid after lookup() returns.
We can fix here or separately... one option to fix is the ResolvedSymbol struct
suggested above.
(The fuzzyFind case looks ok)
================
Comment at: clangd/IncludeFixer.h:85
+ // name or incomplete type in one parse, especially when code is
+ // copy-and-pasted without #includes. As fixes are purely dependent on index
+ // requests and index results at this point, we can cache fixes by index
----------------
This seems pretty messy (the assumption that Fix depends only on the index
lookup).
It saves some code now at the expense of being magic. If we want to e.g. insert
qualifiers too, I worry it's going to (stop us || return incorrect cached
results || lead to unneccesary cache misses) depending on what we do.
What would we need to store to calculate Fix?
Maybe a struct ResolvedSymbol with {scope, name, inserted header}? (or even the
edit to insert the header)
If it's not hugely complicated, that seems both cleaner and more extensible -
wdyt?
================
Comment at: clangd/IncludeFixer.h:90
+ mutable llvm::StringMap<std::vector<Fix>> FuzzyReqToFixesCache;
+ mutable llvm::StringMap<std::vector<Fix>> LookupIDToFixesCache;
};
----------------
DenseMap<SymbolID, ...>?
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58239/new/
https://reviews.llvm.org/D58239
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
