From patchwork Wed Aug 7 23:31:47 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Arsen_Arsenovi=C4=87?= X-Patchwork-Id: 1970300 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=aarsen.me header.i=@aarsen.me header.a=rsa-sha256 header.s=MBO0001 header.b=d8sp4tws; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WfRjX5P3Zz1yf8 for ; Thu, 8 Aug 2024 09:49:36 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 93E933858C35 for ; Wed, 7 Aug 2024 23:49:34 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mout-p-103.mailbox.org (mout-p-103.mailbox.org [IPv6:2001:67c:2050:0:465::103]) by sourceware.org (Postfix) with ESMTPS id 84AF23858CD9 for ; Wed, 7 Aug 2024 23:49:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 84AF23858CD9 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=aarsen.me Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=aarsen.me ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 84AF23858CD9 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:67c:2050:0:465::103 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1723074554; cv=none; b=nEhbINoHIlqtbZdS8a8g604f2Noww9zcEzt/dTQTAtLZRBbaSaQSPx3vXZYyK6zo0Kauhw0AqWWF9Ld+kI8EUDFQ0R6IMrMJHVHPICgIZYNY2cq6aK+KZToIvIysCjvRp8LgHdG3USxXw3d+xPHuGP5y0Q8d96HuYhuhUxjamHo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1723074554; c=relaxed/simple; bh=gatir1waK03rUSn3rQIte6olxGlEq+rmzRr5TBXafL4=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=T75OL9KChSktFXLWTx/eDJSvOaIufBN8eCowAo+b8doNj9XUUWvB6aYCnhOX8ezlTxjXOhn3zJYs23jW0H1f9D1NYFUJ7lfmykHdbnYkPxkXlqCryeZco0Of9iyVUDWuGyjnXPEu1SFdKo9J5fM0Q6Dg6O3bUET3nODGpgDGYyI= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from smtp2.mailbox.org (smtp2.mailbox.org [10.196.197.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-103.mailbox.org (Postfix) with ESMTPS id 4WfRj14fbWz9tG7; Thu, 8 Aug 2024 01:49:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=aarsen.me; s=MBO0001; t=1723074549; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=m23ZmUfDWCMeRhZD/4ljpQnMiepQKdnA0SH0orKj/xY=; b=d8sp4twsT2/kO8uarx3oT2R9OwsWfD9PtGHaJUob6FNFIPpKkJSpimot/d8xNOlmML7+/H D0QE1ulelMelCVtVzC5/0+1BwFyTIzFhLAEeCUnYDa6Id1I1LSVa+PvndrQRSeBJHYfQqC 9JM0C9qfjz1uiGBIXshd5v/NmB5mV+nWKY/FuNRPS0uRfed5itJ8LW7HI9h7uuWCA8C4Vb IuOGXpwXWRJ5xI/kz0sk0JzjowTuSywvlCTTJ5ngKYzpQlZcoMpdr0WqqoSlzeUjfA7GUo R/3nalT4RNtD3YaCoIl7fIaHe6vy4IjoMJZrKgM9jo2RSp70OOKwex0BCalYvA== From: =?utf-8?q?Arsen_Arsenovi=C4=87?= To: gcc-patches@gcc.gnu.org Cc: Jason Merrill , Iain Sandoe , =?utf-8?q?Arsen_Arsenovi=C4=87?= Subject: [PATCH] c++: improve diagnostic of 'return's in coroutines Date: Thu, 8 Aug 2024 01:31:47 +0200 Message-ID: <20240807234811.36513-1-arsen@aarsen.me> MIME-Version: 1.0 X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_INFOUSMEBIZ, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org Enlargening the function-specific data block is not great. I've considered changing the location of RETURN_STMT expressions to cover everything from the return expression to input_location after parsing the returned expr. The result of that is: test.cc:38:3: error: a ‘return’ statement is not allowed in coroutine; did you mean ‘co_return’? 38 | return {}; | ^~~~~~~~~ test.cc:37:3: note: function was made a coroutine here 37 | co_return; | ^~~~~~~~~ ... so, not bad, but I'm not sure how intrusive such a change would be (haven't tried the testsuite). The current patch produces: test.cc:36:3: error: a ‘return’ statement is not allowed in coroutine; did you mean ‘co_return’? 36 | return {}; | ^~~~~~ test.cc:35:3: note: function was made a coroutine here 35 | co_return; | ^~~~~~~~~ Is there a better location to use here or is the current (latter) one OK? I haven't managed to found a nicer existing one. We also can't stash it in coroutine_info because a function might not have that at time we parse a return. Tested on x86_64-pc-linux-gnu. Have a lovely evening. ---------- >8 ---------- We now point out why a function is a coroutine. gcc/cp/ChangeLog: * coroutines.cc (coro_function_valid_p): Change how we diagnose returning coroutines. * cp-tree.h (struct language_function): Add first_return_loc field. Tracks the location of the first return encountered during parsing. (current_function_first_return_loc): New macro. Expands to the current functions' first_return_loc. * parser.cc (cp_parser_jump_statement): If parsing a RID_RETURN, save its location to current_function_first_return_loc. gcc/testsuite/ChangeLog: * g++.dg/coroutines/co-return-syntax-08-bad-return.C: Update to match new diagnostic. --- gcc/cp/coroutines.cc | 14 +++-- gcc/cp/cp-tree.h | 6 +++ gcc/cp/parser.cc | 4 ++ .../co-return-syntax-08-bad-return.C | 52 +++++++++++++++++-- 4 files changed, 68 insertions(+), 8 deletions(-) diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 0f4dc42ec1c8..f32c7a2eec8d 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -968,11 +968,15 @@ coro_function_valid_p (tree fndecl) if (current_function_returns_value || current_function_returns_null) { - /* TODO: record or extract positions of returns (and the first coro - keyword) so that we can add notes to the diagnostic about where - the bad keyword is and what made the function into a coro. */ - error_at (f_loc, "a % statement is not allowed in coroutine;" - " did you mean %?"); + coroutine_info *coro_info = get_or_insert_coroutine_info (fndecl); + auto retloc = current_function_first_return_loc; + gcc_checking_assert (retloc && coro_info->first_coro_keyword); + + auto_diagnostic_group diaggrp; + error_at (retloc, "a % statement is not allowed in coroutine;" + " did you mean %?"); + inform (coro_info->first_coro_keyword, + "function was made a coroutine here"); return false; } diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 911d1d7924cc..68c681150a1f 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -2123,6 +2123,8 @@ struct GTY(()) language_function { tree x_vtt_parm; tree x_return_value; + location_t first_return_loc; + BOOL_BITFIELD returns_value : 1; BOOL_BITFIELD returns_null : 1; BOOL_BITFIELD returns_abnormally : 1; @@ -2217,6 +2219,10 @@ struct GTY(()) language_function { #define current_function_return_value \ (cp_function_chain->x_return_value) +/* Location of the first 'return' stumbled upon during parsing. */ + +#define current_function_first_return_loc cp_function_chain->first_return_loc + /* In parser.cc. */ extern tree cp_literal_operator_id (const char *); diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index eb102dea8299..6cfe42f3bdd6 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -14957,6 +14957,10 @@ cp_parser_jump_statement (cp_parser* parser, tree &std_attrs) AGGR_INIT_EXPR_MUST_TAIL (ret_expr) = musttail_p; else set_musttail_on_return (expr, token->location, musttail_p); + + /* Save where we saw this keyword. */ + if (current_function_first_return_loc == UNKNOWN_LOCATION) + current_function_first_return_loc = token->location; } /* Build the return-statement, check co-return first, since type diff --git a/gcc/testsuite/g++.dg/coroutines/co-return-syntax-08-bad-return.C b/gcc/testsuite/g++.dg/coroutines/co-return-syntax-08-bad-return.C index 148ee4543e87..1e5d9e7a65a1 100644 --- a/gcc/testsuite/g++.dg/coroutines/co-return-syntax-08-bad-return.C +++ b/gcc/testsuite/g++.dg/coroutines/co-return-syntax-08-bad-return.C @@ -27,6 +27,7 @@ struct Coro { auto final_suspend () noexcept { return coro::suspend_always{}; } void return_void () { } void unhandled_exception() { } + auto yield_value (auto) noexcept { return coro::suspend_always{}; } }; }; @@ -34,10 +35,55 @@ extern int x; // Diagnose disallowed "return" in coroutine. Coro -bar () // { dg-error {a 'return' statement is not allowed} } +bar () { if (x) - return Coro(); + return Coro(); // { dg-error {a 'return' statement is not allowed} } else - co_return; + co_return; // { dg-note "function was made a coroutine here" } +} + +Coro +bar1 () +{ + if (x) + return Coro(); // { dg-error {a 'return' statement is not allowed} } + else + co_await std::suspend_never{}; // { dg-note "function was made a coroutine here" } +} + +Coro +bar2 () +{ + if (x) + return Coro(); // { dg-error {a 'return' statement is not allowed} } + else + co_yield 123; // { dg-note "function was made a coroutine here" } +} + +Coro +bar3 () +{ + if (x) + co_return; // { dg-note "function was made a coroutine here" } + else + return Coro(); // { dg-error {a 'return' statement is not allowed} } +} + +Coro +bar4 () +{ + if (x) + co_await std::suspend_never{}; // { dg-note "function was made a coroutine here" } + else + return Coro(); // { dg-error {a 'return' statement is not allowed} } +} + +Coro +bar5 () +{ + if (x) + co_yield 123; // { dg-note "function was made a coroutine here" } + else + return Coro(); // { dg-error {a 'return' statement is not allowed} } }