ArcsinX wrote: > And there are more clangd crash reports, which I suspect is due to mixing PCH > and clang named modules. Since all these reports said the project can be > built with clang but not clangd. And from implementors point of view, I don't > think we can fix this in clang in any time soon.
So, this patch is not just a fix for https://github.com/llvm/llvm-project/issues/181770, but also a stability improvement. > For the first point, as the maintainer of modules in clang, I deeply concerns > the usablity of mixing PCH and named modules in clang. But with this patch we build preamble with module import inside > And from my point of view, from clang and clangd's perspective, including > technical point and human resources and community ecosystem, this is the best > solution we can have right now So, if now we are talking about clangd stability with modules support enabled (but not about wrong diagnostics appearing), I think it's ok just not to build a preamble in such cases (and maybe it's ok not to build preambles at all if experimental modules support is enabled?). With this patch the behavior is a bit weird: - we are still building preamble with module import inside, so mixing PCH and C++20 modules, which is not good, as you said. - we drop preamble after that You mentioned in this patch description, skipping preamble build is something slightly complex, but maybe we can put a bit effort to do this, because it looks like a correct direction. https://github.com/llvm/llvm-project/pull/187432 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
