george.burgess.iv added a comment.

Should we also have a quick test-case in Sema/ verifying that we still get the 
warnings that Eli mentioned?



================
Comment at: clang/lib/AST/Decl.cpp:3006
 
+bool FunctionDecl::isReplaceableSystemFunction() const {
+  FunctionDecl const *Definition;
----------------
serge-sans-paille wrote:
> Note to reviewers: I'm not super happy with that name.
Yeah, `isReplaceableBuiltin()` sounds like "can this be replaced at all?" 
rather than "is this acting as a replacement for something else?"

`isReplacementForBuiltin()` pops to mind, though in a lot of cases, these 
functions may end up calling the "actual" builtin (as you handle later in the 
patch), so maybe `isProxyForBuiltin()` is better?


================
Comment at: clang/lib/AST/Decl.cpp:3010
+
+  FunctionDecl const *Definition;
+  if (hasBody(Definition)) {
----------------
`const FunctionDecl *`


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

Reply via email to