From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id T8BvD7y3GWp+OScAWB0awg (envelope-from ) for ; Fri, 29 May 2026 11:58:52 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=jCDExPS4; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 30AF91E062; Fri, 29 May 2026 11:58:52 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id A084F1E062 for ; Fri, 29 May 2026 11:58:51 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 294BA4BA23FC for ; Fri, 29 May 2026 15:58:51 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 294BA4BA23FC Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=jCDExPS4 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 686F44BA23DD for ; Fri, 29 May 2026 15:58:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 686F44BA23DD Authentication-Results: sourceware.org; dmarc=pass (p=quarantine 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 686F44BA23DD Authentication-Results: sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1780070305; cv=none; b=o1eJAPHrIP2EPSECMQxm42qJ7MOCsgwRWrsXN/EgdYt7bigCPhmpdehV9CRO0Xi/5f6xmjai2ZJ4ABqwDDKxgy6Xw3fTAj+sLud8buriHY4MqQZS4MGNByhIZcCyrhjcsHoZLt/MGwAu3v1vU2rMytl2fuPzrgHKwMEk21qhL0w= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1780070305; c=relaxed/simple; bh=rROgBSp4XTjaqbEm8oUlxPrKByjAsuZ4pY3cQS4bHY8=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=gvnkLI4Dcm9k7mFSRTXfJvjypldGdNmx2upTshMFHf+hCnYZduDMMOTbbiYgo7BKoOWHZo+xFX5qD+CMk5zezC7QoUNCRxET/hen8LVN8ZDPpDiE6NvpfA2BsMWdreNgi7mAP8tgae+u7Ai3MdyDK30PrxZBXrltlEBVorTbN5k= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=jCDExPS4 DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 686F44BA23DD DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1780070305; 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: in-reply-to:in-reply-to:references:references; bh=RmhTVy+d0p1lIRo9mfoXfFpaodtM2hCz5i8gIblhHzQ=; b=jCDExPS4o/UxMQsn3TkQD9GVZ0TtrdwuZ008nHk/RYQSdn81Obx2TLKSU+dG6HTIq52CjY zGToLRp3jdysRZU8yv6S9cXxLMIPsR8faFwtkgmy9HlH+G7wBxz/xK+mCUDfDbzJVYm7Ji bgiG2/IBnID/ZQaE8CJxW68nkkE8hsc= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-124-ezNiIpxkPzC5HFbe8y0Lsg-1; Fri, 29 May 2026 11:58:22 -0400 X-MC-Unique: ezNiIpxkPzC5HFbe8y0Lsg-1 X-Mimecast-MFC-AGG-ID: ezNiIpxkPzC5HFbe8y0Lsg_1780070301 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-45ef5e38a18so209784f8f.1 for ; Fri, 29 May 2026 08:58:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1780070301; x=1780675101; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=RmhTVy+d0p1lIRo9mfoXfFpaodtM2hCz5i8gIblhHzQ=; b=C1zR6apDL08I1NEqo+5h2hBbihwjzAFP5D1PIVoc4W0pG1g5MtoXkmIu2ydG6/K5Rk 4yz4/SUeZTGS1GEsN20OjCo+SAWNJRXFYUbZHP+QiPVtkEy8M65sTOlO3ya1zbXNGxMN sigox75NiTWaWiU48bcumP4Zw7EJDeY6RFSgevYy/NP+zOFtZqj66SHiQxuHYi4FOhX+ fnuIewhzFlymafefRKyFyoS7xLr7c4IsLDPgl0KdugCVMC8tS9GmqMGo8M7W0hPbY1CN AJxwiHghjQSeWqZSrHiNRXM9SK60rkxXGxElaWYJGd+Gq9mbm5u192xRyRFhKtcs15bL xjLg== X-Forwarded-Encrypted: i=1; AFNElJ9nusMQpbKFWarJZUvTyugtV2E+GdVO10cLOu2dlZImOPTKL5E31kSUwONYVrRtzQlkuez7sM9vBjsYkQ==@sourceware.org X-Gm-Message-State: AOJu0YwbkeZOBUDis8K6o9AZRLLN1IA1zjGM1yq93HV0Z9G2UaTUD4fg jxcpuu2yQ6kKRF6FQ+IWkpLAsKCyHv1OzDdwx3vUxa5SfwzfDuzyx2uRB690efyTTlfznTQ3ydd nBpGVy5BfwVVvrptLvG8r1kkl9VH/wQSqM4aQPJlo4OjNUBeIsTnejWn3JAGrGxg= X-Gm-Gg: Acq92OHW1j+P3ibS5cqb0Ns2S2tKiPfsgmQuxnD57r5SpgpRrbv5O8wIO9P6vi5ynYi 9wZFHbPGBbPXBEihbCrrIl+nC9Jlq1wJwR6a54F2MbC/8Ud+PRzoYg3YXvT6PbmWcIzcvWHv3yo 6nhhJIU51AINCD+lI+oAFtgt6VSXh+hpLRYHoLNpfHRohEpMPyFk2livNsBm86SCnT+0hhlfFl1 OFXrQjOa/EpcQDtdxXSv7ClneYzJpwKVzEmIBc5DptnEIsnioh4UBS2zQKGBbOtyfBUrQoq9OpX lqDvFpIBv8/CWxvtRxgDVnWOLEVuKSCVIZtdPKRSKCR1hs6MxtMP/XRxKdF87aeCXHfD9Z2GzpQ EmFoSiXovWiArvTei2NfK3X8NGw== X-Received: by 2002:a05:600c:c3cc:20b0:490:3ff5:737f with SMTP id 5b1f17b1804b1-490a292fd05mr3187005e9.18.1780070300796; Fri, 29 May 2026 08:58:20 -0700 (PDT) X-Received: by 2002:a05:600c:c3cc:20b0:490:3ff5:737f with SMTP id 5b1f17b1804b1-490a292fd05mr3186645e9.18.1780070300305; Fri, 29 May 2026 08:58:20 -0700 (PDT) Received: from localhost ([213.31.44.43]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4909c0a93b5sm16935495e9.8.2026.05.29.08.58.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 May 2026 08:58:19 -0700 (PDT) From: Andrew Burgess To: Tom Tromey Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [PATCH v2 3/4] Add wrappers for Python implementation functions and methods In-Reply-To: <871pf4jshc.fsf@tromey.com> References: <20260515-python-safety-initial-v2-0-6129cadf258a@tromey.com> <20260515-python-safety-initial-v2-3-6129cadf258a@tromey.com> <874ik5ca1g.fsf@redhat.com> <871pf4jshc.fsf@tromey.com> Date: Fri, 29 May 2026 16:58:18 +0100 Message-ID: <87o6hyxl5x.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: l6EwO3HA8SbNDmLDXdZCqGonUyIn_YfKmrn5SW5lKE8_1780070301 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Tom Tromey writes: >>> Note this patch is not 100% complete. There should be one more >>> wrapper for case where a method takes a single argument (though we >>> probably cannot use METH_O unfortunately). There may be some other >>> holes as well. > > Andrew> Maybe reword this so it doesn't imply that THIS patch is not complete, > Andrew> but rather the implementation as a whole is not complete and will > Andrew> require future follow on patches. What you've got is good enough to > Andrew> start using it. > > I updated the text a bit. > >>> +template> > > Andrew> I think the SFINAE part here is wrong, like in the previous commit. I > Andrew> think gdb::Requires> might be what you mean. > > Fixed. > > Andrew> None of the to_python functions check their return values for error, for > Andrew> example PyUnicode_Decode can fail and return NULL, but you don't check > Andrew> for this. > > Andrew> But this is OK. to_python is only used at the point where we transition > Andrew> back from GDB's C++ code to the Python internals, so if PyUnicode_Decode > Andrew> (for example) returns NULL and sets an exception, this will be caught by > Andrew> Python. > > Andrew> I have two pieces of feedback on this: > > Andrew> 1. I think this should be explicitly called out in the comment above > Andrew> the to_python functions, rather than making everyone figure out that > Andrew> this is not a mistake. > > I updated the comment that precedes the to_python functions as a whole. > >>> + /* Note that this cannot fail. */ > > Andrew> Is (IMHO) confusing. It implies the lack of error checking here is > Andrew> because PyBool_FromLong cannot fail, which is why, when I look at > Andrew> later to_python functions which include calls that *can* fail, I > Andrew> asked myself, where's the error checking. > > Andrew> I think this comment should just go. > > Deleted. > >>> + gdbpy_borrowed_ref or gdbpy_opt_borrowed_ref), and then then calls > > Andrew> typo: ".... then THEN calls ..." > > Fixed. > >>> +/* An instantiation of this function is used when calling a gdb method >>> + from Python. It accepts some number of arguments (normally >>> + gdbpy_borrowed_ref or gdbpy_opt_borrowed_ref), and then then calls >>> + the underlying function F. Any exceptions are caught and >>> + converted, and the return value of F is converted to a Python >>> + object as appropriate. */ > > Andrew> This comment needs updating. It also include the 'then then' typo from > Andrew> the wrapped_function comment, but also references function F, when it > Andrew> should be taking about method CLASS::METH or maybe just METH? Anyway, > Andrew> certainly not F. > > Fixed. > >>> +/* A function that wraps a "repr" or "str" method. */ >>> +template > > Andrew> In wrap_setter below you are explicit about the signature of M. I much > Andrew> prefer the explicit form, but here in wrap_repr and in wrap_getter you > Andrew> use 'auto'. Could we switch to the explicit form in these two too? > > There are two issues with changing. > > One is that while a setter should probably just return void, a getter > could return anything. And, requiring a specific return type for tp_str > or tp_repr seems a bit heavy, like maybe it would be convenient to > return std::string in some spots or gdbpy_ref<> in others. > > The other issue is that 'auto' means it automatically accepts const- or > non-const-methods. This can be handled by overloads of course. > > Anyway I left this as is but we can discuss further if you want. No, that's fine, thanks for explaining the benefit here. I'm perfectly happy with 'auto' when it's serving a good purpose like this. Thanks, Andrew