sammccall added a comment. I think at this point we specifically don't want to commit to preserving particular modules behavior, this is more or less what the unsupported state means (https://github.com/clangd/clangd/issues/1293).
In particular the pattern where modules are built externally by clang, clang must be version-locked to clangd, and module content is never indexed, doesn't seem to be a good foundation for most users, so having the freedom to tear it down later seems better than encouraging people to rely on it. ================ Comment at: clang-tools-extra/clangd/test/CMakeLists.txt:2 set(CLANGD_TEST_DEPS + clang clangd ---------------- A dependency on clang here probably isn't acceptable: - it's a *large* increase in what needs to be built for `check-clangd`, which we try quite hard to keep small - the conceptual reason is that clangd is consuming modules built by (version-locked) clang, for various reasons this shouldn't be required (not least: users likely don't have a version-locked clang installed) ================ Comment at: clang-tools-extra/clangd/test/completion-modules.test:1 +# Tests that the importee can find the exported entities. +# RUN: rm -fr %t ---------------- We generally find lit tests too hard to maintain for this sort of thing. In particular: - lots of noise added to each test - failures are too hard to parse - too much redundant setup work duplicated in each testcase - substituting filenames tends to run into escaping issues with windows and backslashes, etc - test scope ends up being too large - use of real filesystem doesn't verify the implementation is VFS-clean It's usually OK to have a single smoke test if needed, but generally these are written as unittests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137693/new/ https://reviews.llvm.org/D137693 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
