From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-40134.protonmail.ch (mail-40134.protonmail.ch [185.70.40.134]) by sourceware.org (Postfix) with ESMTPS id 523A0385B52D for ; Sat, 28 Oct 2023 04:07:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 523A0385B52D Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=protonmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=protonmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 523A0385B52D Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=185.70.40.134 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698466069; cv=none; b=NgxRxETM5YH1w1vffBPnaG8myPzgJH1DQmMtl3bv5o+huVziF4B9BOm4Hb4806coC4rhosMZi3qQcCkCrR3+wgh/Kj0VgietHlIJh/u8hJ2uxV8ooN97cMNC7lukmqIKE9CFLrw9FgYoiFirGCUltVYfS37rKJGHQ8j9MMLkzp4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698466069; c=relaxed/simple; bh=JKsxV9hiHA+T5Qyk2mmgaI4B6Cz4lUSapggqnIiWGlI=; h=DKIM-Signature:Date:To:From:Subject:Message-ID:MIME-Version; b=OdvhqSENpHFsuLQy37OaJXpYCW2bSYN6Jzbffkm3+jM1vtbHPCMwpinkJcK07qix5+8AiigizUkfeNLWKtuFcirl3HbyNVRqAAYyJiqueM3BK3nk49zK0U92y2zTLB91kPBjYLq8XoD2h/bsvITDhZjnxY3FnPeC5BRGpyFbS9c= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1698466065; x=1698725265; bh=ogvASKH/RuioiAs7uHMs8Mzay4gSOBMOXe2KRCkx2/M=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=CeoNmZgYDjdtIdEOErs1RpqOP4OQ3t8Q9SdRSWOm0PxVC/eA02RNRWxpnvO1KOmqD CDOePlXqcdUBdmMI47vTR2Mj0VKMEA/jJvQYfDA+LdARyssBfALT06MrkOM1q8MXkX tgrbd+yvz0VkE+q0Koe7o3dU6l2ff7/lKn3itS8RCLaTS2CzaVf7o6k+zbNk9pxInl mq9G91WKhuil85PVFLNMRsRngFHHFj+oTvas7/6NH9jNHuqujslmAfa83zKi+5gY4d mPCsnkpkx2g7XD23+mW1fd9TGxXDv+P1vT9ZYNDVyEfbgU8y730ft7p4bkYV7P+QBx CKJFIzNasMJ+A== Date: Sat, 28 Oct 2023 04:07:36 +0000 To: Jason Merrill From: waffl3x Cc: "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609] Message-ID: In-Reply-To: <635f5d6d-2807-4dff-8a37-73d323d6ea97@redhat.com> References: <0cc5b21d-4b27-4964-bec3-544c86307c74@redhat.com> <1fdadb6b-e4ca-40c1-bb1c-43a0f42826ba@redhat.com> <635f5d6d-2807-4dff-8a37-73d323d6ea97@redhat.com> Feedback-ID: 14591686:user:proton MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,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 List-Id: I've been under the weather so I took a few days break, I honestly was also very reluctant to pick it back up. The current problem is what I like to call "not friendly", but I am back at it now. > > I don't understand what this means exactly, under what circumstances > > would &f find the member function. Oh, I guess while in the body of > > it's class, I hadn't considered that. Is that what you're referring to? > > > Right: > > struct A { > void g(this A&); > A() { > &A::g; // ok > &g; // same error as for an implicit object member function > } > }; I fully get this now, I threw together a test for it so this case doesn't get forgotten about. Unfortunately though, I am concerned that the approach I was going to take to fix the crash would have the incorrect behavior for this. Here is what I added to cp_build_addr_expr_1 with context included. ``` case OFFSET_REF: offset_ref: /* Turn a reference to a non-static data member into a =09 pointer-to-member. */ { =09tree type; =09tree t; =09gcc_assert (PTRMEM_OK_P (arg)); =09t =3D TREE_OPERAND (arg, 1); =09if (TYPE_REF_P (TREE_TYPE (t))) =09 { =09 if (complain & tf_error) =09 error_at (loc, =09=09=09"cannot create pointer to reference member %qD", t); =09 return error_mark_node; =09 } /* -- Waffl3x additions start -- */ =09/* Exception for non-overloaded explicit object member function. */ =09if (TREE_CODE (TREE_TYPE (t)) =3D=3D FUNCTION_TYPE) =09 return build1 (ADDR_EXPR, unknown_type_node, arg); /* -- Waffl3x additions end -- */ =09/* Forming a pointer-to-member is a use of non-pure-virtual fns. */ =09if (TREE_CODE (t) =3D=3D FUNCTION_DECL =09 && !DECL_PURE_VIRTUAL_P (t) =09 && !mark_used (t, complain) && !(complain & tf_error)) =09 return error_mark_node; ``` I had hoped this naive solution would work just fine, but unfortunately the following code fails to compile with an error. ``` struct S { void f(this S&) {} }; int main() { void (*a)(S&) =3D &S::f; } ``` normal_static.C: In function =E2=80=98int main()=E2=80=99: normal_static.C:13:25: error: cannot convert =E2=80=98S::f=E2=80=99 from ty= pe =E2=80=98void(S&)=E2=80=99 to type =E2=80=98void (*)(S&)=E2=80=99 13 | void (*a)(S&) =3D &S::f; | ^ So clearly it isn't going to be that easy. I went up and down looking at how the single static case is handled, and tried to read the code in build_ptrmem_type and build_ptrmemfunc_type but I had a hard time figuring it out. The good news is, this problem was difficult enough that it made me pick a proper diff tool with regex support instead of using a silly web browser tool and pasting things into it. Or worse, pasting them into a tool and doing replacement and then pasting them into the silly web browser tool. I have been forced to improve my workflow thanks to this head scratcher. So it's not all for naught. Back on topic, it's not really the optimization returning a baselink that is causing the original crash. It's just the assert in build_ptrmem_type failing when a FUNCTION_TYPE is reaching it. The optimization did throw me for a loop when I was comparing how my previous version (that incorrectly set the lang_decl_fn :: static_function flag) was handling things. Looking back, I think I explained myself and the methodology I was using to investigate really poorly, I apologize for the confusion I might have caused :). To state it plainly, it seems to me that the arg parameter being passed into cp_build_addr_expr_1 for explicit object member functions is (mostly) pretty much correct and what we would want. So the whole thing with the baselink optimization was really just a red herring that I was following. Now that I have a better understanding of what's happening leading up to and in cp_build_addr_expr_1 I don't think it's relevant at all for this problem. With that said, I am questioning again if the optimization that returns a baselink node is the right way to do things. So this is something I'm going to put down into my little notes text file to investigate at a later time, and forget about it for the moment as it shouldn't be causing any friction for us here. Anyway, as I eluded to above, if I can't figure out the right way to solve this problem in a decent amount of time I think I'm going to leave it for now. I'll come back to it once other higher priority things are fixed or finished. And hopefully someone more familiar with this area of the code will have a better idea of what we need to do to handle this case in a non-intrusive manner. That wraps up my current status on this specifically. But while investigating it I uncovered a few things that I feel are important to discuss/note. I wanted to change DECL_NONSTATIC_MEMBER_FUNCTION_P to include explicit object member functions, but it had some problems when I made the modification. I also noticed that it's used in cp-objcp-common.cc so would making changes to it be a bad idea? -- cp-tree.h ``` /* Nonzero for FUNCTION_DECL means that this decl is a non-static member function. */ #define DECL_NONSTATIC_MEMBER_FUNCTION_P(NODE) \ (TREE_CODE (TREE_TYPE (NODE)) =3D=3D METHOD_TYPE) ``` I didn't want to investigate the problems as I was knee deep in investigating the addressof bug. So I instead modified DECL_FUNCTION_MEMBER_P to include explicit object member functions and moved on. -- cp-tree.h ``` /* Nonzero for FUNCTION_DECL means that this decl is a member function (static or non-static). */ #define DECL_FUNCTION_MEMBER_P(NODE) \ (DECL_NONSTATIC_MEMBER_FUNCTION_P (NODE) || DECL_STATIC_FUNCTION_P (NODE)= \ || DECL_IS_XOBJ_MEMBER_FUNC (NODE)) ``` I am mostly just mentioning this here in case it becomes more relevant later. Looking at how much DECL_NONSTATIC_MEMBER_FUNCTION_P is used throughout the code I now suspect that adding explicit object member functions to it might cause xobj member functions to be treated as regular member functions when they should not be. If this change were to stick it would cause a discrepancy in the behavior of DECL_NONSTATIC_MEMBER_FUNCTION_P and it's name. If we were to do this, I think it's important we document the discrepancy and why it exists, and in the future, it should possibly be refactored. One option would be to simply rename it to DECL_IOBJ_MEMBER_FUNCTION_P. After all, I suspect that it's unlikely that the current macro (DECL_NONSTATIC_MEMBER_FUNCTION_P) is being used in places that concern explicit object member functions. So just adding explicit object member functions to it will most likely just result in headaches. It seems to me that would be the best solution, so when and if it comes up again, I think that route should be considered. Secondly, there are some differences in the nodes describing an explicit object member function from those describing static member functions and implicit object member functions that I am not sure should be present. I did my best to summarize the differences, if you want the logs of tree_debug that I derived them from I can provide them. Most of my understanding of the layout of the nodes is from reading print-tree.cc and looking at debug_tree outputs, so it's possible I made a mistake. I am opting to use the names of members as they are output by debug_tree, I recognize this is not always the actual name of the member in the actual tree_node structures. Additionally, some of the differences listed are to be expected and are most likely the correct values for each node. However, I wanted to be exhaustive when listing them just in case I am mistaken in my opinion on whether the differences should or should not occur. The following declarations were used as input to the compiler. iobj decl: struct S { void f() {} }; xobj decl: struct S { void f(this S&) {} }; static decl: struct S { static void f(S&) {} }; These differences can be observed in the return values of grokdeclarator for each declaration. 1. function_decl::type::tree_code iobj: method_type xobj: function_type stat: function_type 2. function_decl::type::method basetype iobj: xobj: NULL/no output stat: NULL/no output 3. function_decl::type::arg-types::tree_list[0]::value iobj: xobj: stat: 4. function_decl::decl_6 iobj: false/no output xobj: false/no output stat: true 5. function_decl::align iobj: 16 xobj: 8 stat: 8 6. function_decl::result::uid iobj: D.2513 xobj: D.2513 stat: D.2512 7. function_decl::full-name iobj: "void S::f()" xobj: "void S::f(this S&)" Differences 1, 3, and 7 seem obviously correct to me for all 3 declarations, 6 is a little bizarre to me, but since it's just a UID it's merely an oddity, I doubt it is concerning. That leaves 2, 4, and 5. 2. I am pretty sure xobj functions should have the struct they are a part of recorded as the method basetype member. I have already checked that function_type and method_type are the same node type under the hood and it does appear to be, so it should be trivial to set it. However I do have to wonder why static member functions don't set it, it seems to be that it would be convenient to use the same field. Can you provide any insight into that? 4. I have no comment here, but it does concern me since I don't understand it at all. 5. I am pretty sure this is fine for now, but if xobj member functions ever were to support virtual/override capabilities, then it would be a problem. Is my understanding correct, or is there some other reason that iobj member functions have a different value here? There are also some differences in the arg param in cp_build_addr_expr_1 that concerns me, but most of them are reflected in the differences I have already noted. I had wanted to include these differences as well but I have been spending too much time staring at it, it's no longer productive. In short, the indirect_ref node for xobj member functions has reference_to_this set, while iobj member functions do not. The baselink binfo field has the private flag set for xobj member functions, iobj member functions does not. I've spent too much time on this write up, so I am calling it here, it wasn't all a waste of time because half of what I was doing here are things I was going to need to do anyway at least. I still feel like I spent too much time on it. Hopefully it's of some value for me/others later. Alex