Hi Jason,
> On 20 Apr 2022, at 04:45, Jason Merrill <ja...@redhat.com> wrote:
>
> On 4/18/22 15:49, Eric Gallager via Gcc-patches wrote:
>> On Mon, Apr 18, 2022 at 10:01 AM Iain Sandoe via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>>>
>>> From: Nathan Sidwell <nat...@acm.org>
>>>
>>> This is a forward-port of a patch by Nathan (against 10.x) which fixes an
>>> open
>>> PR.
>>>
>>> We are ICEing because we ended up tsubst_copying something that had already
>>> been tsubst, leading to an assert failure (mostly such repeated tsubsting is
>>> harmless).
>
> I wouldn't say "mostly". It should always be avoided, it frequently causes
> problems. Pretty much any time there's a class prvalue.
>
>>> We had a non-dependent co_await in a non-dependent-type template fn, so we
>>> processed it at definition time, and then reprocessed at instantiation time.
>>> We fix this here by deferring substitution while processing templates.
>>>
>>> Additional observations (for a better future fix, in the GCC13 timescale):
>>>
>>> Exprs only have dependent type if at least one operand is dependent which
>>> was
>>> what the current code was intending to do. Coroutines have the additional
>>> wrinkle, that the current fn's type is an implicit operand.
>>>
>>> So, if the coroutine function's type is not dependent, and the operand is
>>> not
>>> dependent, we should determine the type of the co_await expression using the
>>> DEPENDENT_EXPR wrapper machinery. That allows us to determine the
>>> subexpression type, but leave its operand unchanged and then instantiate it
>>> later.
>
> Sure, like what build_x_binary_op and the like do.
>
>>> Tested on x86_64-darwin (it does also fix the original testcase, but that is
>>> far too time-consuming for the testsuite).
>
> The compiler change seems fine as a temporary workaround. Is it not feasible
> to write a new short testcase that reproduces the problem, now that you
> understand it?
With the fix it was possible to write a reduction test that fails for the bad
compiler and passes for the fixed without other issues. Threw that at cvice
and we have now a 149 line test which is quick to run.
OK now?
thanks
Iain
From: Nathan Sidwell <nat...@acm.org>
Date: Sun, 3 Apr 2022 11:35:03 +0100
Subject: [PATCH] c++, coroutines: Avoid expanding within templates [PR103868]
This is a forward-port of a patch by Nathan (against 10.x) which fixes an open
PR.
We are ICEing because we ended up tsubst_copying something that had already
been tsubst, leading to an assert failure (mostly such repeated tsubsting is
harmless).
We had a non-dependent co_await in a non-dependent-type template fn, so we
processed it at definition time, and then reprocessed at instantiation time.
We fix this here by deferring substitution while processing templates.
Additional observations (for a better future fix, in the GCC13 timescale):
Exprs only have dependent type if at least one operand is dependent which was
what the current code was intending to do. Coroutines have the additional
wrinkle, that the current fn's type is an implicit operand.
So, if the coroutine function's type is not dependent, and the operand is not
dependent, we should determine the type of the co_await expression using the
DEPENDENT_EXPR wrapper machinery. That allows us to determine the
subexpression type, but leave its operand unchanged and then instantiate it
later.
PR c++/103868
gcc/cp/ChangeLog:
* coroutines.cc (finish_co_await_expr): Do not process non-dependent
coroutine expressions at template definition time.
(finish_co_yield_expr): Likewise.
(finish_co_return_stmt): Likewise.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/pr103868.C: New test.
Co-Authored-by: Iain Sandoe <i...@sandoe.co.uk>
---
gcc/cp/coroutines.cc | 18 +--
gcc/testsuite/g++.dg/coroutines/pr103868.C | 150 +++++++++++++++++++++
2 files changed, 156 insertions(+), 12 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr103868.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index cdf6503c4d3..a9ce6e050dd 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1148,10 +1148,8 @@ finish_co_await_expr (location_t kw, tree expr)
extraneous warnings during substitution. */
suppress_warning (current_function_decl, OPT_Wreturn_type);
- /* If we don't know the promise type, we can't proceed, build the
- co_await with the expression unchanged. */
- tree functype = TREE_TYPE (current_function_decl);
- if (dependent_type_p (functype) || type_dependent_expression_p (expr))
+ /* Defer processing when we have dependent types. */
+ if (processing_template_decl)
{
tree aw_expr = build5_loc (kw, CO_AWAIT_EXPR, unknown_type_node, expr,
NULL_TREE, NULL_TREE, NULL_TREE,
@@ -1222,10 +1220,8 @@ finish_co_yield_expr (location_t kw, tree expr)
extraneous warnings during substitution. */
suppress_warning (current_function_decl, OPT_Wreturn_type);
- /* If we don't know the promise type, we can't proceed, build the
- co_await with the expression unchanged. */
- tree functype = TREE_TYPE (current_function_decl);
- if (dependent_type_p (functype) || type_dependent_expression_p (expr))
+ /* Defer processing when we have dependent types. */
+ if (processing_template_decl)
return build2_loc (kw, CO_YIELD_EXPR, unknown_type_node, expr, NULL_TREE);
if (!coro_promise_type_found_p (current_function_decl, kw))
@@ -1307,10 +1303,8 @@ finish_co_return_stmt (location_t kw, tree expr)
&& check_for_bare_parameter_packs (expr))
return error_mark_node;
- /* If we don't know the promise type, we can't proceed, build the
- co_return with the expression unchanged. */
- tree functype = TREE_TYPE (current_function_decl);
- if (dependent_type_p (functype) || type_dependent_expression_p (expr))
+ /* Defer processing when we have dependent types. */
+ if (processing_template_decl)
{
/* co_return expressions are always void type, regardless of the
expression type. */
diff --git a/gcc/testsuite/g++.dg/coroutines/pr103868.C
b/gcc/testsuite/g++.dg/coroutines/pr103868.C
new file mode 100644
index 00000000000..8687effb563
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr103868.C
@@ -0,0 +1,150 @@
+// { dg-additional-options "-fpreprocessed -std=gnu++20 -w -fconcepts" }
+int _M_invoke___functor;
+namespace std {
+template <int __v> struct integral_constant {
+ static constexpr bool value = __v;
+};
+template <typename> struct decay { typedef int type; };
+template <bool, typename> struct enable_if;
+template <typename _Tp> struct enable_if<true, _Tp> { typedef _Tp type; };
+template <typename, typename _Fn, typename... _Args>
+void __invoke_impl(_Fn __f, _Args... __args) {
+ __f(__args...);
+}
+template <typename, typename _Callable, typename... _Args>
+void __invoke_r(_Callable __fn, _Args... __args) {
+ using __result = int;
+ using __type = __result;
+ __invoke_impl<__type>(__fn, __args...);
+}
+template <typename> class function;
+template <typename _Functor> struct _Base_manager {
+ static _Functor *_M_get_pointer(int) {}
+};
+template <typename, typename> class _Function_handler;
+template <typename _Res, typename _Functor, typename... _ArgTypes>
+struct _Function_handler<_Res(_ArgTypes...), _Functor> {
+ static _Res _M_invoke(_ArgTypes... __args) {
+ auto __trans_tmp_1 =
+ _Base_manager<_Functor>::_M_get_pointer(_M_invoke___functor);
+ __invoke_r<_Res>(*__trans_tmp_1, __args...);
+ }
+};
+template <typename _Res, typename... _ArgTypes>
+struct function<_Res(_ArgTypes...)> {
+ template <typename _Functor> function(_Functor) {
+ _Function_handler<_Res(_ArgTypes...), _Functor>::_M_invoke;
+ }
+};
+template <typename _Tp> struct __shared_ptr_access {
+ using element_type = _Tp;
+ element_type *operator->();
+};
+template <typename> void make_shared();
+} // namespace std
+namespace boost {
+using std::decay;
+using std::enable_if;
+using std::integral_constant;
+namespace asio {
+namespace detail {
+template <typename> struct is_completion_signature;
+template <typename R, typename... Args>
+struct is_completion_signature<R(Args...)> : integral_constant<true> {};
+} // namespace detail
+template <typename T>
+concept completion_signature = detail::is_completion_signature<T>::value;
+template <typename...> struct async_result;
+namespace detail {
+template <completion_signature>
+struct async_result_has_initiate_memfn : integral_constant<1> {};
+} // namespace detail
+template <typename CompletionToken, completion_signature... Signatures,
+ typename Initiation>
+enable_if<detail::async_result_has_initiate_memfn<Signatures...>::value,
+ decltype(async_result<typename decay<CompletionToken>::type,
+ Signatures...>::initiate(0))>::type
+async_initiate(Initiation) {}
+} // namespace asio
+} // namespace boost
+namespace malloy::websocket {
+struct connection {
+ auto read(auto, auto done) {
+ auto wrapper = [] {};
+ return boost::asio::async_initiate<decltype(done), void()>(wrapper);
+ }
+};
+} // namespace malloy::websocket
+namespace std {
+template <typename...> struct coroutine_traits;
+template <typename = void> struct coroutine_handle {
+ operator coroutine_handle<>();
+};
+struct suspend_always {
+ bool await_ready();
+ void await_suspend(coroutine_handle<>);
+ void await_resume();
+};
+} // namespace std
+namespace boost {
+namespace asio {
+namespace detail {
+using std::coroutine_handle;
+using std::suspend_always;
+template <typename> class awaitable_frame;
+} // namespace detail
+template <typename, typename = int> struct awaitable {
+ bool await_ready();
+ template <class U>
+ void await_suspend(detail::coroutine_handle<detail::awaitable_frame<U>>);
+ void await_resume();
+};
+namespace detail {
+struct awaitable_frame_base {
+ auto initial_suspend() { return suspend_always(); }
+ auto final_suspend() noexcept {
+ struct result {
+ bool await_ready() noexcept;
+ void await_suspend(coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
+ };
+ return result{};
+ }
+ void unhandled_exception();
+ template <typename T> auto await_transform(T a) { return a; }
+};
+template <> struct awaitable_frame<void> : awaitable_frame_base {
+ void get_return_object();
+};
+} // namespace detail
+} // namespace asio
+} // namespace boost
+namespace std {
+template <typename T, typename Executor, typename... Args>
+struct coroutine_traits<boost::asio::awaitable<T, Executor>, Args...> {
+ typedef boost::asio::detail::awaitable_frame<T> promise_type;
+};
+} // namespace std
+namespace boost {
+namespace asio {
+struct use_awaitable_t {
+ use_awaitable_t(char, int, char);
+} use_awaitable(0, 0, 0);
+template <typename Executor, typename R, typename... Args>
+struct async_result<Executor, R(Args...)> {
+ template <typename Initiation> static awaitable<int> initiate(Initiation);
+};
+} // namespace asio
+} // namespace boost
+namespace malloy::test {
+void roundtrip_coro(std::function<void(int)>);
+}
+using boost::asio::awaitable;
+template <int> awaitable<void> client_ws_handler_coro() {
+ std::__shared_ptr_access<malloy::websocket::connection> conn;
+ auto buffer = std::make_shared<int>;
+ co_await conn->read(buffer, boost::asio::use_awaitable);
+}
+void port() {
+ malloy::test::roundtrip_coro([](auto) { client_ws_handler_coro<false>; });
+}
--
