From patchwork Fri Feb 9 18:31:45 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 1897181 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=PIcWw04H; 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 4TWjBQ3PwQz23j2 for ; Sat, 10 Feb 2024 05:32:14 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 384633858283 for ; Fri, 9 Feb 2024 18:32:12 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 925B53858C39 for ; Fri, 9 Feb 2024 18:31:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 925B53858C39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 925B53858C39 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707503515; cv=none; b=PfggfkIlYbrLi3JeXa5VxOraO6tTLMfEcbRdb2OUch9itabZQD75WRoEXELqJH+hUmp/lE8H19yK3V/zoT7toh6yeXFjbgY3C7A66RDvP6T9g5mFTzvYAMxzQu5EAI4B5mdsioadvyeWdffXNlwzjDWhfwyD3HGvDDb/+icUZg0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707503515; c=relaxed/simple; bh=QCXgD58GbiFSNOOOj5XqlCWZIdnD55o7wwjqnxH5A2w=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=NFRjGwIDKWXU3L1tZCta/xL9kk83e1vTXZextbEULgTqDcCwMQFs0sxd4DAA8MOK/dQ2XAow4cMe9DVZ4M4F2uIBU/muth/Va3SV3ous2JmBcIuDJdi3IGZHg9f3kQto6Lpifi4Il3a6oS7zSvl+JxrndhacXqyKecu55BFYeHo= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1707503512; 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: in-reply-to:in-reply-to:references:references; bh=mVymul3lc/F5gbChDwTTso8homyaZlJhTzimcR2AEZo=; b=PIcWw04H9naBw6JQ0XiOHOqK3omPjo7ZIEXAkp6duvV3Wlpr20oLSOCulq3NeuMTPhTHMD bjXZ7Wmz4GS9lez7PIyzTN4TACbBAgZ9HZrE2JBQoaPuD0vqF2yfj54YGDlu3t533lGpio 4GwvkuoHjF8kTaux2bF/Jys2McsUANs= Received: from mail-oi1-f200.google.com (mail-oi1-f200.google.com [209.85.167.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-630-UIyr_6WwPa-i-e-kUTHcyg-1; Fri, 09 Feb 2024 13:31:49 -0500 X-MC-Unique: UIyr_6WwPa-i-e-kUTHcyg-1 Received: by mail-oi1-f200.google.com with SMTP id 5614622812f47-3bfc3be1b8eso1378904b6e.0 for ; Fri, 09 Feb 2024 10:31:49 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707503508; x=1708108308; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=mVymul3lc/F5gbChDwTTso8homyaZlJhTzimcR2AEZo=; b=xOLVXcD97TqWDgOCaCYqjRYl78ZmjX4bUtzMl54Us9HyXqUcGHknRbU0hkxkJfNqAh S2KdW0LUe2lQFdY+10segD2pnBDlgRuiRqdmqLuf85gGIvi5zy46a3/nIuBbiwehBvC4 wG/R8a95WtXFK+rUQHRFLog37S3OowkYI3iUgxP/2yDFDTsH78I5ACAhGA3siHvQQXzJ V6yCB5F6ziknYbgAaZnFkaSIECOb4Jia6k1HL/v0oqiqP+fyHZBk5tJ6Fp+7IF4Jl8cA R4xW38aQfBeoI4s64tM1uyw3VTCYz7HK9cm1rDgwsOP6FEXxU1CX4FAH7ldAA1g/dLeQ /DmA== X-Gm-Message-State: AOJu0YzhTKNXKgK5q2Xf36s1mPj2ssjztMPyq6Mmgp4oGlbVuq1/ohA+ ZAzFrkcVCK0BFTVR0Jp0Zqj/90m4G1PWD+oHJUaxQcYcEU08HEEuZdZIlAbaUmWknhurQwDP50w Ye9nirjyoWH9LgJQDvzEn5RAGx7/HLtqas3ZDn5Nn1X7H+QmOqDDYRSo= X-Received: by 2002:a05:6808:18a9:b0:3bd:cb00:3fed with SMTP id bi41-20020a05680818a900b003bdcb003fedmr2757917oib.41.1707503508430; Fri, 09 Feb 2024 10:31:48 -0800 (PST) X-Google-Smtp-Source: AGHT+IFP1JqD0RfHG9HXnxZzQzmIa68SILdURMg0lqAl6vLsQSK8yx7ZG7QMACeoQSS934qDHc3Cfw== X-Received: by 2002:a05:6808:18a9:b0:3bd:cb00:3fed with SMTP id bi41-20020a05680818a900b003bdcb003fedmr2757895oib.41.1707503508047; Fri, 09 Feb 2024 10:31:48 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCV4xhqrhXNvPI8AarXq+sPYjDv+qh4gGOyaErymqjgo5efWSfzFNaWxnqs6/nIJFNLOBP3/9/+xwRdwYlu4V8lkBCANm4rWXg== Received: from redhat.com (2603-7000-9500-34a5-0000-0000-0000-1db4.res6.spectrum.com. [2603:7000:9500:34a5::1db4]) by smtp.gmail.com with ESMTPSA id o1-20020ac84281000000b0042c258aabfasm905036qtl.86.2024.02.09.10.31.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 09 Feb 2024 10:31:47 -0800 (PST) Date: Fri, 9 Feb 2024 13:31:45 -0500 From: Marek Polacek To: Jason Merrill Cc: Patrick Palka , GCC Patches Subject: [PATCH v3] c++: make build_throw SFINAE-friendly [PR98388] Message-ID: References: <20240208020601.479863-1-polacek@redhat.com> <4237f72c-7d85-b45c-36c6-7724acfca898@idea> <20f15673-d134-490f-9a5f-d580bfdb7e5f@redhat.com> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/2.2.12 (2023-09-09) X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE 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 On Fri, Feb 09, 2024 at 10:07:33AM -0500, Jason Merrill wrote: > On 2/8/24 17:20, Marek Polacek wrote: > > On Thu, Feb 08, 2024 at 04:53:45PM -0500, Jason Merrill wrote: > > > On 2/8/24 11:51, Marek Polacek wrote: > > > > On Thu, Feb 08, 2024 at 08:49:28AM -0500, Patrick Palka wrote: > > > > > On Wed, 7 Feb 2024, Marek Polacek wrote: > > > > > > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > > > > > > > > > -- >8 -- > > > > > > Here the problem is that we give hard errors while substituting > > > > > > template parameters during overload resolution of is_throwable > > > > > > which has an invalid throw in decltype. > > > > > > > > > > > > The backtrace shows that fn_type_unification -> instantiate_template > > > > > > -> tsubst* passes complain=0 as expected, but build_throw doesn't > > > > > > have a complain parameter. So let's add one. Also remove a redundant > > > > > > local variable which I should have removed in my P2266 patch. > > > > > > > > > > > > But there's still something not quite clear to me. I claim that 'b' > > > > > > in the testcase should evaluate to false since the first overload ought > > > > > > to have been discarded. EDG 6.6 agrees, but clang++, msvc, and icx evaluate > > > > > > it to true. Who's right? > > > > > > I think it should be true since P1155, which we implement in C++20 mode and > > > above (or rather, we implement the sequel P2266); since then we implicitly > > > move from the function parameter. > > > > > > The patch looks good except that we should test this expected value. > > > > I could add > > #if __cplusplus >= 202002L > > static_assert (b, "move from the function parameter"); > > #else > > static_assert (!b, "no move from the function parameter"); > > #endif > > > > but that's going to fail for C++20 and above. > > Ah, because treat_lvalue_as_rvalue_p doesn't recognize t as being from the > current scope in the trailing return type. But that shouldn't be necessary: > > https://eel.is/c++draft/expr.prim.id.unqual#4.2 says it's move-eligible > "if the id-expression (possibly parenthesized) is the operand of a > throw-expression ([expr.throw]), and names an implicitly movable entity that > belongs to a scope that does not contain the compound-statement of the > innermost lambda-expression, try-block, or function-try-block (if any) whose > compound-statement or ctor-initializer contains the throw-expression." > > here there is no enclosing lambda or try, so it seems move-eligible. ...and as a result, 't' will be an xvalue, therefore moveonly(moveonly&&) should be used, therefore the first overload should be used, which means 'b' is true. > > I wonder if this is the > > second half of the problem in 113789? > > > > I could comment the first static_assert and add a FIXME if that sounds good? > > dg-bogus would be better than commenting it out. Ok, in this patch the only change I made is: +#if __cplusplus >= 202002L +static_assert (b, "move from the function parameter"); // { dg-bogus "" "PR113853" { xfail c++20 } } +#else +static_assert (!b, "no move from the function parameter"); +#endif > Will you also look into fixing the treat_ bug? That can be a separate > patch. I'd like to. I opened . -- >8 -- Here the problem is that we give hard errors while substituting template parameters during overload resolution of is_throwable which has an invalid throw in decltype. The backtrace shows that fn_type_unification -> instantiate_template -> tsubst* passes complain=0 as expected, but build_throw doesn't have a complain parameter. So let's add one. Also remove a redundant local variable which I should have removed in my P2266 patch. There's still one problem for which I opened . We need to patch up treat_lvalue_as_rvalue_p and remove the dg-bogus. Thanks to Patrick for notifying me of this PR. This doesn't fully fix 113789; there I think I'll have to figure our why a candidate wasn't discarded from the overload set. gcc/cp/ChangeLog: * coroutines.cc (coro_rewrite_function_body): Pass tf_warning_or_error to build_throw. (morph_fn_to_coro): Likewise. * cp-tree.h (build_throw): Adjust. * except.cc (expand_end_catch_block): Pass tf_warning_or_error to build_throw. (build_throw): Add a tsubst_flags_t parameter. Use it. Remove redundant variable. Guard an inform call. * parser.cc (cp_parser_throw_expression): Pass tf_warning_or_error to build_throw. * pt.cc (tsubst_expr) : Pass complain to build_throw. libcc1/ChangeLog: * libcp1plugin.cc (plugin_build_unary_expr): Pass tf_error to build_throw. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/sfinae69.C: New test. --- gcc/cp/coroutines.cc | 4 ++-- gcc/cp/cp-tree.h | 3 ++- gcc/cp/except.cc | 33 ++++++++++++--------------- gcc/cp/parser.cc | 2 +- gcc/cp/pt.cc | 2 +- gcc/testsuite/g++.dg/cpp0x/sfinae69.C | 21 +++++++++++++++++ libcc1/libcp1plugin.cc | 4 ++-- 7 files changed, 43 insertions(+), 26 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/sfinae69.C base-commit: 41a6d2560500a202708e7b661b8b2ad432aee3a6 diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 3194c911e8c..9b037edbd14 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -4246,7 +4246,7 @@ coro_rewrite_function_body (location_t fn_start, tree fnbody, tree orig, boolean_type_node, i_a_r_c); finish_if_stmt_cond (not_iarc, not_iarc_if); /* If the initial await resume called value is false, rethrow... */ - tree rethrow = build_throw (fn_start, NULL_TREE); + tree rethrow = build_throw (fn_start, NULL_TREE, tf_warning_or_error); suppress_warning (rethrow); finish_expr_stmt (rethrow); finish_then_clause (not_iarc_if); @@ -5151,7 +5151,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) tree del_coro_fr = coro_get_frame_dtor (coro_fp, orig, frame_size, promise_type, fn_start); finish_expr_stmt (del_coro_fr); - tree rethrow = build_throw (fn_start, NULL_TREE); + tree rethrow = build_throw (fn_start, NULL_TREE, tf_warning_or_error); suppress_warning (rethrow); finish_expr_stmt (rethrow); finish_handler (handler); diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 969c7239c97..334c11396c2 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7185,7 +7185,8 @@ extern void init_exception_processing (void); extern tree expand_start_catch_block (tree); extern void expand_end_catch_block (void); extern tree build_exc_ptr (void); -extern tree build_throw (location_t, tree); +extern tree build_throw (location_t, tree, + tsubst_flags_t); extern int nothrow_libfn_p (const_tree); extern void check_handlers (tree); extern tree finish_noexcept_expr (tree, tsubst_flags_t); diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc index d17a57d3fbc..ea3d6f57396 100644 --- a/gcc/cp/except.cc +++ b/gcc/cp/except.cc @@ -486,7 +486,8 @@ expand_end_catch_block (void) || DECL_DESTRUCTOR_P (current_function_decl)) && !in_nested_catch ()) { - tree rethrow = build_throw (input_location, NULL_TREE); + tree rethrow = build_throw (input_location, NULL_TREE, + tf_warning_or_error); /* Disable all warnings for the generated rethrow statement. */ suppress_warning (rethrow); finish_expr_stmt (rethrow); @@ -607,7 +608,7 @@ wrap_cleanups_r (tree *tp, int *walk_subtrees, void * /*data*/) /* Build a throw expression. */ tree -build_throw (location_t loc, tree exp) +build_throw (location_t loc, tree exp, tsubst_flags_t complain) { if (exp == error_mark_node) return exp; @@ -621,7 +622,7 @@ build_throw (location_t loc, tree exp) return exp; } - if (exp && null_node_p (exp)) + if (exp && null_node_p (exp) && (complain & tf_warning)) warning_at (loc, 0, "throwing NULL, which has integral, not pointer type"); @@ -642,8 +643,6 @@ build_throw (location_t loc, tree exp) tree object, ptr; tree allocate_expr; - tsubst_flags_t complain = tf_warning_or_error; - /* The CLEANUP_TYPE is the internal type of a destructor. */ if (!cleanup_type) { @@ -714,7 +713,6 @@ build_throw (location_t loc, tree exp) if (CLASS_TYPE_P (temp_type)) { int flags = LOOKUP_NORMAL | LOOKUP_ONLYCONVERTING; - bool converted = false; location_t exp_loc = cp_expr_loc_or_loc (exp, loc); /* Under C++0x [12.8/16 class.copy], a thrown lvalue is sometimes @@ -727,23 +725,20 @@ build_throw (location_t loc, tree exp) exp = moved; /* Call the copy constructor. */ - if (!converted) - { - releasing_vec exp_vec (make_tree_vector_single (exp)); - exp = (build_special_member_call - (object, complete_ctor_identifier, &exp_vec, - TREE_TYPE (object), flags, tf_warning_or_error)); - } - + releasing_vec exp_vec (make_tree_vector_single (exp)); + exp = build_special_member_call (object, complete_ctor_identifier, + &exp_vec, TREE_TYPE (object), flags, + complain); if (exp == error_mark_node) { - inform (exp_loc, " in thrown expression"); + if (complain & tf_error) + inform (exp_loc, " in thrown expression"); return error_mark_node; } } else { - tree tmp = decay_conversion (exp, tf_warning_or_error); + tree tmp = decay_conversion (exp, complain); if (tmp == error_mark_node) return error_mark_node; exp = cp_build_init_expr (object, tmp); @@ -768,7 +763,7 @@ build_throw (location_t loc, tree exp) tree binfo = TYPE_BINFO (TREE_TYPE (object)); tree dtor_fn = lookup_fnfields (binfo, complete_dtor_identifier, 0, - tf_warning_or_error); + complain); dtor_fn = BASELINK_FUNCTIONS (dtor_fn); if (!mark_used (dtor_fn) || !perform_or_defer_access_check (binfo, dtor_fn, @@ -785,7 +780,7 @@ build_throw (location_t loc, tree exp) cleanup = build_int_cst (cleanup_type, 0); /* ??? Indicate that this function call throws throw_type. */ - tree tmp = cp_build_function_call_nary (throw_fn, tf_warning_or_error, + tree tmp = cp_build_function_call_nary (throw_fn, complain, ptr, throw_type, cleanup, NULL_TREE); @@ -807,7 +802,7 @@ build_throw (location_t loc, tree exp) /* ??? Indicate that this function call allows exceptions of the type of the enclosing catch block (if known). */ - exp = cp_build_function_call_vec (rethrow_fn, NULL, tf_warning_or_error); + exp = cp_build_function_call_vec (rethrow_fn, NULL, complain); } exp = build1_loc (loc, THROW_EXPR, void_type_node, exp); diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index c4292c49ce2..09ecfa23b5d 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -29310,7 +29310,7 @@ cp_parser_throw_expression (cp_parser* parser) the end at the end of the final token we consumed. */ location_t combined_loc = make_location (start_loc, start_loc, parser->lexer); - expression = build_throw (combined_loc, expression); + expression = build_throw (combined_loc, expression, tf_warning_or_error); return expression; } diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index f9abb21a9a2..9c225c095c8 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -21180,7 +21180,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl) case THROW_EXPR: RETURN (build_throw - (input_location, RECUR (TREE_OPERAND (t, 0)))); + (input_location, RECUR (TREE_OPERAND (t, 0)), complain)); case CONSTRUCTOR: { diff --git a/gcc/testsuite/g++.dg/cpp0x/sfinae69.C b/gcc/testsuite/g++.dg/cpp0x/sfinae69.C new file mode 100644 index 00000000000..361e0b79ba2 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/sfinae69.C @@ -0,0 +1,21 @@ +// PR c++/98388 +// { dg-do compile { target c++11 } } + +struct moveonly { + moveonly() = default; + moveonly(moveonly&&) = default; +}; + +template +constexpr auto is_throwable(T t) -> decltype(throw t, true) { + return true; +} +template +constexpr bool is_throwable(...) { return false; } + +constexpr bool b = is_throwable(moveonly{}); +#if __cplusplus >= 202002L +static_assert (b, "move from the function parameter"); // { dg-bogus "" "PR113853" { xfail c++20 } } +#else +static_assert (!b, "no move from the function parameter"); +#endif diff --git a/libcc1/libcp1plugin.cc b/libcc1/libcp1plugin.cc index 7f4e4c98060..0eff7c68d29 100644 --- a/libcc1/libcp1plugin.cc +++ b/libcc1/libcp1plugin.cc @@ -2640,7 +2640,7 @@ plugin_build_unary_expr (cc1_plugin::connection *self, break; case THROW_EXPR: - result = build_throw (input_location, op0); + result = build_throw (input_location, op0, tf_error); break; case TYPEID_EXPR: @@ -2664,7 +2664,7 @@ plugin_build_unary_expr (cc1_plugin::connection *self, result = make_pack_expansion (op0); break; - // We're using this for sizeof...(pack). */ + /* We're using this for sizeof...(pack). */ case TYPE_PACK_EXPANSION: result = make_pack_expansion (op0); PACK_EXPANSION_SIZEOF_P (result) = true;