serge-sans-paille added a comment. @efriedma validation passes without the checks you were pointed out, see https://github.com/serge-sans-paille/llvm-project/pull/4/checks Thanks for making the code simpler!
================ Comment at: clang/lib/AST/Decl.cpp:3019 + if (SL.isValid()) + return SM.isInSystemHeader(SL); + } ---------------- serge-sans-paille wrote: > efriedma wrote: > > I'm a little concerned about this; we're subtly changing our generated code > > based on the "system-headerness" of the definition. We generally avoid > > depending on this for anything other than warnings. It could lead to weird > > results with preprocessed source, or if someone specifies their include > > paths incorrectly. > With the extra check on builtin status, it may no longer be necessary, let me > check that. @efriedma I confirm this test is no longer needed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71082/new/ https://reviews.llvm.org/D71082 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
