Coroutine ignores overloaded operator new

[vogelsgesang]

Expected behavior

clang should reject the code

#include<coroutine>

struct resumable {
   struct promise_type;
   using coro_handle = std::coroutine_handle<promise_type>;

   resumable(coro_handle handle) : handle_(handle) {}
   resumable(resumable&) = delete;
   ~resumable() { if (handle_) { handle_.destroy(); } }

   coro_handle handle_;
};

struct Allocator;

struct resumable::promise_type {
   void* operator new(std::size_t sz, Allocator&);

   std::coroutine_handle<promise_type> get_return_object() { return std::coroutine_handle<promise_type>::from_promise(*this); }
   auto initial_suspend() { return std::suspend_always(); }
   auto final_suspend() noexcept { return std::suspend_always(); }
   void unhandled_exception() {}
   void return_void() {};
};

resumable foo() {
    co_return;
}

because the operator new cannot be called for foo().

Both MSVC and gcc reject it. gcc provides the error message:

<source>:26:11: error: 'operator new' is provided by 'std::__n4861::__coroutine_traits_impl<resumable, void>::promise_type' {aka 'resumable::promise_type'} but is not usable with the function signature 'resumable foo()'
   26 | resumable foo() {
      |           ^~~

Full example: https://godbolt.org/z/a89vjc77Y

Observed behavior

clang accepts the code, and calls the global ::operator new instead of the user-provided overload

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[llvmbot]

@llvm/issue-subscribers-c-20

[ChuanqiXu9]

(The issue could be found quickly if we add the corresponding tag)

According to my understanding to the spec, I think the behavior of clang is correct. Here is the wording http://eel.is/c++draft/dcl.fct.def.coroutine#9:

The allocation function's name is looked up by searching for it in the scope of the promise type.

• if any declarations are found, overload resolution is performed on a function call created by assembling an argument list. The first argument is the amount of space requested, and has type std​::​size_­t. ...
• Otherwise, a search is performed in the global scope.
  If no viable function is found ([over.match.viable]), overload resolution is performed again on a function call created by passing just the amount of space required as an argument of type std​::​size_­t.

So the behavior here for foo is, we look up for the name operator new in promise_type and we found one. But we don't find a viable one (whose argument type should be 'std::size_t' only but the one we found is std::size_t, allocator). So we lookup again for operator new(std::size_t) and we found the global one.

Here is another example: https://godbolt.org/z/1jhKx5dv4

Generally, when we use this functionality, we would use varidic templates to match as many cases as we can. And I guess this is what you want: https://godbolt.org/z/WbW6rYe7a

[vogelsgesang]

Interesting... I think the standard is ambigous in that point.
My interpretation of the standard here is:

if any declarations are found, [...]

this condition is clearly fulfilled

Otherwise, a search is performed in the global scope.

The "Otherwise" is not fulfilled because the "if" case's condition was already fulfilled.
As such, we should not do a global lookup. I think this is where we disagree.

Or in code (without the ambiguity of human language): My interpretation of the C++ standard is

if (any declaration in scope of the promise type found) {
   do overload resolution
} else {
   lookup for "new" in global scope
}

[vogelsgesang]

afaict, the current implementation in clang also contradicts the implementation of std::generator, proposed as part of P2502R1.

This implementation uses

        static void* operator new(const size_t _Size) requires default_initializable<_Alloc> {
            return _Allocate(_Alloc{}, _Size);
        }

with the produce an error if the parameter-less of new is invoked for a non-default-constructible allocator type. However, clang just ignores the operator new here if its requires clause is not fulfilled and does not generate an error.

In particular the following code (see https://godbolt.org/z/Gr8hno358 for a complete example) should not compile, but is still accepted by clang:

template<typename T>
struct StatefulAllocator {
    // Not default constructible!
    StatefulAllocator(const char*);

    using value_type = T;

    void* allocate(size_t sz);
    void deallocate(void* ptr, size_t sz);
};

// This should not compile!
// I am missing the `StatefulAllocator` argument as the first parameter.
// Nevertheless, clang accepts this code.
std::generator<int, void, StatefulAllocator<int>> mySeries() {
    for(int i = 0; i < 10; ++i) co_yield i;
}

int main() {
   auto myGen = mySeries();
}

[ChuanqiXu9]

Interesting... I think the standard is ambigous in that point. My interpretation of the standard here is:

if any declarations are found, [...]

this condition is clearly fulfilled

Otherwise, a search is performed in the global scope.

The "Otherwise" is not fulfilled because the "if" case's condition was already fulfilled. As such, we should not do a global lookup. I think this is where we disagree.

Or in code (without the ambiguity of human language): My interpretation of the C++ standard is

if (any declaration in scope of the promise type found) {
   do overload resolution
} else {
   lookup for "new" in global scope
}

Oh, my interpretation would be:

viable_function = nullptr;
if (any declaration in scope of the promise type found) {
   do overload resolution for viable_function
} else if (found "new" in global scope){
   do overload resolution for viable_function
}
if (!viable_function)
    lookup_again for operator new(std::size_t)
return viable_function

The extra if here corresponds to the If no viable function is found ([[over.match.viable]](http://eel.is/c++draft/over.match.viable)), overload resolution is performed again

So I think it is ambiguous.

[ChuanqiXu9]

After the consideration, I think the wording could be translated to:

// Lookup operator new(std::size_t, p0, p1, ..., pn)
if (any declaration found of operator new in scope of the promise_type)
   do overload resolution for `operator new(std::size_t, p0, p1, ..., pn)`
else if (any declaration found  of `operator new` in global scope)
    do overload resolution for `operator new(std::size_t, p0, p1, ..., pn)`

if (found viable function)
    return viable function
// Lookup operator new(std::size_t)
if (any declaration found of `operator new` in scope of the promise_type)
  do overload resolution for `operator new(std::size_t)`
else if (any declaration found of `operator new(std::size_t)` in global scope)
  do overload resolution for `operator new(std::size_t)`

if (found viable function)
  return viable function

emit error diagnostic message

If it is the case, I think the behavior of clang in the first example is correct. And I need to do more reading to be sure whether or not the second example is right...

I would try to send this question to WG21.

[ChuanqiXu9]

There is the related issue: https://cplusplus.github.io/CWG/issues/2585.html

It looks like the behavior of clang is wrong indeed. I would try to fix it.

[ChuanqiXu9]

Sent https://reviews.llvm.org/D125517