[PATCH] D79279: Add overloaded versions of builtin mem* functions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

You need to add user docs for these builtins.



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8917
+
+def err_atomic_type_must_be_lock_free : Error<"_Atomic type must always be lock-free, %0 isn't">;
+
----------------
I don't know why you're adding a bunch of new diagnostics about _Atomic.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:636
+  return ArgTy->castAs<clang::PointerType>()->getPointeeType();
+}
+
----------------
Since arrays are handled separately now, this is just `getPointeeType()`, but I don't know why you need to support ObjC object pointer types here at all.


================
Comment at: clang/lib/CodeGen/CGExpr.cpp:1070
   // We allow this with ObjC object pointers because of fragile ABIs.
-  assert(E->getType()->isPointerType() ||
+  assert(E->getType()->isPointerType() || E->getType()->isArrayType() ||
          E->getType()->isObjCObjectPointerType());
----------------
Why arrays?


================
Comment at: clang/lib/Sema/SemaChecking.cpp:5446
+    return ExprError();
+  clang::Expr *SrcOp = SrcPtr.get();
+
----------------
Do you ever write these back into the call?


================
Comment at: clang/lib/Sema/SemaChecking.cpp:5468
+  bool isAtomic = (DstTy && DstValTy->isAtomicType()) ||
+                  (SrcTy && SrcValTy->isAtomicType());
+
----------------
You already know that DstTy and SrcTy are non-null here.

Why do you need to support atomic types for these operations anyway?  It just seems treacherous and unnecessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279