[PATCH] D71082: Allow system header to provide their own implementation of some builtin

[serge via Phabricator via cfe-commits]

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