Skip to content

<generator>: Revert unnecessary ADL firewall for _Gen_promise_base #4827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Stephan T. Lavavej (StephanTLavavej) merged 3 commits into from
Jul 17, 2024
Merged

<generator>: Revert unnecessary ADL firewall for _Gen_promise_base #4827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0e88e90
Revert unnecessary ADL firewall for `_Gen_promise_base`
frederick-vs-ja Jul 10, 2024
b115e9f
Remove `_NODISCARD_FRIEND` workaround
frederick-vs-ja Jul 11, 2024
adcc8a4
Merge 'microsoft:feature/generator' into 'frederick-vs-ja:feature/gen…
frederick-vs-ja Jul 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 17 additions & 24 deletions stl/inc/generator
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,18 +199,13 @@ using _Gen_reference_t = conditional_t<is_void_v<_Vty>, _Rty&&, _Rty>;
template <class _Ref>
using _Gen_yield_t = conditional_t<is_reference_v<_Ref>, _Ref, const _Ref&>;

template <class>
struct _Gen_promise_base_provider {
class _Base;
};

template <class, class>
struct _Gen_iter_provider {
class _Iterator;
};

template <class _Yielded>
class _Gen_promise_base_provider<_Yielded>::_Base {
class _Gen_promise_base {
public:
_STL_INTERNAL_STATIC_ASSERT(is_reference_v<_Yielded>);

Expand Down Expand Up @@ -284,20 +279,20 @@ private:
template <class _CoroPromise>
constexpr void await_suspend(coroutine_handle<_CoroPromise> _Handle) noexcept {
#ifdef __cpp_lib_is_pointer_interconvertible // TRANSITION, LLVM-48860
_STL_INTERNAL_STATIC_ASSERT(is_pointer_interconvertible_base_of_v<_Base, _CoroPromise>);
_STL_INTERNAL_STATIC_ASSERT(is_pointer_interconvertible_base_of_v<_Gen_promise_base, _CoroPromise>);
#endif // ^^^ no workaround ^^^

_Base& _Current = _Handle.promise();
_Current._Ptr = _STD addressof(_Val);
_Gen_promise_base& _Current = _Handle.promise();
_Current._Ptr = _STD addressof(_Val);
}

constexpr void await_resume() const noexcept {}
};

struct _Nest_info {
exception_ptr _Except;
coroutine_handle<_Base> _Parent;
coroutine_handle<_Base> _Root;
coroutine_handle<_Gen_promise_base> _Parent;
coroutine_handle<_Gen_promise_base> _Root;
};

struct _Final_awaiter {
Expand All @@ -308,11 +303,11 @@ private:
template <class _CoroPromise>
_NODISCARD coroutine_handle<> await_suspend(coroutine_handle<_CoroPromise> _Handle) noexcept {
#ifdef __cpp_lib_is_pointer_interconvertible // TRANSITION, LLVM-48860
_STL_INTERNAL_STATIC_ASSERT(is_pointer_interconvertible_base_of_v<_Base, _CoroPromise>);
_STL_INTERNAL_STATIC_ASSERT(is_pointer_interconvertible_base_of_v<_Gen_promise_base, _CoroPromise>);
#endif // ^^^ no workaround ^^^

if (const auto _Info = _Handle.promise()._Try_get_nest_info()) {
coroutine_handle<_Base> _Cont = _Info->_Parent;
coroutine_handle<_Gen_promise_base> _Cont = _Info->_Parent;
_Info->_Root.promise()._Set_top(_Cont);
return _Cont;
}
Expand All @@ -338,12 +333,13 @@ private:
}

template <class _CoroPromise>
_NODISCARD coroutine_handle<_Base> await_suspend(coroutine_handle<_CoroPromise> _Current) noexcept {
_NODISCARD coroutine_handle<_Gen_promise_base> await_suspend(
coroutine_handle<_CoroPromise> _Current) noexcept {
#ifdef __cpp_lib_is_pointer_interconvertible // TRANSITION, LLVM-48860
_STL_INTERNAL_STATIC_ASSERT(is_pointer_interconvertible_base_of_v<_Base, _CoroPromise>);
_STL_INTERNAL_STATIC_ASSERT(is_pointer_interconvertible_base_of_v<_Gen_promise_base, _CoroPromise>);
#endif // ^^^ no workaround ^^^
auto _Target = coroutine_handle<_Base>::from_address(_Gen._Coro.address());
_Nested._Parent = coroutine_handle<_Base>::from_address(_Current.address());
auto _Target = coroutine_handle<_Gen_promise_base>::from_address(_Gen._Coro.address());
_Nested._Parent = coroutine_handle<_Gen_promise_base>::from_address(_Current.address());
if (const auto _Parent_nest_info = _Nested._Parent.promise()._Try_get_nest_info()) {
_Nested._Root = _Parent_nest_info->_Root;
} else {
Expand All @@ -370,16 +366,16 @@ private:
return nullptr;
}

_NODISCARD coroutine_handle<_Base> _Get_top() const noexcept {
_NODISCARD coroutine_handle<_Gen_promise_base> _Get_top() const noexcept {
_STL_INTERNAL_CHECK((_Data & 1U) == 0);
return coroutine_handle<_Base>::from_address(reinterpret_cast<void*>(_Data));
return coroutine_handle<_Gen_promise_base>::from_address(reinterpret_cast<void*>(_Data));
}

void _Set_nest_info(_Nest_info* _Info) noexcept {
_Data = reinterpret_cast<uintptr_t>(_Info) | 1U;
}

void _Set_top(coroutine_handle<_Base> _Top) noexcept {
void _Set_top(coroutine_handle<_Gen_promise_base> _Top) noexcept {
_Data = reinterpret_cast<uintptr_t>(_Top.address());
}

Expand All @@ -389,13 +385,10 @@ private:
// Least significant bit of `_Data` indicates stored information:
// LSB 0: `_Data` is a top coroutine handle,
// LSB 1: `_Data ^ 1U` is a pointer to an object of type `_Nest_info`.
uintptr_t _Data = reinterpret_cast<uintptr_t>(coroutine_handle<_Base>::from_promise(*this).address());
uintptr_t _Data = reinterpret_cast<uintptr_t>(coroutine_handle<_Gen_promise_base>::from_promise(*this).address());
add_pointer_t<_Yielded> _Ptr = nullptr;
};

template <class _Yielded>
using _Gen_promise_base = _Gen_promise_base_provider<_Yielded>::_Base;

struct _Gen_secret_tag {};

template <class _Value, class _Ref>
Expand Down
61 changes: 39 additions & 22 deletions tests/std/tests/P2502R2_generator_promise/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ void test_operator_new(typename Gen::promise_type& p, const Alloc2& alloc2 = {})
}
}

template <class Ref, class V = void, class Alloc = void>
template <class Ref, class V, class Alloc, bool TestingIncomplete>
void test_one() {
using Gen = generator<Ref, V, Alloc>;
using Promise = Gen::promise_type;
Expand All @@ -109,6 +109,10 @@ void test_one() {

Promise p;

// Test that operator& for promise_type resolves to the built-in version and doesn't involve ADL
static_assert(same_as<decltype(&p), Promise*>);
assert(&p == addressof(p));

// Test 'get_return_object'
static_assert(same_as<decltype(p.get_return_object()), Gen>);
static_assert(noexcept(p.get_return_object()));
Expand Down Expand Up @@ -154,7 +158,8 @@ void test_one() {
}

using ValTy = conditional_t<is_void_v<V>, remove_cvref_t<Ref>, V>;
if constexpr (convertible_to<ValTy&, Yielded>) { // Test 'yield_value(ranges::elements_of<range>)'
if constexpr (!TestingIncomplete && convertible_to<ValTy&, Yielded>) {
// Test 'yield_value(ranges::elements_of<range>)'
test_yield_elements_of_range<Gen, vector<ValTy>>(p);
test_yield_elements_of_range<Gen, list<ValTy>>(p);
test_yield_elements_of_range<Gen, forward_list<ValTy>>(p);
Expand Down Expand Up @@ -186,37 +191,49 @@ void test_one() {
}
}

template <class Ref, class V = void>
template <class Ref, class V, bool TestingIncomplete = false>
void test_with_allocator() {
test_one<Ref, V>();
test_one<Ref, V, allocator<void>>();
test_one<Ref, V, pmr::polymorphic_allocator<void>>();
test_one<Ref, V, StatelessAlloc<void>>();
test_one<Ref, V, StatelessAlloc<void, false_type>>();
test_one<Ref, V, StatelessAlloc<void, true_type, int>>();
test_one<Ref, V, StatelessAlloc<void, false_type, int>>();
test_one<Ref, V, void, TestingIncomplete>();
test_one<Ref, V, allocator<void>, TestingIncomplete>();
test_one<Ref, V, pmr::polymorphic_allocator<void>, TestingIncomplete>();
test_one<Ref, V, StatelessAlloc<void>, TestingIncomplete>();
test_one<Ref, V, StatelessAlloc<void, false_type>, TestingIncomplete>();
test_one<Ref, V, StatelessAlloc<void, true_type, int>, TestingIncomplete>();
test_one<Ref, V, StatelessAlloc<void, false_type, int>, TestingIncomplete>();
}

template <class T>
template <class T, bool TestingIncomplete = false>
void test_with_type() {
test_with_allocator<T>();
test_with_allocator<T&>();
test_with_allocator<const T&>();
test_with_allocator<T&&>();
test_with_allocator<const T&&>();

test_with_allocator<Proxy<T>, T>();
test_with_allocator<Proxy<T>&, T>();
test_with_allocator<const Proxy<T>&, T>();
test_with_allocator<Proxy<T>&&, T>();
test_with_allocator<const Proxy<T>&&, T>();
test_with_allocator<T, void, TestingIncomplete>();
test_with_allocator<T&, void, TestingIncomplete>();
test_with_allocator<const T&, void, TestingIncomplete>();
test_with_allocator<T&&, void, TestingIncomplete>();
test_with_allocator<const T&&, void, TestingIncomplete>();

test_with_allocator<Proxy<T>, T, TestingIncomplete>();
test_with_allocator<Proxy<T>&, T, TestingIncomplete>();
test_with_allocator<const Proxy<T>&, T, TestingIncomplete>();
test_with_allocator<Proxy<T>&&, T, TestingIncomplete>();
test_with_allocator<const Proxy<T>&&, T, TestingIncomplete>();
}

#ifndef _M_CEE // TRANSITION, VSO-1659496

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) Jul 17, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change requested: Technically, this isn't necessary because <generator> is C++23, and /clr is currently limited to C++20. However, following the usual pattern seems fine.

template <class T>
struct Holder {
T t;
};

struct Incomplete;
#endif // ^^^ no workaround ^^^

int main() {
test_with_type<int>();
test_with_type<float>();
test_with_type<string>();
test_with_type<MoveOnly>();
test_with_type<Immovable>();
test_with_allocator<vector<bool>::reference, bool>();
#ifndef _M_CEE // TRANSITION, VSO-1659496
test_with_type<Holder<Incomplete>*, true>();
#endif // ^^^ no workaround ^^^
}