From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua1-x929.google.com (mail-ua1-x929.google.com [IPv6:2607:f8b0:4864:20::929]) by sourceware.org (Postfix) with ESMTPS id A00793858D37 for ; Tue, 20 Feb 2024 01:35:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A00793858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=cs.washington.edu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=cs.washington.edu ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A00793858D37 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::929 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708392945; cv=none; b=L/54Av+I3efr53MYVFyqXxpiryM9ylBs6M79YoV1mH0CGHqZoOwz06ShnkosH7gB2xYKcxAYPw+dfVKWgRJZWV/AhVnv/RFtbW4j9bNZiIXXQK7noXu26tHJzT7516pktSUSHpF5RQRwimd3NBmIrgexNvD/DkuC50Co2EnCTbA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708392945; c=relaxed/simple; bh=mNQCDufHIU97/zuibuX7YJSuSAqdNEPBpSbHR972Wzw=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=fNwezBfyLAsXTg7DDSX6NZDBFY9MbfSZAIQra0+hnz7/lyutx3ItDIncCZHFhjimucRf9JUxbEz5HSPngnSfOzljOtZopt1g0cmuOz8lc+IaByGvCiO6yaCT2jibh21e16P4xJqeKNqjE0pcHjtgLd8GEY0PSzuiRxMMKuPa2os= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-ua1-x929.google.com with SMTP id a1e0cc1a2514c-7d5cbc4a585so1108173241.3 for ; Mon, 19 Feb 2024 17:35:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cs.washington.edu; s=goo201206; t=1708392941; x=1708997741; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=lctIeq6/j8l0a4OOITiUwBdRtY0No/wCXekClkRPfgw=; b=LeDtzA5+kFsOZ6PVmrm9VQhzhbskz4pfTuzSBWLC52P9yp1manJT5KTlVG+E7rmNL7 T9mp9rWoaAADJH+l1OIJ6o/jfzK2tE+qu/WSCUE6OAAbapr49yidzNQ4kAHmlaKwDuAO b+U26GNRvb3veGPsbAXJkG3IScToUpmpbUkC0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708392941; x=1708997741; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=lctIeq6/j8l0a4OOITiUwBdRtY0No/wCXekClkRPfgw=; b=V6ytEidv7zyt66pMNXfJ9zYaewvHCoi6n5XOANSvkwgBLEdA8vE85yByGF/NKPKmgt 2iPJ+pEuJtc2wZop5jCjP/KrYe1raX7wrTuGNnd91PyMIEXz5qYzyVrjhOkjh5xfymzv U1SNI97KyfW4Jz+Cnau+qD1zk7T1Ew76rsYA+LL2d7PocVo0C/ZUwHfXpreFQATkDT1a DX1wocjrAaokP6HwE4zNr5y0rNddn86vF9y0hSm9OIPMPm52kx+SfsXFVcPjowmDqQZU vehlyV6g9gB1FqIuUWCmY3HoiKBjAGUGHdfMF/iYWz8G2Ol7zJXmFWBq3LErEqgl/JR1 5r4g== X-Forwarded-Encrypted: i=1; AJvYcCXKw+vpx5BXTC0/a8Fl3YBgaUcN6e2vLYnRGb8dTu+YJbTH6L2mDZAP6aHhBj85gxAnAv3mF6b9APaawyXsDUgRmrsR/vk= X-Gm-Message-State: AOJu0Yz0oyaoVFKogR530517y8u4GMV/tBaPbBt1R0VLikJAhHNhYFse ZrN1dgvZy0ZUsElmJ3xs/XeVp4Afa/6IiuNWyVFhcfbIsVReK0K9KaEWRlvuer1TC96pPQVc23O o4/J37e3zb/tDgB594I4VFLpzhxBVtdWNRAzd X-Google-Smtp-Source: AGHT+IED0/YeTC31Vd7542IfWrIJSiVH0BYXUFA3bWq/P0Ww/iWlEGUZ+G8ANYC2PUV0N74oDSx8V+IAaa0mIu1zoeQ= X-Received: by 2002:a05:6102:18ca:b0:470:6009:16cd with SMTP id jj10-20020a05610218ca00b00470600916cdmr2556202vsb.33.1708392940718; Mon, 19 Feb 2024 17:35:40 -0800 (PST) MIME-Version: 1.0 References: <20231017113822.677344-1-kmatsui@gcc.gnu.org> <20231020135748.1846670-1-kmatsui@gcc.gnu.org> <20231020135748.1846670-33-kmatsui@gcc.gnu.org> <024758c0-21f1-ef6e-4bc7-e615a7084c86@idea> In-Reply-To: From: Ken Matsui Date: Mon, 19 Feb 2024 17:35:04 -0800 Message-ID: Subject: Re: [PATCH v23 32/33] c++: Implement __is_invocable built-in trait To: Jason Merrill Cc: Ken Matsui , gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org, Patrick Palka Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,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 List-Id: On Mon, Oct 23, 2023 at 2:23=E2=80=AFPM Jason Merrill wr= ote: > > On 10/20/23 17:37, Patrick Palka wrote: > > On Fri, 20 Oct 2023, Patrick Palka wrote: > > > >> On Fri, 20 Oct 2023, Patrick Palka wrote: > >> > >>> On Fri, 20 Oct 2023, Ken Matsui wrote: > >>> > >>>> This patch implements built-in trait for std::is_invocable. > >>> > >>> Nice! My email client unfortunately ate my first review attempt, so > >>> apologies for my brevity this time around. > >>> > >>>> gcc/cp/ChangeLog: > >>>> > >>>> * cp-trait.def: Define __is_invocable. > >>>> * constraint.cc (diagnose_trait_expr): Handle CPTK_IS_INVOCABLE. > >>>> * semantics.cc (trait_expr_value): Likewise. > >>>> (finish_trait_expr): Likewise. > >>>> (is_invocable_p): New function. > >>>> * method.h: New file to export build_trait_object in method.cc. > > Given how much larger semantics.cc is than method.cc, maybe let's put > is_invocable_p in method.cc instead? And in general declarations can go > in cp-tree.h. > > >>>> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc > >>>> index 7cccbae5287..cc2e400531a 100644 > >>>> --- a/gcc/cp/semantics.cc > >>>> +++ b/gcc/cp/semantics.cc > >>>> @@ -45,6 +45,10 @@ along with GCC; see the file COPYING3. If not se= e > >>>> #include "gomp-constants.h" > >>>> #include "predict.h" > >>>> #include "memmodel.h" > >>>> +#include "method.h" > >>>> + > >>>> +#include "print-tree.h" > >>>> +#include "tree-pretty-print.h" > >>>> > >>>> /* There routines provide a modular interface to perform many pars= ing > >>>> operations. They may therefore be used during actual parsing, = or > >>>> @@ -11714,6 +11718,133 @@ classtype_has_nothrow_assign_or_copy_p (tr= ee type, bool assign_p) > >>>> return saw_copy; > >>>> } > >>>> > >>>> +/* Return true if FN_TYPE is invocable with the given ARG_TYPES. *= / > >>>> + > >>>> +static bool > >>>> +is_invocable_p (tree fn_type, tree arg_types) > > > > (Sorry for the spam) We'll eventually want to implement a built-in for > > invoke_result, so perhaps we should preemptively factor out the bulk > > of this function into a 'build_INVOKE' helper function that returns the > > built tree? > > > >>>> +{ > >>>> + /* ARG_TYPES must be a TREE_VEC. */ > >>>> + gcc_assert (TREE_CODE (arg_types) =3D=3D TREE_VEC); > >>>> + > >>>> + /* Access check is required to determine if the given is invocabl= e. */ > >>>> + deferring_access_check_sentinel acs (dk_no_deferred); > >>>> + > >>>> + /* std::is_invocable is an unevaluated context. */ > >>>> + cp_unevaluated cp_uneval_guard; > >>>> + > >>>> + bool is_ptrdatamem; > >>>> + bool is_ptrmemfunc; > >>>> + if (TREE_CODE (fn_type) =3D=3D REFERENCE_TYPE) > >>>> + { > >>>> + tree deref_fn_type =3D TREE_TYPE (fn_type); > >>>> + is_ptrdatamem =3D TYPE_PTRDATAMEM_P (deref_fn_type); > >>>> + is_ptrmemfunc =3D TYPE_PTRMEMFUNC_P (deref_fn_type); > >>>> + > >>>> + /* Dereference fn_type if it is a pointer to member. */ > >>>> + if (is_ptrdatamem || is_ptrmemfunc) > >>>> + fn_type =3D deref_fn_type; > >>>> + } > >>>> + else > >>>> + { > >>>> + is_ptrdatamem =3D TYPE_PTRDATAMEM_P (fn_type); > >>>> + is_ptrmemfunc =3D TYPE_PTRMEMFUNC_P (fn_type); > >>>> + } > >>>> + > >>>> + if (is_ptrdatamem && TREE_VEC_LENGTH (arg_types) !=3D 1) > >>>> + /* A pointer to data member with non-one argument is not invoca= ble. */ > >>>> + return false; > >>>> + > >>>> + if (is_ptrmemfunc && TREE_VEC_LENGTH (arg_types) =3D=3D 0) > >>>> + /* A pointer to member function with no arguments is not invoca= ble. */ > >>>> + return false; > >>>> + > >>>> + /* Construct an expression of a pointer to member. */ > >>>> + tree datum; > >>>> + if (is_ptrdatamem || is_ptrmemfunc) > >>>> + { > >>>> + tree datum_type =3D TREE_VEC_ELT (arg_types, 0); > >>>> + > >>>> + /* Dereference datum. */ > >>>> + if (CLASS_TYPE_P (datum_type)) > >>>> + { > >>>> + bool is_refwrap =3D false; > >>>> + > >>>> + tree datum_decl =3D TYPE_NAME (TYPE_MAIN_VARIANT (datum_type)); > >>>> + if (decl_in_std_namespace_p (datum_decl)) > >>>> + { > >>>> + tree name =3D DECL_NAME (datum_decl); > >>>> + if (name && (id_equal (name, "reference_wrapper"))) > >>>> + { > >>>> + /* Handle std::reference_wrapper. */ > >>>> + is_refwrap =3D true; > >>>> + datum_type =3D cp_build_reference_type (datum_type, fal= se); > > Why do you change datum_type from std::reference_wrapper<...> to > std::reference_wrapper<...>&? > > >>>> + } > >>>> + } > >>>> + > >>>> + datum =3D build_trait_object (datum_type); > >>>> + > >>>> + /* If datum_type was not std::reference_wrapper, check if it ha= s > >>>> + operator*() overload. If datum_type was std::reference_wrap= per, > >>>> + avoid dereferencing the datum twice. */ > >>>> + if (!is_refwrap) > >>>> + if (get_class_binding (datum_type, get_identifier ("operator*= "))) > >>> > >>> We probably should use lookup_member instead of get_class_binding sin= ce > >>> IIUC the latter doesn't look into bases: > >>> > >>> struct A { int m; }; > >>> struct B { A& operator*(): }; > >>> struct C : B { }; > >>> static_assert(std::is_invocable_v); > >>> > >>> However, I notice that the specification of INVOKE > >>> (https://eel.is/c++draft/func.require#lib:INVOKE) doesn't mention nam= e > >>> lookup at all so it strikes me as suspicious that we'd perform name > >>> lookup here. > > Agreed. It seems that whether or not to build_x_indirect_ref should > depend instead on whether f is a pointer to a member of decltype(t1) (as > well as is_refwrap). > > >>> I think this would misbehave for: > >>> > >>> struct A { }; > >>> struct B : A { A& operator*() =3D delete; }; > >>> static_assert(std::is_invocable_v); > >>> > >>> struct C : private A { A& operator*(); }; > >>> static_assert(std::is_invocable_v); > >> > >> Oops, this static_assert is missing a ! > >> > >>> > >>> ultimately because we end up choosing the dereference form of INVOKE, > >>> but according to 1.1/1.4 we should choose the non-dereference form? > >>> > >>>> + /* Handle operator*(). */ > >>>> + datum =3D build_x_indirect_ref (UNKNOWN_LOCATION, datum, > >>>> + RO_UNARY_STAR, NULL_TREE, > >>>> + tf_none); > >>>> + } > >>>> + else if (POINTER_TYPE_P (datum_type)) > >>>> + datum =3D build_trait_object (TREE_TYPE (datum_type)); > >>>> + else > >>>> + datum =3D build_trait_object (datum_type); > >>>> + } > >>>> + > >>>> + /* Build a function expression. */ > >>>> + tree fn; > >>>> + if (is_ptrdatamem) > >>>> + fn =3D build_m_component_ref (datum, build_trait_object (fn_typ= e), tf_none); > >>> > >>> Maybe exit early for the is_ptrdatamem case here (and simplify the re= st > >>> of the function accordingly)? > >>> > >>>> + else if (is_ptrmemfunc) > >>>> + fn =3D build_trait_object (TYPE_PTRMEMFUNC_FN_TYPE (fn_type)); > > Why not use build_m_component_ref and build_offset_ref_call_from_tree > like it would if you wrote (t1.*f)() directly? > Thank you so much for your review! I will apply your suggestions. > Jason >