zixuw added a comment. Hey Bruno! Thanks for the review.
In D82118#2224423 <https://reviews.llvm.org/D82118#2224423>, @bruno wrote: > Hi Zixu, thanks for working on improving this. > > I agree with @vsapsai on the the `GenModuleActionWrapper` approach. Also, it > seems to me that even though it would somehow improve the accuracy, it would > be solving a more general problem, not really specific to this patch. Yes the wrapper is definitely problematic. I'm checking how the diagnostic itself is handled correctly and start from there when I have time. For now would it be better to separate this into multiple patches and get the diagnostic improvement in first? ================ Comment at: clang/test/Modules/Inputs/incomplete-umbrella/normal-module/Umbrella.h:1-3 +// Umbrella.h +// CHECK: #import "A1.h" +// CHECK: #import "A2.h" ---------------- > What about `CHECK`s for the introduced fix-its? I don't see a `#include` or > `#import` being matched in the tests. Is this the check you're looking for? It is checked in `incomplete-umbrella-fixit.m`: ``` RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t -Wno-everything -Wincomplete-umbrella -fixit %s && FileCheck %t/Umbrella.h --input-file %t/Umbrella.h --match-full-lines ``` 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
