[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/Sema/SemaExprCXX.cpp:3463
 
+  DiagnoseUseOfDecl(OperatorNewOrDelete, TheCall->getExprLoc());
+
----------------
Are we also missing a `MarkFunctionReferenced` call here? (I don't think it matters much for the predefined new/delete, since they can't be inline or templated, but it's still wrong to not mark the function the builtin will call as referenced.)


================
Comment at: test/CXX/drs/dr2xx.cpp:721-722
 
   // FIXME: These are ill-formed, with a required diagnostic, for the same
   // reason.
   struct B {
----------------
These are -> This is


================
Comment at: test/SemaCUDA/call-host-fn-from-device.cu:88
 __host__ __device__ void class_specific_delete(T *t, U *u) {
-  delete t; // ok, call sized device delete even though host has preferable non-sized version
+  delete t; // expected-error {{reference to __host__ function 'operator delete' in __host__ __device__ function}}
   delete u; // ok, call non-sized HD delete rather than sized D delete
----------------
tra wrote:
> The C++ magic is way above my paygrade, but as far as CUDA goes this is a regression, compared to what nvcc does. This code in NVCC produced a warning and clang should not error out at this point in time either as 
> it's not an error to call a host function from HD unless we use HD in a host function, and we would not know how it's used until later. I think the error should be postponed until codegen. 
> 
> 
> 
> 
We're in `-fcuda-is-device` mode, so IIUC it's correct to reject a call to a host function here (because `__host__ __device__` is treated as basically meaning `__device__` in that mode for the purpose of checking whether a call is valid), right?

However, the comment suggests that the intent was that this would instead call the device version. Did that actually previously happen (in which case this patch is somehow affecting overload resolution and should be fixed), or is the comment prior to this patch wrong and we were silently calling a host function from a device function (in which case this patch is fine, but we should add a FIXME here to select the device delete function if we think that's appropriate)?


Repository:
  rC Clang

https://reviews.llvm.org/D47757