zixuw added a comment. In D82118#2154280 <https://reviews.llvm.org/D82118#2154280>, @vsapsai wrote:
> Didn't get into details but overall GenModuleActionWrapper approach looks > pretty complicated. Haven't tried to accomplish the same myself, so cannot > say if such complexity is warranted or not. Yes the approach is complicated. But one thing to note is that the `GenModuleActionWrapper`-related facilities are upstreamed from apple/llvm-project. I adopted this approach just because it was available. Haven't explored other ways, but yes there might be a simpler approach. > What happens with fixits in modules when you don't have > GenModuleActionWrapper? If the fixit action is not forwarded to the new compiler instance, it will be handled by the 'outer' compiler instance, which has a different source manager that does not see the module files, so the source location cannot be correctly interpreted. From my experiments, the fixit hint is trying to write into the `<builtin>` file buffer in this test case, when I don't have the action forwarded using `GenModuleActionWrapper`. In this case the fixit rewriter quietly fails because `<builtin>` cannot be rewritten, but since this is not caught I think there might be cases where the wrong file ends up getting updated. > How does it work with modules disabled? Also looks like there are no tests > for incomplete umbrella warning when modules are disabled. Haven't tested this, will give a try. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82118/new/ https://reviews.llvm.org/D82118 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
