jansvoboda11 accepted this revision. jansvoboda11 added a comment. This revision is now accepted and ready to land.
LGTM, thanks! ================ Comment at: clang/test/ClangScanDeps/optimize-system-warnings.m:19 +// CHECK: ], +// CHECK-NEXT: "name": "A" +// CHECK-NEXT: }, ---------------- Bigcheese wrote: > jansvoboda11 wrote: > > I'd like to see a check line that would fail if the scanner reports another > > variant of module `A`. The `CHECK:` lines make it possible for FileCheck to > > skip an entire element in the modules array. (Structural matching on JSON > > in FileCheck would be a godsend.) > The way FileCheck works I do not believe this can happen. A `CHECK:` line > finds the first line that matches, and `CHECK-NEXT:` follows exactly after > that. There's no case where we have a `CHECK:` that can skip over another > entire module entry. You're right, this is good then. ================ Comment at: clang/test/ClangScanDeps/optimize-system-warnings.m:30 +// CHECK: ], +// CHECK-NEXT: "name": "B" +// CHECK-NEXT: } ---------------- Bigcheese wrote: > jansvoboda11 wrote: > > I'd expect the scanner to still produce two variants of module `B`, since > > it's a user module. Is that correct? If so, let's add explicit check for > > it, since we intentionally want to preserve the warning flags there. > Both `A` and `B` are system modules. `A` is found via `-isystem`, and `B` has > the `[system]` attribute. The FileCheck lines will only succeed if there are > exactly two modules returned. Thanks for pointing that out. Can you put a comment saying just that at the start of the test along with description of the expected behavior? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150689/new/ https://reviews.llvm.org/D150689 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
