ken-matsui added a comment.
Thank you for your review!
================
Comment at: clang/test/Preprocessor/line-directive-system-headers.c:2-6
-// RUN: not %clang_cc1 -E %s 2>&1 | grep 'blonk.c:92:2: error: ABC'
-// RUN: not %clang_cc1 -E %s 2>&1 | grep 'blonk.c:93:2: error: DEF'
-// RUN: not %clang_cc1 -E %s 2>&1 | grep 'line-directive.c:11:2: error: MAIN7'
-// RUN: not %clang_cc1 -E %s 2>&1 | grep 'enter-1:118:2: error: ENTER1'
-// RUN: not %clang_cc1 -E %s 2>&1 | grep 'main:121:2: error: MAIN2'
----------------
aaron.ballman wrote:
> I think we're losing a bunch of test coverage from this. Why did you drop
> these?
These are also tested on `line-directive.c` and possibly redundant, so I
removed them.
Should I keep them?
================
Comment at: clang/test/Preprocessor/line-directive-system-headers.c:57-58
+# 192 "glomp.h" 3 // System header.: expected-warning {{this style of line
directive is a GNU extension}}
+typedef int y; // expected-note {{previous definition is here}}
+typedef int y; // expected-warning {{redefinition of typedef 'y' is a C11
feature}}
----------------
aaron.ballman wrote:
> It took me a moment, but we get these diagnostics because we asked for
> warnings in a system header. So I think we're losing test coverage from this
> change -- previously the test was checking that we suppressed the diagnostics
> as expected.
>
> I think you should have two RUN lines, one with `-Wsystem-headers` and one
> without, so you can test the behavior of both situations. Note, you can do
> `-verify=system` on the one with `-Wsystem-headers` so that you can write `//
> system-warning {{whatever}}` on the diagnostics expected to only appear from
> that RUN line.
>
> Alternatively, you could make this test solely about system header behavior
> and modify line-directive.c to not do anything with system headers. But I
> think that's a much more invasive change.
Thank you! I'm going to edit this file, not `line-directive.c`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124534/new/
https://reviews.llvm.org/D124534
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
