From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id 8E4043858D1E for ; Fri, 23 Sep 2022 23:00:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8E4043858D1E Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id CADCE1F890; Fri, 23 Sep 2022 23:00:34 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id B23F713AA5; Fri, 23 Sep 2022 23:00:34 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id atjoKZI6LmNRTgAAMHmgww (envelope-from ); Fri, 23 Sep 2022 23:00:34 +0000 Content-Type: multipart/mixed; boundary="------------pzeaDJ53g23iOYc30YNjtHVP" Message-ID: <4baea1af-e671-239c-e3c3-d91e222d9929@suse.de> Date: Sat, 24 Sep 2022 01:00:34 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Subject: Re: [PATCH] Fix printing destructors with void in parameters for clang programs Content-Language: en-US To: Bruno Larsen , gdb-patches@sourceware.org References: <20220923155013.124413-1-blarsen@redhat.com> From: Tom de Vries In-Reply-To: <20220923155013.124413-1-blarsen@redhat.com> X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 Sep 2022 23:00:38 -0000 This is a multi-part message in MIME format. --------------pzeaDJ53g23iOYc30YNjtHVP Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 9/23/22 17:50, Bruno Larsen via Gdb-patches wrote: > When GDB prints the methods of a C++ class, it will always skip the first > parameter, assuming it to be a 'this' pointer. However, when deciding if > it should print "void" for the parameters, it disregards the fact that > the first parameter was skipped, so if the method only had that > parameter, for instance how clang compiles destructors, we'd see > ~foo(void) instead of ~foo(). > Hi, I wrote the following test-case to investigate the problem: ... $ cat test.cc class A { public: int a; A() { this->a = 3; } ~A() { a = 0; } }; int main (void) { A a; return a.a; } $ g++ test.cc -g $ ./a.out; echo $? 3 ... With g++ (7.5.0) I see: ... $ gdb -q -batch a.out -ex "ptype A" type = class A { public: int a; A(void); ~A(); } ... and with clang++ (13.0.1): ... $ gdb -q -batch a.out -ex "ptype A" type = class A { public: int a; A(void); ~A(void); } ... So, ok, I see how the patch makes the output for clang++ and g++ the same, getting us "A()" and "~A()". I don't have a preference of say ~A::A() vs A::~A(void), but I suppose the former is the newer, more c++-like style, and the latter leaves less room for confusion. Do you have a preference, or were you merely trying to make the output for the destructor the same for both cases? [ But now look at the expression parsing side. This does not get us anything, with both g++ and clang++: ... $ gdb -q -batch a.out -ex "p A::A()" -ex "p A::~A()" Cannot resolve method A::A to any overloaded instance Cannot resolve method A::~A to any overloaded instance ... but again with both g++ and clang++ (and with and without your patch): ... $ gdb -q -batch a.out -ex "p A::A(void)" -ex "p A::~A(void)" $1 = {void (A * const)} 0x4004f4 $2 = {void (A * const)} 0x40050a ... So, I wonder if the expression parsing could be improved to handle the "A (void) == A ()" equivalence. But that aside. ] Anyway, an interesting detail is that the g++-generated destructor was already printed without void. Investigation shows that it is due to having two artificial parameters. I think that shows that we're making decisions about how to handle similar things in different places in the code, which means it needs a bit of restructuring. > - else if (language == language_cplus) > + else if (language == language_cplus && nargs == 0) > gdb_printf (stream, "void"); I remain confused about why we're doing things differently based on language. Anyway, the nargs == 0 implies staticp, so I wonder if we should not handle a static function with implicit first argument the same (not sure if that can happen, but even so, maybe we don't want to use that assumption into the code), in which case the nargs == 0 doesn't address that scenario. I tried rewriting the code in a more regular for loop structure, in attached patch, not fully tested yet but passes at least gdb.cp/*.exp for me. WDYT ? Thanks, - Tom --------------pzeaDJ53g23iOYc30YNjtHVP Content-Type: text/x-patch; charset=UTF-8; name="tmp.patch" Content-Disposition: attachment; filename="tmp.patch" Content-Transfer-Encoding: base64 ZGlmZiAtLWdpdCBhL2dkYi9jLXR5cGVwcmludC5jIGIvZ2RiL2MtdHlwZXByaW50LmMKaW5k ZXggM2E2MTFjZGFjNWQuLmUxN2E0Y2NmYTFlIDEwMDY0NAotLS0gYS9nZGIvYy10eXBlcHJp bnQuYworKysgYi9nZGIvYy10eXBlcHJpbnQuYwpAQCAtMjczLDM1ICsyNzMsNDYgQEAgY3Bf dHlwZV9wcmludF9tZXRob2RfYXJncyAoc3RydWN0IHR5cGUgKm10eXBlLCBjb25zdCBjaGFy ICpwcmVmaXgsCiAJCSAgbGFuZ3VhZ2VfY3BsdXMsIERNR0xfQU5TSSk7CiAgIGdkYl9wdXRz ICgiKCIsIHN0cmVhbSk7CiAKLSAgLyogU2tpcCB0aGUgY2xhc3MgdmFyaWFibGUuICBXZSBr ZWVwIHRoaXMgaGVyZSB0byBhY2NvbW1vZGF0ZSBvbGRlcgotICAgICBjb21waWxlcnMgYW5k IGRlYnVnIGZvcm1hdHMgd2hpY2ggbWF5IG5vdCBzdXBwb3J0IGFydGlmaWNpYWwKLSAgICAg cGFyYW1ldGVycy4gICovCi0gIGkgPSBzdGF0aWNwID8gMCA6IDE7Ci0gIGlmIChuYXJncyA+ IGkpCisgIGludCBwcmludGVkX2FyZ3MgPSAwOworICBmb3IgKGkgPSAwOyBpIDwgbmFyZ3M7 ICsraSkKICAgICB7Ci0gICAgICB3aGlsZSAoaSA8IG5hcmdzKQorICAgICAgaWYgKGkgPT0g MCAmJiAhc3RhdGljcCkKIAl7Ci0JICBzdHJ1Y3QgZmllbGQgYXJnID0gYXJnc1tpKytdOwot Ci0JICAvKiBTa2lwIGFueSBhcnRpZmljaWFsIGFyZ3VtZW50cy4gICovCi0JICBpZiAoRklF TERfQVJUSUZJQ0lBTCAoYXJnKSkKLQkgICAgY29udGludWU7CisJICAvKiBTa2lwIHRoZSBj bGFzcyB2YXJpYWJsZS4gIFdlIGtlZXAgdGhpcyBoZXJlIHRvIGFjY29tbW9kYXRlIG9sZGVy CisJICAgICBjb21waWxlcnMgYW5kIGRlYnVnIGZvcm1hdHMgd2hpY2ggbWF5IG5vdCBzdXBw b3J0IGFydGlmaWNpYWwKKwkgICAgIHBhcmFtZXRlcnMuICAqLworCSAgY29udGludWU7CisJ fQogCi0JICBjX3ByaW50X3R5cGUgKGFyZy50eXBlICgpLCAiIiwgc3RyZWFtLCAwLCAwLCBs YW5ndWFnZSwgZmxhZ3MpOworICAgICAgc3RydWN0IGZpZWxkIGFyZyA9IGFyZ3NbaV07Cisg ICAgICAvKiBTa2lwIGFueSBhcnRpZmljaWFsIGFyZ3VtZW50cy4gICovCisgICAgICBpZiAo RklFTERfQVJUSUZJQ0lBTCAoYXJnKSkKKwljb250aW51ZTsKIAotCSAgaWYgKGkgPT0gbmFy Z3MgJiYgdmFyYXJncykKLQkgICAgZ2RiX3ByaW50ZiAoc3RyZWFtLCAiLCAuLi4iKTsKLQkg IGVsc2UgaWYgKGkgPCBuYXJncykKLQkgICAgewotCSAgICAgIGdkYl9wcmludGYgKHN0cmVh bSwgIiwgIik7Ci0JICAgICAgc3RyZWFtLT53cmFwX2hlcmUgKDgpOwotCSAgICB9CisgICAg ICBpZiAocHJpbnRlZF9hcmdzID4gMCkKKwl7CisJICBnZGJfcHJpbnRmIChzdHJlYW0sICIs ICIpOworCSAgc3RyZWFtLT53cmFwX2hlcmUgKDgpOwogCX0KKworICAgICAgY19wcmludF90 eXBlIChhcmcudHlwZSAoKSwgIiIsIHN0cmVhbSwgMCwgMCwgbGFuZ3VhZ2UsIGZsYWdzKTsK KyAgICAgIHByaW50ZWRfYXJncysrOworICAgIH0KKworICBpZiAodmFyYXJncykKKyAgICB7 CisgICAgICBpZiAocHJpbnRlZF9hcmdzID09IDApCisJZ2RiX3ByaW50ZiAoc3RyZWFtLCAi Li4uIik7CisgICAgICBlbHNlCisJLyogTm90ZTogbm8gd3JhcHBpbmcgZG9uZSBoZXJlLiAg SXMgdGhhdCBpbnRlbmRlZD8gICovCisJZ2RiX3ByaW50ZiAoc3RyZWFtLCAiLCAuLi4iKTsK KyAgICB9CisgIGVsc2UgaWYgKHByaW50ZWRfYXJncyA9PSAwKQorICAgIHsKKyAgICAgIC8q IFdoeSBkbyB3ZSBkbyB0aGlzIGRpZmZlcmVudGx5IGZvciBub24tYysrPyAgKi8KKyAgICAg IGlmIChsYW5ndWFnZSA9PSBsYW5ndWFnZV9jcGx1cyAmJiBzdGF0aWNwKQorCWdkYl9wcmlu dGYgKHN0cmVhbSwgInZvaWQiKTsKICAgICB9Ci0gIGVsc2UgaWYgKHZhcmFyZ3MpCi0gICAg Z2RiX3ByaW50ZiAoc3RyZWFtLCAiLi4uIik7Ci0gIGVsc2UgaWYgKGxhbmd1YWdlID09IGxh bmd1YWdlX2NwbHVzKQotICAgIGdkYl9wcmludGYgKHN0cmVhbSwgInZvaWQiKTsKIAogICBn ZGJfcHJpbnRmIChzdHJlYW0sICIpIik7CiAKZGlmZiAtLWdpdCBhL2dkYi90ZXN0c3VpdGUv Z2RiLmNwL2NsYXNzZXMuZXhwIGIvZ2RiL3Rlc3RzdWl0ZS9nZGIuY3AvY2xhc3Nlcy5leHAK aW5kZXggN2I4YjMxNWFjMWYuLjdjMjE0YTU3MTAwIDEwMDY0NAotLS0gYS9nZGIvdGVzdHN1 aXRlL2dkYi5jcC9jbGFzc2VzLmV4cAorKysgYi9nZGIvdGVzdHN1aXRlL2dkYi5jcC9jbGFz c2VzLmV4cApAQCAtMzI4LDcgKzMyOCw3IEBAIHByb2MgdGVzdF9wdHlwZV9jbGFzc19vYmpl Y3RzIHt9IHsKIAkJICAiY2xhc3Nfd2l0aF90eXBlZGVmczo6cHJvdGVjdGVkX2ludCBwcm90 ZWN0ZWRfaW50XzsiIH0KIAkgICAgeyBmaWVsZCBwcm90ZWN0ZWQgXAogCQkgICJjbGFzc193 aXRoX3R5cGVkZWZzOjpwcml2YXRlX2ludCBwcml2YXRlX2ludF87IiB9Ci0JICAgIHsgbWV0 aG9kIHB1YmxpYyAiY2xhc3Nfd2l0aF90eXBlZGVmcyh2b2lkKTsiIH0KKwkgICAgeyBtZXRo b2QgcHVibGljICJjbGFzc193aXRoX3R5cGVkZWZzKCk7IiB9CiAJICAgIHsgbWV0aG9kIHB1 YmxpYyAiY2xhc3Nfd2l0aF90eXBlZGVmczo6cHVibGljX2ludCBhZGRfcHVibGljKGNsYXNz X3dpdGhfdHlwZWRlZnM6OnB1YmxpY19pbnQpOyIgfQogCSAgICB7IG1ldGhvZCBwdWJsaWMg XAogCQkgICJjbGFzc193aXRoX3R5cGVkZWZzOjpwdWJsaWNfaW50IGFkZF9hbGwoaW50KTsi IH0KZGlmZiAtLWdpdCBhL2dkYi90ZXN0c3VpdGUvZ2RiLmNwL3J2YWx1ZS1yZWYtb3Zlcmxv YWQuZXhwIGIvZ2RiL3Rlc3RzdWl0ZS9nZGIuY3AvcnZhbHVlLXJlZi1vdmVybG9hZC5leHAK aW5kZXggMzYzNDI2NzNhYWQuLmIwYzU1NWE4ZmE5IDEwMDY0NAotLS0gYS9nZGIvdGVzdHN1 aXRlL2dkYi5jcC9ydmFsdWUtcmVmLW92ZXJsb2FkLmV4cAorKysgYi9nZGIvdGVzdHN1aXRl L2dkYi5jcC9ydmFsdWUtcmVmLW92ZXJsb2FkLmV4cApAQCAtNDMsNyArNDMsNyBAQCBnZGJf dGVzdCAidXAiICIuKm1haW4uKiIgInVwIGZyb20gbWFya2VyMSIKICMgUHJpbnQgdGhlIG1v bnN0ZXIgY2xhc3MgdHlwZS4KIGNwX3Rlc3RfcHR5cGVfY2xhc3MgImZvb19ycl9pbnN0YW5j ZTEiICIiICJjbGFzcyIgImZvbyIgXAogICAgIHsKLQl7IG1ldGhvZCBwdWJsaWMgImZvbyh2 b2lkKTsiIH0KKwl7IG1ldGhvZCBwdWJsaWMgImZvbygpOyIgfQogCXsgbWV0aG9kIHB1Ymxp YyAiZm9vKGZvb19sdmFsX3JlZik7IiB9CiAJeyBtZXRob2QgcHVibGljICJmb28oZm9vX3J2 YWxfcmVmKTsiIH0KIAl7IG1ldGhvZCBwdWJsaWMgIn5mb28oKTsiIH0K --------------pzeaDJ53g23iOYc30YNjtHVP--