aaron.ballman added inline comments.
================
Comment at: clang/test/Preprocessor/line-directive.c:33
# 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
----------------
ken-matsui wrote:
> aaron.ballman wrote:
> > Something is still not quite correct -- these should also be diagnosed as
> > an extension (it's the same feature just with flags).
> This didn't cause a warning on this test file, but another test file I
> created caused a warning.
>
> ```
> // RUN: %clang_cc1 -std=c99 -fsyntax-only -verify -pedantic %s
>
> # 42 "foo" 1 3 // enter: expected-warning {{this style of line directive is
> a GNU extension}}
> ```
Oooohhhh, wait a tick! This is entering a system header! Perhaps we're
silencing the warning because it's "in" a system header!
That's a neat edge case for us to think about -- do users of this diagnostic
*expect* such a construct to be diagnosed? What about the exit from the system
header? GCC seems to diagnose the entrance to the system header, but not the
exit: https://godbolt.org/z/PKGd4jh64 and I don't know if we want to follow
their behavior or not. Our current behavior is defensible, if it marks the
entrance to a system header or the exit from a system header, we're silent.
User who want to see the system header diagnostics can use `-Wsystem-headers`
to get them. So I think I've talked myself into the current behavior here being
correct -- but we should probably add a RUN line that enables diagnostics in
system headers to show our behavior there.
================
Comment at: clang/test/Preprocessor/line-directive.c:67-77
#line 42 "blonk.h" // doesn't change system headerness.
typedef int z; // ok
typedef int z; // ok
# 97 // doesn't change system headerness.
----------------
ken-matsui wrote:
> Should these directives also be diagnosed?
Yes, but this may be a case of the directive being in a system header and thus
the diagnostic is suppressed.
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
