[PATCH] D126903: [clang] Add support for __builtin_memset_inline

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet marked 2 inline comments as done.
gchatelet added inline comments.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:2293-2315
+  case Builtin::BI__builtin_memset_inline: {
+    if (checkArgCount(*this, TheCall, 3))
+      return ExprError();
+    auto ArgArrayConversionFailed = [&](unsigned Arg) {
+      ExprResult ArgExpr =
+          DefaultFunctionArrayLvalueConversion(TheCall->getArg(Arg));
+      if (ArgExpr.isInvalid())
----------------
rsmith wrote:
> This custom checking seems unnecessary and also insufficient: you're not enforcing that the arguments have the right types. We should convert the arguments to this builtin in the same way we'd convert arguments for a normally-declared function, which you can achieve by removing the "t" flag from the builtin definition and removing all of this logic (except for the nonnull checking, which I don't think we have a way to model in Builtins.def yet).
> 
> Please do the same to `__builtin_memcpy_inline`, which is wrong in the same way. (For example, `__builtin_memcpy_inline(1, 2, 3)` crashes in code generation because it fails to perform proper conversions and type-checking on its arguments.)
Thx!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126903