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

Ken Matsui via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 10 00:00:20 PDT 2022


ken-matsui marked an inline comment as not done.
ken-matsui 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
----------------
aaron.ballman wrote:
> 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.
Ah, I'm quite not familiar with a preprocessor, so I couldn't have understood what was going on. Thank you for your clear explanation.

I've added a new file that tests with `-Wsystem-headers`. This test shows that it seems mostly correct expectations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534



More information about the cfe-commits mailing list