[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks, I think we're getting pretty close!



================
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'
----------------
I think we're losing a bunch of test coverage from this. Why did you drop these?


================
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}}
 
----------------
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.


================
Comment at: clang/test/Preprocessor/line-directive.c:33-34
 # 42 "foo" 2 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
 # 42 "foo" 1 3  // enter
 # 42 "foo" 2 3  // exit
 # 42 "foo" 2 3 4 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
----------------



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124534/new/

https://reviews.llvm.org/D124534