From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 39270 invoked by alias); 7 Apr 2015 19:13:54 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 39257 invoked by uid 89); 7 Apr 2015 19:13:53 -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; Tue, 07 Apr 2015 19:13:51 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t37JDmN9028620 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Tue, 7 Apr 2015 15:13:49 -0400 Received: from localhost (unused-10-15-17-126.yyz.redhat.com [10.15.17.126]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t37JDldm001553 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA256 bits=256 verify=NO); Tue, 7 Apr 2015 15:13:48 -0400 From: Sergio Durigan Junior To: GDB Patches Cc: Keith Seitz Subject: Re: [PATCH] Fix Python completion when using the "complete" command References: <87d23ovk55.fsf@redhat.com> <87ego4txco.fsf@redhat.com> X-URL: http://blog.sergiodj.net Date: Tue, 07 Apr 2015 19:13:00 -0000 In-Reply-To: <87ego4txco.fsf@redhat.com> (Sergio Durigan Junior's message of "Tue, 31 Mar 2015 22:07:35 -0400") Message-ID: <87vbh74uqd.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2015-04/txt/msg00243.txt.bz2 On Tuesday, March 31 2015, I wrote: > On Tuesday, March 31 2015, I wrote: > >> Hi, > > Sorry, the previous patch contains a version of the testcase that does > not pass (I was using this testcase to test another version of the > patch). Attached is the updated patch with the correct testcase. All > the rest is the same. Ping. > -- > Sergio > GPG key ID: 0x65FC5E36 > Please send encrypted e-mail if possible > http://sergiodj.net/ > > gdb/ChangeLog: > 2015-03-31 Sergio Durigan Junior > > PR python/16699 > * python/py-cmd.c (cmdpy_completer_helper): Adjust function to not > use a caching mechanism. Adjust comments and code to reflect > that. Replace 'sizeof' by 'strlen' when fetching 'wordobj'. > (cmdpy_completer_handle_brkchars): Adjust call to > cmdpy_completer_helper. Call Py_XDECREF for 'resultobj'. > (cmdpy_completer): Likewise. > > gdb/testsuite/ChangeLog: > 2015-03-31 Keith Seitz > > PR python/16699 > * gdb.python/py-completion.exp: New tests for completion. > * gdb.python/py-completion.py (CompleteLimit1): New class. > (CompleteLimit2): Likewise. > (CompleteLimit3): Likewise. > (CompleteLimit4): Likewise. > (CompleteLimit5): Likewise. > (CompleteLimit6): Likewise. > (CompleteLimit7): Likewise. > > diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c > index 1d89912..29d8d90 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: one 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() -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/