From patchwork Wed Apr 8 19:58:55 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergio Durigan Junior X-Patchwork-Id: 6104 Received: (qmail 65188 invoked by alias); 8 Apr 2015 19:59:00 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 65177 invoked by uid 89); 8 Apr 2015 19:59:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 08 Apr 2015 19:58:58 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 26CD18EA3D for ; Wed, 8 Apr 2015 19:58:57 +0000 (UTC) Received: from localhost (unused-10-15-17-126.yyz.redhat.com [10.15.17.126]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t38JwucJ014675 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA256 bits=256 verify=NO); Wed, 8 Apr 2015 15:58:56 -0400 From: Sergio Durigan Junior To: Keith Seitz Cc: GDB Patches Subject: Re: [PATCH] Fix Python completion when using the "complete" command References: <87d23ovk55.fsf@redhat.com> <87ego4txco.fsf@redhat.com> <87vbh74uqd.fsf@redhat.com> <55257877.3040604@redhat.com> X-URL: http://blog.sergiodj.net Date: Wed, 08 Apr 2015 15:58:55 -0400 In-Reply-To: <55257877.3040604@redhat.com> (Keith Seitz's message of "Wed, 08 Apr 2015 11:50:31 -0700") Message-ID: <87384a1jeo.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes On Wednesday, April 08 2015, Keith Seitz wrote: >>> >+ This function is usually called twice: one when we are figuring out > > nitpick (sorry): "once" instead of "one" Thanks, fixed. > This looks good to me. > > I have applied this patch to my completer branch, and I can verify > that it fixes the (other) completion problems I've seen. I recommend > that a maintainer approve this. Thanks for the review, Keith! Here is the updated patch + ChangeLog entry. diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c index 1d89912..017d0b6 100644 --- a/gdb/python/py-cmd.c +++ b/gdb/python/py-cmd.c @@ -210,85 +210,70 @@ cmdpy_function (struct cmd_list_element *command, char *args, int from_tty) /* Helper function for the Python command completers (both "pure" completer and brkchar handler). This function takes COMMAND, TEXT and WORD and tries to call the Python method for completion with - these arguments. It also takes HANDLE_BRKCHARS_P, an argument to - identify whether it is being called from the brkchar handler or - from the "pure" completer. In the first case, it effectively calls - the Python method for completion, and records the PyObject in a - static variable (used as a "cache"). In the second case, it just - returns that variable, without actually calling the Python method - again. This saves us one Python method call. - - The reason for this two step dance is that we need to know the set - of "brkchars" to use early on, before we actually try to perform - the completion. But if a Python command supplies a "complete" - method then we have to call that method first: it may return as its - result the kind of completion to perform and that will in turn - specify which brkchars to use. IOW, we need the result of the - "complete" method before we actually perform the completion. - - It is important to mention that this function is built on the - assumption that the calls to cmdpy_completer_handle_brkchars and - cmdpy_completer will be subsequent with nothing intervening. This - is true for our completer mechanism. + these arguments. + + This function is usually called twice: once when we are figuring out + the break characters to be used, and another to perform the real + completion itself. The reason for this two step dance is that we + need to know the set of "brkchars" to use early on, before we + actually try to perform the completion. But if a Python command + supplies a "complete" method then we have to call that method + first: it may return as its result the kind of completion to + perform and that will in turn specify which brkchars to use. IOW, + we need the result of the "complete" method before we actually + perform the completion. The only situation when this function is + not called twice is when the user uses the "complete" command: in + this scenario, there is no call to determine the "brkchars". + + Ideally, it would be nice to cache the result of the first call (to + determine the "brkchars") and return this value directly in the + second call (to perform the actual completion). However, due to + the peculiarity of the "complete" command mentioned above, it is + possible to put GDB in a bad state if you perform a TAB-completion + and then a "complete"-completion sequentially. Therefore, we just + recalculate everything twice for TAB-completions. This function returns the PyObject representing the Python method call. */ static PyObject * cmdpy_completer_helper (struct cmd_list_element *command, - const char *text, const char *word, - int handle_brkchars_p) + const char *text, const char *word) { cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command); PyObject *textobj, *wordobj; - /* This static variable will server as a "cache" for us, in order to - store the PyObject that results from calling the Python - function. */ - static PyObject *resultobj = NULL; + PyObject *resultobj; - if (handle_brkchars_p) + if (obj == NULL) + error (_("Invalid invocation of Python command object.")); + if (!PyObject_HasAttr ((PyObject *) obj, complete_cst)) { - /* If we were called to handle brkchars, it means this is the - first function call of two that will happen in a row. - Therefore, we need to call the completer ourselves, and cache - the return value in the static variable RESULTOBJ. Then, in - the second call, we can just use the value of RESULTOBJ to do - our job. */ - if (resultobj != NULL) - Py_DECREF (resultobj); - - resultobj = NULL; - if (obj == NULL) - error (_("Invalid invocation of Python command object.")); - if (!PyObject_HasAttr ((PyObject *) obj, complete_cst)) - { - /* If there is no complete method, don't error. */ - return NULL; - } - - textobj = PyUnicode_Decode (text, strlen (text), host_charset (), NULL); - if (textobj == NULL) - error (_("Could not convert argument to Python string.")); - wordobj = PyUnicode_Decode (word, sizeof (word), host_charset (), NULL); - if (wordobj == NULL) - { - Py_DECREF (textobj); - error (_("Could not convert argument to Python string.")); - } + /* If there is no complete method, don't error. */ + return NULL; + } - resultobj = PyObject_CallMethodObjArgs ((PyObject *) obj, complete_cst, - textobj, wordobj, NULL); + textobj = PyUnicode_Decode (text, strlen (text), host_charset (), NULL); + if (textobj == NULL) + error (_("Could not convert argument to Python string.")); + wordobj = PyUnicode_Decode (word, strlen (word), host_charset (), NULL); + if (wordobj == NULL) + { Py_DECREF (textobj); - Py_DECREF (wordobj); - if (!resultobj) - { - /* Just swallow errors here. */ - PyErr_Clear (); - } + error (_("Could not convert argument to Python string.")); + } - Py_XINCREF (resultobj); + resultobj = PyObject_CallMethodObjArgs ((PyObject *) obj, complete_cst, + textobj, wordobj, NULL); + Py_DECREF (textobj); + Py_DECREF (wordobj); + if (!resultobj) + { + /* Just swallow errors here. */ + PyErr_Clear (); } + Py_XINCREF (resultobj); + return resultobj; } @@ -308,7 +293,7 @@ cmdpy_completer_handle_brkchars (struct cmd_list_element *command, /* Calling our helper to obtain the PyObject of the Python function. */ - resultobj = cmdpy_completer_helper (command, text, word, 1); + resultobj = cmdpy_completer_helper (command, text, word); /* Check if there was an error. */ if (resultobj == NULL) @@ -338,8 +323,7 @@ cmdpy_completer_handle_brkchars (struct cmd_list_element *command, done: - /* We do not call Py_XDECREF here because RESULTOBJ will be used in - the subsequent call to cmdpy_completer function. */ + Py_XDECREF (resultobj); do_cleanups (cleanup); } @@ -357,7 +341,7 @@ cmdpy_completer (struct cmd_list_element *command, /* Calling our helper to obtain the PyObject of the Python function. */ - resultobj = cmdpy_completer_helper (command, text, word, 0); + resultobj = cmdpy_completer_helper (command, text, word); /* If the result object of calling the Python function is NULL, it means that there was an error. In this case, just give up and @@ -419,6 +403,7 @@ cmdpy_completer (struct cmd_list_element *command, done: + Py_XDECREF (resultobj); do_cleanups (cleanup); return result; diff --git a/gdb/testsuite/gdb.python/py-completion.exp b/gdb/testsuite/gdb.python/py-completion.exp index 0e283e8..5e45087 100644 --- a/gdb/testsuite/gdb.python/py-completion.exp +++ b/gdb/testsuite/gdb.python/py-completion.exp @@ -40,7 +40,8 @@ gdb_test_multiple "" "completefileinit completion" { } # Just discarding whatever we typed. -gdb_test " " ".*" "discard #1" +set discard 0 +gdb_test " " ".*" "discard #[incr discard]" # This is the problematic one. send_gdb "completefilemethod ${testdir_complete}\t" @@ -54,7 +55,7 @@ gdb_test_multiple "" "completefilemethod completion" { } # Discarding again -gdb_test " " ".*" "discard #2" +gdb_test " " ".*" "discard #[incr discard]" # Another problematic set completion_regex "[string_to_regexp [standard_output_file "py-completion-t"]]" @@ -67,3 +68,63 @@ gdb_test_multiple "" "completefilecommandcond completion" { pass "completefilecommandcond completion" } } + +# Start gdb over again to clear out current state. This can interfere +# with the expected output of the below tests in a buggy gdb. +gdb_exit +gdb_start +gdb_test_no_output "source ${srcdir}/${subdir}/${testfile}.py" + +gdb_test_sequence "complete completel" \ + "list all completions of 'complete completel'" { + "completelimit1" + "completelimit2" + "completelimit3" + "completelimit4" + "completelimit5" + "completelimit6" + "completelimit7" + } + +# Discarding again +gdb_test " " ".*" "discard #[incr discard]" + +gdb_test_sequence "complete completelimit1 c" \ + "list all completions of 'complete completelimit1 c'" { + "completelimit1 cl11" + "completelimit1 cl12" + "completelimit1 cl13" + } + +# Discarding again +gdb_test " " ".*" "discard #[incr discard]" + +# If using readline, we can TAB-complete. This used to trigger a bug +# because the cached result from the completer was being reused for +# a different python command. +if {[readline_is_used]} { + set testname "tab-complete 'completelimit1 c'" + send_gdb "completelimit1 c\t" + gdb_test_multiple "" $testname { + -re "^completelimit1 c\\\x07l1$" { + pass $testname + + # Discard the command line + gdb_test " " ".*" "discard #[incr discard]" + } + } + + gdb_test_sequence "complete completelimit2 c" \ + "list all completions of 'complete completelimit2 c'" { + "completelimit2 cl21" + "completelimit2 cl210" + "completelimit2 cl22" + "completelimit2 cl23" + "completelimit2 cl24" + "completelimit2 cl25" + "completelimit2 cl26" + "completelimit2 cl27" + "completelimit2 cl28" + "completelimit2 cl29" + } +} diff --git a/gdb/testsuite/gdb.python/py-completion.py b/gdb/testsuite/gdb.python/py-completion.py index e75b6fc..1413c08 100644 --- a/gdb/testsuite/gdb.python/py-completion.py +++ b/gdb/testsuite/gdb.python/py-completion.py @@ -53,6 +53,95 @@ class CompleteFileCommandCond(gdb.Command): else: return gdb.COMPLETE_FILENAME +class CompleteLimit1(gdb.Command): + def __init__(self): + gdb.Command.__init__(self,'completelimit1',gdb.COMMAND_USER) + + def invoke(self,argument,from_tty): + raise gdb.GdbError('not implemented') + + def complete(self,text,word): + return ["cl11", "cl12", "cl13"] + +class CompleteLimit2(gdb.Command): + def __init__(self): + gdb.Command.__init__(self,'completelimit2', + gdb.COMMAND_USER) + + def invoke(self,argument,from_tty): + raise gdb.GdbError('not implemented') + + def complete(self,text,word): + return ["cl21", "cl23", "cl25", "cl27", "cl29", + "cl22", "cl24", "cl26", "cl28", "cl210"] + +class CompleteLimit3(gdb.Command): + def __init__(self): + gdb.Command.__init__(self,'completelimit3', + gdb.COMMAND_USER) + + def invoke(self,argument,from_tty): + raise gdb.GdbError('not implemented') + + def complete(self,text,word): + return ["cl31", "cl33", "cl35", "cl37", "cl39", + "cl32", "cl34", "cl36", "cl38", "cl310"] + +class CompleteLimit4(gdb.Command): + def __init__(self): + gdb.Command.__init__(self,'completelimit4', + gdb.COMMAND_USER) + + def invoke(self,argument,from_tty): + raise gdb.GdbError('not implemented') + + def complete(self,text,word): + return ["cl41", "cl43", "cl45", "cl47", "cl49", + "cl42", "cl44", "cl46", "cl48", "cl410"] + +class CompleteLimit5(gdb.Command): + def __init__(self): + gdb.Command.__init__(self,'completelimit5', + gdb.COMMAND_USER) + + def invoke(self,argument,from_tty): + raise gdb.GdbError('not implemented') + + def complete(self,text,word): + return ["cl51", "cl53", "cl55", "cl57", "cl59", + "cl52", "cl54", "cl56", "cl58", "cl510"] + +class CompleteLimit6(gdb.Command): + def __init__(self): + gdb.Command.__init__(self,'completelimit6', + gdb.COMMAND_USER) + + def invoke(self,argument,from_tty): + raise gdb.GdbError('not implemented') + + def complete(self,text,word): + return ["cl61", "cl63", "cl65", "cl67", "cl69", + "cl62", "cl64", "cl66", "cl68", "cl610"] + +class CompleteLimit7(gdb.Command): + def __init__(self): + gdb.Command.__init__(self,'completelimit7', + gdb.COMMAND_USER) + + def invoke(self,argument,from_tty): + raise gdb.GdbError('not implemented') + + def complete(self,text,word): + return ["cl71", "cl73", "cl75", "cl77", "cl79", + "cl72", "cl74", "cl76", "cl78", "cl710"] + CompleteFileInit() CompleteFileMethod() CompleteFileCommandCond() +CompleteLimit1() +CompleteLimit2() +CompleteLimit3() +CompleteLimit4() +CompleteLimit5() +CompleteLimit6() +CompleteLimit7()