[PATCH] D82118: [clang][module] Improve incomplete-umbrella warning

Tue Aug 18 11:53:28 PDT 2020

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