From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id C83E1384B824 for ; Sat, 17 Sep 2022 06:42:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C83E1384B824 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1663396954; h=from:from:reply-to: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=q9SXfhXf6t80S4BMRLQny7ZUQAQZFhXfWEPjmxc7jz8=; b=AyEMRFKHdKcpVaSDRaIcZZvElnaSBFVZWXPqENh1QqGCdnByGOQiSmy6ZQF7oqI9X2K5cl DAYlJB4dYu0z+t3OGQyCNAVqpFRAb9TgXniQNvZBARRD0JuYd55tufg2i9C20yutwV5Am6 AptwxykEcz8iEj9BCUQ0S1guOwGHaQ8= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-241-vO4ybZpqMa2g252Z_WvB8A-1; Sat, 17 Sep 2022 02:42:33 -0400 X-MC-Unique: vO4ybZpqMa2g252Z_WvB8A-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E8FBC3806707 for ; Sat, 17 Sep 2022 06:42:32 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.194]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A06481121314; Sat, 17 Sep 2022 06:42:32 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 28H6gMCJ4077451 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Sat, 17 Sep 2022 08:42:22 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 28H6gLiB4077450; Sat, 17 Sep 2022 08:42:21 +0200 Date: Sat, 17 Sep 2022 08:42:21 +0200 From: Jakub Jelinek To: Jason Merrill Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c++: Implement C++23 P1169R4 - static operator() [PR106651] Message-ID: Reply-To: Jakub Jelinek References: <35c635ad-118f-63bd-eb58-cd949286e28d@redhat.com> MIME-Version: 1.0 In-Reply-To: <35c635ad-118f-63bd-eb58-cd949286e28d@redhat.com> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_NONE,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 List-Id: On Sat, Sep 17, 2022 at 01:23:59AM +0200, Jason Merrill wrote: > On 9/13/22 12:42, Jakub Jelinek wrote: > > The following patch attempts to implement C++23 P1169R4 - static operator() > > paper's compiler side (there is some small library side too not implemented > > yet). This allows static members as user operator() declarations and > > static specifier on lambdas without lambda capture. As decl specifier > > parsing doesn't track about the presence and locations of all specifiers, > > the patch unfortunately replaces the diagnostics about duplicate mutable > > with diagnostics about conflicting specifiers because the information > > whether it was mutable mutable, mutable static, static mutable or static > > static is lost. > > I wonder why we don't give an error when setting the > conflicting_specifiers_p flag in cp_parser_set_storage_class? We should be > able to give a better diagnostic at that point. I will try that. > > Beyond this, the synthetized conversion operator changes > > for static lambdas as it can just return the operator() static method > > address, doesn't need to create a thunk for it. > > The change I'm least sure about is the call.cc (joust) change, one thing > > is that we ICEd because we assumed that len could be different only if > > both candidates are direct calls but it can be one direct and one indirect > > call, > > How do you mean? That is either on the struct less { static constexpr auto operator()(int i, int j) -> bool { return i < j; } using P = bool(*)(int, int); operator P() const { return operator(); } }; static_assert(less{}(1, 2)); testcase from the paper (S in static-operator-call1.C) or any static lambda which also has static operator() and cast operator. The paper then talks about operator()(contrived-parameter, int, int); call-function(bool(*)(int, int), int, int); which is exactly what caused the ICE, one of the cand?->fn was the static operator(), the other cand?->fn isn't a FUNCTION_DECL, but a function pointer (what the cast operator returns). > > + { > > + /* C++23 [over.best.ics.general] says: > > + When the parameter is the implicit object parameter of a static > > + member function, the implicit conversion sequence is a standard > > + conversion sequence that is neither better nor worse than any > > + other standard conversion sequence. > > + Apply this for C++23 or when the static member function is > > + overloaded call operator (C++23 feature we accept as an > > + extension). */ > > + if ((cxx_dialect >= cxx23 > > + || (DECL_OVERLOADED_OPERATOR_P (cand1->fn) > > + && DECL_OVERLOADED_OPERATOR_IS (cand1->fn, CALL_EXPR))) > > + && CONVERSION_RANK (cand2->convs[0]) >= cr_user) > > + winner = -1; > > I don't know what you're trying to do here. The above passage describes how > we've always compared static to non-static, which should work the same way > for operator(). It doesn't work for the static operator() case vs. pointer returned by cast operator, with just the - int static_1 = DECL_STATIC_FUNCTION_P (cand1->fn); - int static_2 = DECL_STATIC_FUNCTION_P (cand2->fn); + int static_1 = (TREE_CODE (cand1->fn) == FUNCTION_DECL + && DECL_STATIC_FUNCTION_P (cand1->fn)); + int static_2 = (TREE_CODE (cand2->fn) == FUNCTION_DECL + && DECL_STATIC_FUNCTION_P (cand2->fn)); - if (DECL_CONSTRUCTOR_P (cand1->fn) + if (TREE_CODE (cand1->fn) == FUNCTION_DECL + && TREE_CODE (cand2->fn) == FUNCTION_DECL + && DECL_CONSTRUCTOR_P (cand1->fn) changes in joust so that it doesn't ICE, the call is ambiguous. Previously the standard had: "If F is a static member function, ICS1(F) is defined such that ICS1(F) is neither better nor worse than ICS1(G) for any function G, and, symmetrically, ICS1(G) is neither better nor worse than ICS1(F); otherwise," but that was removed and instead: "When the parameter is the implicit object parameter of a static member function, the implicit conversion sequence is a standard conversion sequence that is neither better nor worse than any other standard conversion sequence." was added elsewhere. The code implements the former. We don't have any conversion for the non-existing object parameter to static, so what I want to do is keep doing what we were before if the other candidate's conversion on the first parameter is standard (CONVERSION_RANK (cand2->convs[0]) < cr_user) and otherwise indicate that the static operator() has there a standard conversion and so it should be better than any user/ellipsis/bad conversion. Now, it could be done just for cxx_dialect >= cxx23 as that is pedantically what the standard says, but if we allow with a pedwarn static operator() in earlier standards, I think we better treat it there the same, because otherwise one can't call any static lambdas (all those calls would be ambiguous). > > @@ -1135,9 +1141,13 @@ maybe_add_lambda_conv_op (tree type) > > tree fn_args = NULL_TREE; > > { > > int ix = 0; > > - tree src = DECL_CHAIN (DECL_ARGUMENTS (callop)); > > + tree src = DECL_ARGUMENTS (callop); > > tree tgt = NULL; > > + if (thisarg) > > + src = DECL_CHAIN (src); > > You could use FUNCTION_FIRST_USER_PARM to skip 'this' if present. Ok, will try. > > + else if (!decltype_call) > > + src = NULL_TREE; > > while (src) > > { > > tree new_node = copy_node (src); > > @@ -1160,12 +1170,15 @@ maybe_add_lambda_conv_op (tree type) > > if (generic_lambda_p) > > { > > tree a = tgt; > > - if (DECL_PACK_P (tgt)) > > + if (thisarg) > > This is wrong, pack expansion handling has nothing to do with 'this'. Only > the assignment to CALL_EXPR_ARG should depend on thisarg. The point was that make_pack_expansion etc. is only used for what is later then stored to CALL_EXPR_ARG. As we don't need to construct call at all for the static lambda case, that looked like wasted effort. > > @@ -1203,14 +1216,18 @@ maybe_add_lambda_conv_op (tree type) > > direct_argvec->address ()); > > } > > - CALL_FROM_THUNK_P (call) = 1; > > - SET_EXPR_LOCATION (call, UNKNOWN_LOCATION); > > + if (thisarg) > > + { > > + CALL_FROM_THUNK_P (call) = 1; > > + SET_EXPR_LOCATION (call, UNKNOWN_LOCATION); > > + } > > - tree stattype = build_function_type (fn_result, FUNCTION_ARG_CHAIN (callop)); > > - stattype = (cp_build_type_attribute_variant > > - (stattype, TYPE_ATTRIBUTES (optype))); > > - if (flag_noexcept_type > > - && TYPE_NOTHROW_P (TREE_TYPE (callop))) > > + tree stattype > > + = build_function_type (fn_result, thisarg ? FUNCTION_ARG_CHAIN (callop) > > + : TYPE_ARG_TYPES (optype)); > > + stattype = cp_build_type_attribute_variant (stattype, > > + TYPE_ATTRIBUTES (optype)); > > + if (flag_noexcept_type && TYPE_NOTHROW_P (TREE_TYPE (callop))) > > stattype = build_exception_variant (stattype, noexcept_true_spec); > > If !thisarg, shouldn't stattype just be optype? No need to rebuild it in > that case. I thought it should be auto-deduced if it isn't yet. If it doesn't need to be, sure, we wouldn't need to bother with decltype_call for !thisarg either and just not do the loop on arguments etc. I've tried to create a testcase where I could see the generic static lambda but I failed to construct one, even with the template <...> lambda syntax I can't use a template id of the lambda to instantiate some particular case and then use *. Would passing a generic lambda to a function with some function pointer argument trigger it? Jakub