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 1D13F384AB59 for ; Thu, 25 Apr 2024 18:34:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1D13F384AB59 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 1D13F384AB59 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=1714070069; cv=none; b=eKYbd/epLkZNso5PIr6pGJ2tLsv1DuDRXyn8MlIY9guc4J7ZU6VdrT/RrJXpb18Rs9F2u8Y6jAT2pKOxqEXZPmvk4+Nikanrenf5GnHSLfFj67IVQ5H8cU8orZFbzBiQSrAcXZc91NieBcbLKvrHVE67nZhRtyb5jSNi2xBUU8g= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714070069; c=relaxed/simple; bh=YTn3RoRKadT6ALjCyncV5Dp2bADzbdJ/qnFsXrSYFlY=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=VHYfcsFhF3M3qMaa7/fjVt4zYR0o9W8HnSzAIqhq7jXNbbM24AMNjvSwGto+KTLT4+Mlor1Kizukpp0SWhbiGKZt/WbctSK+098GFtbAB1CfUauNeFZ+wGxmHNlhkj5VA/XdDfJkewZba27x6ZNbIRJrMfUfrvb+knnSeD9lYCM= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1714070058; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MpT8qzl6NdErDX8L36YWT6pL22R3o1FzvLD2QsgkTgE=; b=Gvl4Tel3y7gDBhc78dXXCeg+bvTzODkZ6yQg0We7wRCbIv0ctmaD1Uwcy2WMlqwsQHqjL1 eEQ3r/gMFIpnTzoZJ9+Titrqnqj8+TMenD4UJCI+mRdbHFvWceyTPjrnPGz550bNUMbZDm NTqNoTq5Ub2TO5xXI3yCNQ8JflsHZZ8= 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-547-RdGFDUpKPoyO8XD7mUlRVg-1; Thu, 25 Apr 2024 14:34:17 -0400 X-MC-Unique: RdGFDUpKPoyO8XD7mUlRVg-1 Received: by mail-qk1-f200.google.com with SMTP id af79cd13be357-7909bc5790aso153560185a.0 for ; Thu, 25 Apr 2024 11:34:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714070056; x=1714674856; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=MpT8qzl6NdErDX8L36YWT6pL22R3o1FzvLD2QsgkTgE=; b=nAlkC4sY3O023tYXlbIBRZ4ubEOEHPXNZbonuwhst+YGA5kzb86YoKbapUAdKUKO9u 99ZSi5i7RkYNtJPF1Sjs/gFpsTSfQ+G/MzbU//pBBU/7l+mRtS4QM1i51GcP/vwnGn// NamDUHA4CPXbcX53mRdNRXARPUalhJsvEw4lRbVWPWW305BgdMpWjK7/HpwqC9YpjqGb f6e5HRVcINoTjKxTDTLu5WuxhaqdYk72gy5X00yfBhueRl0MXtojdIT+QWuCSjSIcX3J /YIEQi5f1+6aB490D8816aClUuzLRjrHCS1zygTV6jO5Sq36BYy6tLyOjcxmRnqTCNJT pkig== X-Forwarded-Encrypted: i=1; AJvYcCVi/7i24t1zYPVP5p9DelD0iCPQe7NNKhxiPG3Urxi4w7lOqIqv2vXTe7NAyK72cup6ReJbiPW70huVyr8so6ISD1IT2a3EBAI66g== X-Gm-Message-State: AOJu0YyKOBWft/ZeTV2oVuFkQgJkf/rpm7nS1aXdG+HLMrWxvJxZs/HU Ef8L9IDf68yOKECLCZDehN3Ub5a7CdEU0LT/hXam6LMwzWSGXkkzdzEOD4ZTEwQmeWBWhZXJz6w /533F1tfgYHJBJ60g02s3ghBs58ZadNl/dRGavdmyTWSXVklYuRSF30b3FAQ= X-Received: by 2002:a05:620a:a4e:b0:78e:e0d8:4203 with SMTP id j14-20020a05620a0a4e00b0078ee0d84203mr448834qka.73.1714070056442; Thu, 25 Apr 2024 11:34:16 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFn1TPzkS3HtcpRsYmoKe/YhNDbsczYqZke1tqO1BbkSXVOQWg7LTVrBxJFp9b08e4hM4JQTg== X-Received: by 2002:a05:620a:a4e:b0:78e:e0d8:4203 with SMTP id j14-20020a05620a0a4e00b0078ee0d84203mr448814qka.73.1714070056045; Thu, 25 Apr 2024 11:34:16 -0700 (PDT) Received: from ?IPV6:2804:14d:8084:92c5::1001? ([2804:14d:8084:92c5::1001]) by smtp.gmail.com with ESMTPSA id 8-20020a05620a04c800b0079064d06b7esm5142877qks.22.2024.04.25.11.34.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 25 Apr 2024 11:34:15 -0700 (PDT) Message-ID: <28251b39-dda2-4da6-8bb6-2a5d81ef39df@redhat.com> Date: Thu, 25 Apr 2024 15:34:13 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] Allow calling of user-defined function call operators To: Hannes Domani , gdb-patches@sourceware.org References: <20240421124954.3285-1-ssbssa.ref@yahoo.de> <20240421124954.3285-1-ssbssa@yahoo.de> From: Guinevere Larsen In-Reply-To: <20240421124954.3285-1-ssbssa@yahoo.de> 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=-11.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,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 4/21/24 09:49, Hannes Domani wrote: > Currently it's not possible to call user-defined function call > operators, at least not without specifying operator() directly: > ``` > (gdb) l 1 > 1 struct S { > 2 int operator() (int x) { return x + 5; } > 3 }; > 4 > 5 int main () { > 6 S s; > 7 > 8 return s(23); > 9 } > (gdb) p s(10) > Invalid data type for function to be called. > (gdb) p s.operator()(10) > $1 = 15 > ``` > > This now looks if an user-defined call operator is available when > trying to 'call' a struct value, and calls it instead, making this > possible: > ``` > (gdb) p s(10) > $1 = 15 > ``` > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=12213 Thanks for this fix, this has been a long time coming. I have one, possibly big, question. > --- > gdb/eval.c | 46 +++++++++++++++++++++++++++----- > gdb/testsuite/gdb.cp/userdef.cc | 18 +++++++++++++ > gdb/testsuite/gdb.cp/userdef.exp | 4 +++ > 3 files changed, 62 insertions(+), 6 deletions(-) > > diff --git a/gdb/eval.c b/gdb/eval.c > index 6b752e70635..e737774ca28 100644 > --- a/gdb/eval.c > +++ b/gdb/eval.c > @@ -664,14 +664,34 @@ operation::evaluate_funcall (struct type *expect_type, > > value *callee = evaluate_with_coercion (exp, noside); > struct type *type = callee->type (); > - if (type->code () == TYPE_CODE_PTR) > - type = type->target_type (); > - for (int i = 0; i < args.size (); ++i) > + > + /* If the callee is a struct, there might be a user-defined function call > + operator that should be used instead. */ > + if (overload_resolution > + && exp->language_defn->la_language == language_cplus > + && check_typedef (type)->code () == TYPE_CODE_STRUCT) > { > - if (i < type->num_fields ()) > - vals[i] = args[i]->evaluate (type->field (i).type (), exp, noside); > - else > + for (int i = 0; i < args.size (); ++i) > vals[i] = args[i]->evaluate_with_coercion (exp, noside); > + > + vals.insert (vals.begin(), value_addr (callee)); > + int static_memfuncp; > + find_overload_match (vals, "operator()", METHOD, &vals[0], nullptr, > + &callee, nullptr, &static_memfuncp, 0, noside); > + if (static_memfuncp) > + vals.erase (vals.begin ()); > + } > + else > + { > + if (type->code () == TYPE_CODE_PTR) > + type = type->target_type (); > + for (int i = 0; i < args.size (); ++i) > + { > + if (i < type->num_fields ()) > + vals[i] = args[i]->evaluate (type->field (i).type (), exp, noside); > + else > + vals[i] = args[i]->evaluate_with_coercion (exp, noside); > + } This change had me confused for quite a bit. I couldn't figure out why the base operation::evaluate_funcall. The problem seems to be that if command used is something less straightforward, like "print (foo+bar) (args)", we will use the evaluate_funcall of the operation (in this case, add_operation) instead of var_value_operation's. Did I understand it correctly? Assuming I did, I don't think this code duplication is the best way to go. Especially seeing as there are some differences in those functions already, and if all that it takes to defeat out expression parser is use (*&foo) or (a + b) (), we probably already have latent bugs in this area. I don't know how to verify it, though, because I really don't understand the code differences. Ideally, I think the best way would be to put all the code in var_value_operation::evaluate_funcall, but to do this, you'd need to be able to find a way to call the resulting var_value_operation's function from all relevant operations, which is probably a lot. Another option is to just park all the logic in operation::evaluate_funcall, so we're always using the correct function. This comes with the cost of being very confusing in a couple of months to a year. Does this make sense? -- Cheers, Guinevere Larsen She/Her/Hers > } > > return evaluate_subexp_do_call (exp, noside, callee, vals, > @@ -702,6 +722,20 @@ var_value_operation::evaluate_funcall (struct type *expect_type, > value *callee = evaluate_var_value (noside, std::get<0> (m_storage).block, > symp); > > + /* If the callee is a struct, there might be a user-defined function call > + operator that should be used instead. */ > + if (overload_resolution > + && exp->language_defn->la_language == language_cplus > + && check_typedef (callee->type ())->code () == TYPE_CODE_STRUCT) > + { > + argvec.insert (argvec.begin(), value_addr (callee)); > + int static_memfuncp; > + find_overload_match (argvec, "operator()", METHOD, &argvec[0], nullptr, > + &callee, nullptr, &static_memfuncp, 0, noside); > + if (static_memfuncp) > + argvec.erase (argvec.begin ()); > + } > + > return evaluate_subexp_do_call (exp, noside, callee, argvec, > nullptr, expect_type); > } > diff --git a/gdb/testsuite/gdb.cp/userdef.cc b/gdb/testsuite/gdb.cp/userdef.cc > index 774191726f3..48507551079 100644 > --- a/gdb/testsuite/gdb.cp/userdef.cc > +++ b/gdb/testsuite/gdb.cp/userdef.cc > @@ -68,6 +68,9 @@ A1 operator++(int); > A1 operator--(); > A1 operator--(int); > > +int operator()(); > +int operator()(int); > + > }; > > > @@ -293,6 +296,16 @@ ostream& operator<<(ostream& outs, A1 one) > return (outs << endl << "x = " << one.x << endl << "y = " << one.y << endl << "-------" << endl); > } > > +int A1::operator()() > +{ > + return x + y; > +} > + > +int A1::operator()(int value) > +{ > + return value * (x + y); > +} > + > class A2 { > public: > A2 operator+(); > @@ -404,6 +417,11 @@ int main (void) > ++three; > cout << "preinc " << three; > > + val = two(); > + cout << "funcall " << val << endl; > + val = two(10); > + cout << "funcall 2 " << val << endl; > + > (*c).z = 1; > > return 0; > diff --git a/gdb/testsuite/gdb.cp/userdef.exp b/gdb/testsuite/gdb.cp/userdef.exp > index e96636bef0c..667bded6b83 100644 > --- a/gdb/testsuite/gdb.cp/userdef.exp > +++ b/gdb/testsuite/gdb.cp/userdef.exp > @@ -119,6 +119,10 @@ gdb_test "print one += 7" "\\\$\[0-9\]* = {x = 9, y = 10}" > > gdb_test "print two = one" "\\\$\[0-9\]* = {x = 9, y = 10}" > > +gdb_test "print two()" " = 19" > +gdb_test "print two(10)" " = 190" > +gdb_test "print (*&two)(2)" " = 38" > + > # Check that GDB tolerates whitespace in operator names. > gdb_test "break A2::operator+" ".*Breakpoint $decimal at.*" > gdb_test "break A2::operator +" ".*Breakpoint $decimal at.*"