[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

[Raul Tambre via Phabricator via cfe-commits]

tambre added a comment.

If there are no further comments I'll commit this in a few days.



================
Comment at: clang/lib/Sema/SemaDecl.cpp:2088
+
+  if (Error || R.isNull())
+    return nullptr;
----------------
aaron.ballman wrote:
> Should we do this check *before* we create the C linkage decl spec above?
We should. However, since now only `LazilyCreateBuiltin()` calls this and checks these before, we can simply removed them here and pass in the `QualType`.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:9689
+                   Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+          NewFD->addAttr(BuiltinAttr::CreateImplicit(Context, BuiltinID));
+        }
----------------
rsmith wrote:
> Please can you add a `// FIXME` here that we should probably only recognize this as a builtin in the scope where the MS headers actually declare it, rather than in every scope.
Added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491