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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits