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 803583858D33 for ; Mon, 27 Nov 2023 01:16:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 803583858D33 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 803583858D33 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701047794; cv=none; b=G+koszPaPQ09VbUveCUDweAtabK6hBpkQXTwdDCJvExEQNswIkuVJ5rxWlZFCwn+3tWgs2OWM1XCxSQPM7ZIGlUr3xtrRyQTKGSKdQAcdzrMfdHsnQsnxCZMEQEP8afK+HjKtVqapZSxyP3Ack4lOzmu2g91/XL+QmEyaHd0XF8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701047794; c=relaxed/simple; bh=OFr2uLK0v6qfyYmpxDyyYh/rtGTvaXfDgkdhar6zRaU=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=eEmRJyD89TV5aPRQK3gxUvBAXzcuBXQDBPB28gOAMvwToppvI1RNxyL/QQhUIVx6CPluGzK3Hsg5VvSvDUVaUUgcBAvTuE6AIQd0FJoX32jge/2zqjGnJlGPuuQWtP0RyEWKe5Nw2DwVBcG+JBtW3VyUsUuUEyg+at7LRTNicCY= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1701047792; 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: in-reply-to:in-reply-to:references:references; bh=GRm7wejhGIaYs80i0ctV7s7q+ypjYMiP5oCwki3ie9g=; b=XIiZiAKGMKIdvrHHMaPiQaOAxUKbR9ZBc5NftQmegraFv9CDJ/YoIEaCCuSoHcWJLAgT0B H7tW4jFQdRVRD5Ec6AFOCXYMxmTAdlGD0+Odf+dfqpHb9QuGOmVbZo5H8eiLyIxV1rMRY6 rULEBId/tuw9/yiVzkmK8uiNBBxGHTY= Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-625-ghDZlRGsMWCVL_o8Jm_8gw-1; Sun, 26 Nov 2023 20:16:30 -0500 X-MC-Unique: ghDZlRGsMWCVL_o8Jm_8gw-1 Received: by mail-qk1-f200.google.com with SMTP id af79cd13be357-779f645c74cso394896885a.0 for ; Sun, 26 Nov 2023 17:16:30 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701047789; x=1701652589; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=GRm7wejhGIaYs80i0ctV7s7q+ypjYMiP5oCwki3ie9g=; b=DpcDi3YnDrcYTUETAo3wvXkiiFad/A7I+zoqxGjqMb1JkGyh+4lofqj1syLRiDiY2s bJqTTY6TxN+c5qIvEBybsqhsl3x8VPPeIIcUtR1TstlGVJa8XqmufH4UvSHQ7kOEx4Ej fWQmlcF5VdmULfXoGWkANGfmyIxY2ClMAC9iVOqqrQ7HKGFZCiKXEVivf0Z7RC48LK+V wbkqYo/St8N5BEAONJbjVRxLtKOhRrWQ91vdpb27by3ZLwsP88ZYGJMEOxziHgCUAd38 QUfpw3nWhGXVORpduONB3ns9XKSQdFqWFEivhGbIabAtFuUEhlNb8hlzoAToEHNXeUpo Hm2g== X-Gm-Message-State: AOJu0YyYVNVUtZQxyc1wRfdvi31IpfKaVTnxgkbP9KFL8R8/eEyMxFJW r5Aa04K31om2Z2y8x4nGgb441dmdcf2ewYwjhoGmpZZY2isdOafuSakphcCvAbu9xY8VBSEz6nK v7uJBYvRkh0EZUqca/w== X-Received: by 2002:a05:620a:12d8:b0:77d:6046:4f74 with SMTP id e24-20020a05620a12d800b0077d60464f74mr9462927qkl.21.1701047789521; Sun, 26 Nov 2023 17:16:29 -0800 (PST) X-Google-Smtp-Source: AGHT+IHqlwNRNVqgWWwCWSA0al4fwla4EDK/LXnB67LftjbA/o5bhEaOBObCE3YieXKuOJ7Wc5Rn2Q== X-Received: by 2002:a05:620a:12d8:b0:77d:6046:4f74 with SMTP id e24-20020a05620a12d800b0077d60464f74mr9462908qkl.21.1701047789091; Sun, 26 Nov 2023 17:16:29 -0800 (PST) Received: from [192.168.1.145] (130-44-146-16.s12558.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.146.16]) by smtp.gmail.com with ESMTPSA id vz18-20020a05620a495200b0077d5c5af0c1sm3293178qkn.6.2023.11.26.17.16.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 26 Nov 2023 17:16:28 -0800 (PST) Message-ID: <7623e2db-ba29-42f2-85df-c2e796d7305b@redhat.com> Date: Sun, 26 Nov 2023 20:16:27 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609] To: waffl3x Cc: "gcc-patches@gcc.gnu.org" References: <9OlK_2c3Punfm3w-wEHqyHGyKGG8Gr_K0BUnDOuC9Aazur4mWlAM5WuL1Ea0AMLvFLl6LKFVTs813yY0zA7m0_ji_R9qhE52G7MZXrVPfZE=@protonmail.com> <7Xr5Vil7ptZzPaCtc_ZCdcTPuUVY7dheOnklF-vVDb5_jl8PivYWgTT_f3cKLvg7IMnDruCDDrICRI6WVrUT3f8ZScGKAh4ATIkYSuRqPZc=@protonmail.com> <-SP7aKgN1FZED-RAPr2FBDuCrcwnu9-UhHcRXNEsNZRwIzJXCdhVbtBP921Yn8g71d0WL7XpFRetUlBAStzRpZB8p4yj5moRS0DIE9D6cnY=@protonmail.com> From: Jason Merrill In-Reply-To: <-SP7aKgN1FZED-RAPr2FBDuCrcwnu9-UhHcRXNEsNZRwIzJXCdhVbtBP921Yn8g71d0WL7XpFRetUlBAStzRpZB8p4yj5moRS0DIE9D6cnY=@protonmail.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,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 List-Id: On 11/26/23 18:10, waffl3x wrote: > On Sunday, November 26th, 2023 at 2:30 PM, Jason Merrill wrote: >> On 11/24/23 20:14, waffl3x wrote: >> >>> OKAY, I figured out SOMETHING, I think this should be fine. As noted in >>> the comments, this might be a better way of handling the static lambda >>> case too. This is still a nasty hack so it should probably be done >>> differently, but I question if making a whole new fntype node in >>> tsubst_lambda_expr makes sense. On the other hand, maybe there will be >>> problems if a lambda is already instantiated? I'm not sure, mutating >>> things this way makes me uneasy though. >> >> A problem with changing TYPE_METHOD_BASETYPE for a FUNCTION_TYPE is that >> all equivalent FUNCTION_TYPE share the same tree node (through >> type_hash_canon), so you're messing with the type of unrelated functions >> at the same time. I think it's better to stick with the way static >> lambdas are handled. > > Yes, that was my concern as well, without even thinking about hashing. > I will investigate it deeper but I'm pretty sure TYPE_METHOD_BASETYPE, > while a valid field, is not used at all for function_type nodes. I had > to look into this when looking into the DECL_CONTEXT stuff previously, > so I'm pretty confident in this. Come to think of it, does hashing even > take fields that aren't "supposed" to be set for a node into account? It doesn't. The issue is messing with the type of (potentially) a lot of different functions. Even if it doesn't actually break anything, it seems like the kind of hidden mutation that you were objecting to. > Well, even so, I can just clear it after it gets used as we just need > it to pass the closure type down. Perhaps I should have led with this, > but as it stands the version that uses TYPE_METHOD_BASETYPE bootstraps > with no regressions. I'll still look deeper but I'm pretty confident in > my decision here, I really don't want to try to unravel what > build_memfn_type does, I would rather find a whole different way of > passing that information down. But the existing code already works fine, it's just a question of changing the conditions so we handle xob lambdas the same way as static. >>> Regarding the error handling, I just had a thought about it, I have a >>> hunch it definitely needs to go in tsubst_template_decl or >>> tsubst_function_decl. There might need to be more changes to determine >>> the actual type of the lambda in there, but everything else I've done >>> changes the implicit object argument to be treated more like a regular >>> argument, doing error handling for the object type in >>> tsubst_lambda_expr would be inconsistent with that. >> >> But the rule about unrelated type is specific to lambdas, so doing it in >> tsubst_lambda_expr seems preferable to adding a lambda special case to >> generic code. > > No, while that might seem intuitive, the error is not during > instantiation of the closure object's type, it's during instantiation > of the function template. The type of the closure will always be the > type of the closure, but the type of the explicit object parameter can > be different. In the following example we don't go into > tsubst_lambda_expr at all. Ah, good point. > I hope this demonstrates that placing the check in tsubst_function_decl > is the correct way to do this. Sounds right. > Now with that out of the way, I do still kind of feel icky about it. > This really is a MASSIVE edge case that will almost never matter. As > far as I know, instantiating explicitly like so... > > auto f = [x = 42](this auto&&) -> int { return x; }; > int (*fp)(int&) = &decltype(f)::operator(); > > ...is the only way to coerce the explicit object parameter of the > lambda's call operator into deducing as an unrelated type. Cases that > are not deduced can be caught trivially while parsing the lambda and > are the only reasonable cases where you might have an unrelated type. > Perhaps it might become relevant in the future if a proposal like > https://atomgalaxy.github.io/isocpp-1107/D1107.html ever made it in, > but we aren't there yet. So as it stands, I'm pretty certain that it's > just impossible to instantiate a lambda's call operator with an > unrelated xobj parameter type except for the above case with > address-of. If you can think of another, please do share, but I've > spent a fair amount of time on it and came up with nothing. I think you're right. > Anyway, due to this, I am not currently concerned about the lack of a > diagnostic, and I believe my implementation is the most correct way to > do it. The problem with resolve_address_of_overloaded_function that I > go into below (depending on if you agree that it's a problem) is what > needs to be fixed to allow the current diagnostic to be properly > emitted. As of right now, there is no execution path that I know of > where the diagnostic will be properly emitted. > > I go into more detail about this in the comment in > tsubst_function_decl, although I do have to revise it a bit as I wrote > it before I had a discussion with another developer on the correct > behavior of the following case. I previously wondered if the overload > resolution from initializing p1 should result in a hard fault. > > template > struct S : T { > using T::operator(); > void operator()(this int&, auto) {} > }; > > int main() > { > auto s0 = S{[](this auto&&, auto){}}; > // error, ambiguous > void (*p0)(int&, int) = &decltype(s0)::operator(); > > auto s1 = S{[x = 42](this auto&&, auto){}}; > // SFINAE's, calls S::operator() > void (*p1)(int&, int) = &decltype(s1)::operator(); > } > > The wording in [expr.prim.lambda.closure-5] is similar to that in > [dcl.fct-15], and we harness SFINAE in that case, so he concluded that > this case (initializing p1) should also harness SFINAE. Agreed. >>> The other problem I'm having is >>> >>> auto f0 = [n = 5, &m](this auto const&){ n = 10; }; >>> This errors just fine, the lambda is unconditionally const so >>> LAMBDA_EXPR_MUTABLE_P flag is set for the closure. >>> >>> This on the other hand does not. The constness of the captures depends >>> on (I assume) LAMBDA_EXPR_MUTABLE_P so all the captures are non-const >>> here. >>> auto f1 = [n = 5](this auto&& self){ n = 10; }; >>> as_const(f1)(); >> >> That sounds correct, why would this be an error? >> >> The constness of the captures doesn't depend on LAMBDA_EXPR_MUTABLE_P, >> it depends on the type of the object parameter, which in this case is >> non-const, so so are the captures. > > Oops, I should have just made a less terse example, you missed the > "as_const" in the call to the lambda. The object parameter should be > deduced as const here. I definitely should have made that example > better, my bad. Ah, yes, I see it now. >>> I was going to close out this message there but I just realized why >>> exactly you think erroring in instantiate_body is too late, it's >>> because at that point an error is an error, while if we error a little >>> bit earlier during substitution it's not a hard error. I'm glad I >>> realized this now, because I am much more confident in how to implement >>> the errors for unrelated type now. I'm still a little confused on what >>> the exact protocol for emitting errors at this stage is, as there >>> aren't many explicit errors written in. It seems to me like the errors >>> are supposed to be emitted when calling back into non-template stages >>> of the compiler with substituted types. It also seems like that hasn't >>> always been strictly followed, and I hate to contribute to code debt >>> but I'm not sure how I would do it in this case, nor if I actually have >>> a valid understanding of how this all works. >> >> Most errors that could occur in template substitution are guarded by if >> (complain & tf_error) so that they aren't emitted during deduction >> substitution. It's better if those are in code that's shared with the >> non-template case, but sometimes that isn't feasible. > > Yeah, makes sense, but the case going through > resolve_address_of_overloaded_function -> fn_type_unification -> > instantiate_template -> tsubst_decl -> tsubst_function_decl does not > behave super nicely. I tried adding (complain & tf_error) to the > explain_p parameter of fn_type_unification in > resolve_address_of_overloaded_function, but I immediately learned why > that wasn't being handled that way. I think > resolve_address_of_overloaded_function has a number of problems and > should probably use print_z_candidates instead of print_candidates in > the long term. This would actually solve the problem in > tsubst_function_decl where it never emits an error for an unrelated > type, print_z_candidates does call fn_type_unification with true passed > to explain_p. I go into this in detail in the large comment in > tsubst_function_decl (that I have to revise) so I won't waste time > rehashing it here. Makes sense, we won't see a message about the unrelated type because print_candidates doesn't repeat deduction like print_z_candidates does. > Thank you for explaining though, some of the code doesn't reflect the > method you've relayed here (such as an unguarded error_at in > tsubst_function_decl) so I was starting to get confused on what exactly > is supposed to be going on. This clarifies things for me. That unguarded error does indeed look suspicious, though it could be that OMP reductions are called in a sufficiently unusual way that it isn't actually a problem in practice. /shrug Jason