From patchwork Mon Nov 20 11:40:05 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 80351 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D1EDA3858C53 for ; Mon, 20 Nov 2023 11:40:25 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 8986C3858D33 for ; Mon, 20 Nov 2023 11:40:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8986C3858D33 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 8986C3858D33 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700480413; cv=none; b=CYfZ7uHYCfwwNLQE1Ck4wfj4kc50ZzpgmxKFnNbxFKOfMiEkoyj6OHpqdTLRPaNHAqj5nQ7UVX/HhIt+2ZqXR4COtcMaau/1JsRZwkx1ue4/jiJ8exoqYEILd6rp5HqppLgRIuvBYW+kzMCQplPKVm3NGhNPQIqYKL3fvf2xFt0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700480413; c=relaxed/simple; bh=/k8j8imiHNIw2BfMxRXE2W5foXNLOZFwGnRZDQ5GfUo=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=SjzUoffzuQIWb8ScwFlseI+kHIegiHk1NdzevZ84Mp+DxdmDZqT9I44Fecu7md9XQjR6UogZHKBZ8DZ0kLLKrACvryz5MPw8rDwmtnZ+frUS6sBiNFM36Ncuu9QHrDop9NQmR7txEssgvu+fOTe7gHsC43/XD4vIT1rGB1idU64= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1700480412; 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=uswIyBw7N2ZxPaMwJtXS8174Yv08XVLQI1xXvMT3I6A=; b=aSzkqEG6RS22jv50rje+tcPQeDbmzxK+6uikZAGst5A5c2kuPRiTAYLVwGXR6AI3XHPa3I 2Wi9QOQ036cBnMN2WCMO9mXy3ub86rfkdo/O6HVAGZZVBxAeiH8JxhVjDRLheOvE0nFLna Z2zDCD+SSq9pid9U2wjgwPbaQncVZNY= Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-588-F4A_L8cHMByRs7u26h9xVg-1; Mon, 20 Nov 2023 06:40:11 -0500 X-MC-Unique: F4A_L8cHMByRs7u26h9xVg-1 Received: by mail-qv1-f69.google.com with SMTP id 6a1803df08f44-66d026cae6eso28620946d6.3 for ; Mon, 20 Nov 2023 03:40:11 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700480410; x=1701085210; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=uswIyBw7N2ZxPaMwJtXS8174Yv08XVLQI1xXvMT3I6A=; b=L0hxZY49DqfkUvQuP5KjUieFXiENmIs5o3lpQmzYNPbaHQBXRKZLHwGChLUIapjRbN G3qS0JmZZT9+QMb03+BDdyEPCa9x5zoF8Hcnu/1jjIbzh40G8Khb+FIT75H4C2EJzXXu AbAFd3rYq8Ui97ffj0GSD/1BNFZ4NvQra0kgdI15cY/tUGpko6xNR8FaVL49wXBcFaoN X7sb20IXrf5U5Vgrus7pM7amfuvmtTgtQ96pm6/nkdH0vLh9Pdjy6qpt3dKcTCunTbkt 1K4eBqUgBkR8pr/xtniRm95EkhnWq3uIeyedeFrn+aL9z+kMWZiU+N4KOLHWXa8zT0/F 7E1Q== X-Gm-Message-State: AOJu0YxUTn6Nrm21X+aadWOZeP+hANUWa65UGf3RGb7f0YZCMxqMBBkw 3FNQsA8NK9qUThf0AN+r8v/QR9nkt8Ah++fcUmXIloQNQlo3uPn39P4kvhGkNIESgwzKaaKCDgQ ucaPqOnqplpv4cLtue8/dn+c+Ln+kgvzfRuimjURncb1FPDqCvL6lmN9EGJiz5DTbNZz7uwWVLC X5Wc56ZA== X-Received: by 2002:a0c:e210:0:b0:647:4060:1074 with SMTP id q16-20020a0ce210000000b0064740601074mr8294360qvl.22.1700480410566; Mon, 20 Nov 2023 03:40:10 -0800 (PST) X-Google-Smtp-Source: AGHT+IHc+WXe7f+KY/pZJXCc84sh8T2NgCM7JeICHr7rHC4LWdQ24qmDNcMXynJqK0tNHWwXfNTaqg== X-Received: by 2002:a0c:e210:0:b0:647:4060:1074 with SMTP id q16-20020a0ce210000000b0064740601074mr8294341qvl.22.1700480410193; Mon, 20 Nov 2023 03:40:10 -0800 (PST) Received: from localhost (105.226.159.143.dyn.plus.net. [143.159.226.105]) by smtp.gmail.com with ESMTPSA id ld31-20020a056214419f00b00674bdf8bfa9sm2844759qvb.29.2023.11.20.03.40.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Nov 2023 03:40:09 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess , Paul Koning , Tom Tromey Subject: [PATCHv2] gdb/python: handle completion returning a non-sequence Date: Mon, 20 Nov 2023 11:40:05 +0000 Message-Id: <521486ffacc94e0139bf63886c873fc7c142cbc2.1700480026.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: <=87fa2a8161452cf2eb17ca7ba561831eebe4f254.1700146245.git.aburgess@redhat.com> References: <=87fa2a8161452cf2eb17ca7ba561831eebe4f254.1700146245.git.aburgess@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.8 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_H3, 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 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+patchwork=sourceware.org@sourceware.org Thanks for the feedback. In V2: - Instead of calling 'PyErr_Clear ();' to ignore the error, I'm now calling 'PySequence_Check (resultobj.get ())' to specifically ignore non-sequence like things. I think this is a better choice, and should have done this in V1, - I'm no longer ignoring the error. The Python completion API already ignores many errors/exceptions, so addressing Tom's feedback would involve a bigger change, which can come later, - As I no longer want to take a position on ignoring vs announcing errors, I am no longer adding a test for what happens if the completion code raises an exception, as a consequence, I'm not going to change the docs as Paul suggested. If I change the docs to explicitly state that exceptions are ignored and mean no completions, then this is counter to Tom's feedback that exceptions should be displayed. As both of these issues are not the main focus of this patch, I'm just going to ignore them for now, - Commit message is updated to reflect narrower focus of the patch. Thanks, Andrew --- GDB's Python API documentation for gdb.Command.complete() says: The 'complete' method can return several values: * If the return value is a sequence, the contents of the sequence are used as the completions. It is up to 'complete' to ensure that the contents actually do complete the word. A zero-length sequence is allowed, it means that there were no completions available. Only string elements of the sequence are used; other elements in the sequence are ignored. * If the return value is one of the 'COMPLETE_' constants defined below, then the corresponding GDB-internal completion function is invoked, and its result is used. * All other results are treated as though there were no available completions. So, returning a non-sequence, and non-integer from a complete method should be fine; it should just be treated as though there are no completions. However, if I write a complete method that returns None, I see this behaviour: (gdb) complete completefilenone x Python Exception : 'NoneType' object is not iterable warning: internal error: Unhandled Python exception (gdb) Which is caused because we currently assume that anything that is not an integer must be iterable, and we call PyObject_GetIter on it. When this call fails a Python exception is set, but instead of clearing (and therefore ignoring) this exception as we do everywhere else in the Python completion code, we instead just return with the exception set. In this commit I add a PySequence_Check call. If this call returns false (and we've already checked the integer case) then we can assume there are no completion results. I've added a test which checks returning a non-sequence. --- gdb/python/py-cmd.c | 2 +- gdb/testsuite/gdb.python/py-completion.exp | 5 +++++ gdb/testsuite/gdb.python/py-completion.py | 12 ++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) base-commit: 70fd94b244573225cb04ae41101d495def78b9e6 diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c index 20a384d6907..d3845fc7509 100644 --- a/gdb/python/py-cmd.c +++ b/gdb/python/py-cmd.c @@ -290,7 +290,7 @@ cmdpy_completer (struct cmd_list_element *command, else if (value >= 0 && value < (long) N_COMPLETERS) completers[value].completer (command, tracker, text, word); } - else + else if (PySequence_Check (resultobj.get ())) { gdbpy_ref<> iter (PyObject_GetIter (resultobj.get ())); diff --git a/gdb/testsuite/gdb.python/py-completion.exp b/gdb/testsuite/gdb.python/py-completion.exp index 23f981e944a..89843c96f1f 100644 --- a/gdb/testsuite/gdb.python/py-completion.exp +++ b/gdb/testsuite/gdb.python/py-completion.exp @@ -46,6 +46,11 @@ if { [readline_is_used] && ![is_remote host] } { # Just discarding whatever we typed. gdb_test " " ".*" "discard #[incr discard]" + # This should offer no suggestions - the complete() methods + # returns something that is neither an integer, or a sequence. + gdb_test_no_output "complete completefilenone ${testdir_complete}" \ + "no suggestions given" + # This is the problematic one. send_gdb "completefilemethod ${testdir_complete}\t" gdb_test_multiple "" "completefilemethod completion" { diff --git a/gdb/testsuite/gdb.python/py-completion.py b/gdb/testsuite/gdb.python/py-completion.py index abec06921c0..61b6beffa25 100644 --- a/gdb/testsuite/gdb.python/py-completion.py +++ b/gdb/testsuite/gdb.python/py-completion.py @@ -28,6 +28,17 @@ class CompleteFileInit(gdb.Command): raise gdb.GdbError("not implemented") +class CompleteFileNone(gdb.Command): + def __init__(self): + gdb.Command.__init__(self, "completefilenone", gdb.COMMAND_USER) + + def invoke(self, argument, from_tty): + raise gdb.GdbError("not implemented") + + def complete(self, text, word): + return None + + class CompleteFileMethod(gdb.Command): def __init__(self): gdb.Command.__init__(self, "completefilemethod", gdb.COMMAND_USER) @@ -203,6 +214,7 @@ class CompleteLimit7(gdb.Command): CompleteFileInit() +CompleteFileNone() CompleteFileMethod() CompleteFileCommandCond() CompleteLimit1()