Re: [PATCH 06/40] Expression completer should not match explicit location options

Pedro Alves
2017-06-29T15:23:49Z

On 06/29/2017 12:07 PM, Pedro Alves wrote:
> On 06/29/2017 11:56 AM, Pedro Alves wrote:
>
>> The patch fixes this by having the expression_completer function
>> bypass the function that completes explicit locations:
>>
>> /* Not ideal but it is what we used to do before... */
>> - return location_completer (ignore, p, word);
>> + return linespec_location_completer (ignore, text, word);
>>
>
> Hmm, reading this back made my realize that I'm dropping "p"
> here, which is computed just above. That doesn't make a difference
> to the test added by 37cd5d19fecc (2008), which was the commit
> that added that code. Hmm, looks like that's a bit of dead code
> we can remove.
>

I've split that to a separate preparatory patch and pushed it in,
as below, after doing some archaeology.

From 195bcdd5183f2c137399db23a68a26a4e4193f8f Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 29 Jun 2017 15:52:37 +0100
Subject: [PATCH] Remove old stale expression_completer hack

The code in question was introduced by:

https://sourceware.com/ml/gdb-patches/2008-06/msg00143.html

"The fix is to make sure that the entire expression is passed to
expression_completer, then duplicate some logic there in the case
where location_completer is called."

The logic that was duplicated was much later on removed by the
original explicit locations patch:

commit 87f0e7204722a986f79f245eee716f0870832d47
Author: Keith Seitz <keiths@redhat.com>
AuthorDate: Tue Aug 11 17:09:36 2015 -0700
Commit: Keith Seitz <keiths@redhat.com>
CommitDate: Tue Aug 11 17:09:36 2015 -0700

Explicit locations: add UI features for CLI

@@ -688,16 +880,6 @@ complete_line_internal (const char *text,
rl_completer_word_break_characters =
gdb_completer_file_name_break_characters;
}
- else if (c->completer == location_completer)
- {
- /* Commands which complete on locations want to
- see the entire argument. */
- for (p = word;
- p > tmp_command
- && p[-1] != ' ' && p[-1] != '\t';
- p--)
- ;
- }

However this case in expression_completer was left behind.

I couldn't come up with a test where this currently makes any
difference.

gdb/ChangeLog:
2017-06-29 Pedro Alves <palves@redhat.com>

* completer.c (expression_completer): Remove code that recomputes
'text' from 'word'.
---
gdb/ChangeLog | 5 +++++
gdb/completer.c | 10 +---------
2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9be08dd..c76158f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2017-06-29 Pedro Alves <palves@redhat.com>
+
+ * completer.c (expression_completer): Remove code that recomputes
+ 'text' from 'word'.
+
2017-06-29 Yao Qi <yao.qi@linaro.org>

* regformats/regdat.sh: Generate code with
diff --git a/gdb/completer.c b/gdb/completer.c
index 6acf115..f152dd5 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -587,7 +587,6 @@ expression_completer (struct cmd_list_element *ignore,
{
struct type *type = NULL;
char *fieldname;
- const char *p;
enum type_code code = TYPE_CODE_UNDEF;

/* Perform a tentative parse of the expression, to see whether a
@@ -635,15 +634,8 @@ expression_completer (struct cmd_list_element *ignore,
}
xfree (fieldname);

- /* Commands which complete on locations want to see the entire
- argument. */
- for (p = word;
- p > text && p[-1] != ' ' && p[-1] != '\t';
- p--)
- ;
-
/* Not ideal but it is what we used to do before... */
- return location_completer (ignore, p, word);
+ return location_completer (ignore, text, word);
}

/* See definition in completer.h. */
--
2.5.5