[PATCH] D89297: [clangd] Add a TestWorkspace utility

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 15 11:05:29 PDT 2020


kadircet added a comment.

This looks like a useful infra to have indeed, we currently don't have an easy way to setup a testcase that would require interactions between multiple files (e.g. a workspace), as you've noticed while working on callhierarchy tests (sorry for that).

A couple of suggestions from my side:

- Rather than enforcing files to come in header/source pairs, why not have a `isTU` flag. That way we can use the infra in a more flexible way.
- Instead of having a MockFS, maybe have a single `TestTU` with all the workspace files put into `AdditionalFiles`.  Later on when building an AST, you can just replace the `TestTU::Code` and build the source as if it is in the `workspace`.
- Background or FileIndex is not the point handling include resolution (or AST build process). In theory during the construction time, after populating all the `AdditionalFiles` in a `TestTU`, you can have a single `FileIndex` and populated it for each TU using the AST coming from `TestTU::build()` and preamble from `TestTU::preamble()`. Currently `TestTU::preamble()` doesn't take a callback, but you can change that to populate the `FileIndex` using preamble information.
- In addition to having a constructor that takes all the files(or just some base files) at once, it might be better to have an interface like:

  struct WorkspaceTest {
    void addSource(FilePath, Code);  // These are just sourcefiles, ASTs can be built for them, but they won't be MainFiles during indexing.
    void addMainFile(FilePath, Code); // These are entrypoints, will be marked as TU, and would be used for index builds.
    std::unique_ptr<SymbolIndex> index(); // Builds the index for whole forkspace, by indexing all the MainFiles and merging them under a single FileIndex.
    ParsedAST ast(FilePath);
  };


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89297



More information about the cfe-commits mailing list