[PATCH] D69165: [clangd] Store Index in Tweak::Selection
Ilya Biryukov via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 21 03:11:17 PDT 2019
ilya-biryukov added inline comments.
================
Comment at: clang-tools-extra/clangd/unittests/TweakTesting.h:71
+ // Index to be passed into Tweak::Selection.
+ const SymbolIndex *Index = nullptr;
+
----------------
kadircet wrote:
> ilya-biryukov wrote:
> > How is this index populated? Each test has to mock it?
> that's what we usually do with the indexes in the rest of the testing code.(see code completion tests for an example)
>
> we might decide to have some defaults if there are a substantial amount of tests making use of the same pattern, wdyt?
I see two potential problems:
- Lifetime of the index. Why not make this field a `unique_ptr<SymbolIndex>`? Client code wouldn't need to worry about keeping the stale pointer in the `Index` field when exiting...
- Ease of discovery for common patterns. Searching for a proper helper does not usually take a lot of time, but I somehow always forget what it is. Should we maybe add a comment mentioning a function that is commonly used for mocking index in tests. WDYT?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69165/new/
https://reviews.llvm.org/D69165
More information about the cfe-commits
mailing list