[Ada] Fix completion for multiple function matches
Commit Message
Before this change, trying to complete an expression ending with an
ambiguous function name (i.e. for which there are multiple matches)
would display a menu with a prompt for the user to pick one. For
instance:
(gdb) p func<tab>Multiple matches for func
[0] cancel
[1] pack2.func at pack2.adb:5
[2] pack.func at pack.adb:5
>
This is not user friendly and actually triggered a segmentation fault
after the user did pick one. It is not clear whether the segmentation
fault needs a separate fix, but this is the only known case which
exhibits it at the moment, and this case must be fixed itself.
The problem lies in ada-lang.c (ada_resolve_function): when we got
multiple matches, we should not display the menu if we are in completion
mode. This patch adjusts the corresponding condition accordingly.
gdb/ChangeLog:
* ada-lang.c (ada_resolve_function): Do not ask the user what
match to use when in completion mode.
gdb/testsuite/ChangeLog:
* gdb.ada/complete.exp: Add "pck.ambiguous_func" to the relevant
expected outputs. Add two testcases for completing ambiguous
functions.
* gdb.ada/complete/aux_pck.adb: New file.
* gdb.ada/complete/aux_pck.ads: New file.
* gdb.ada/complete/foo.adb: Pull Aux_Pck and call the two
Ambiguous_Func functions.
* gdb.ada/complete/pck.ads: Add an Ambiguous_Func function.
* gdb.ada/complete/pck.adb: Likewise.
Tested on x86_64-linux, no regression.
---
gdb/ada-lang.c | 5 ++++-
gdb/testsuite/gdb.ada/complete.exp | 10 ++++++++++
gdb/testsuite/gdb.ada/complete/aux_pck.adb | 23 +++++++++++++++++++++++
gdb/testsuite/gdb.ada/complete/aux_pck.ads | 20 ++++++++++++++++++++
gdb/testsuite/gdb.ada/complete/foo.adb | 5 ++++-
gdb/testsuite/gdb.ada/complete/pck.adb | 5 +++++
gdb/testsuite/gdb.ada/complete/pck.ads | 2 ++
7 files changed, 68 insertions(+), 2 deletions(-)
create mode 100644 gdb/testsuite/gdb.ada/complete/aux_pck.adb
create mode 100644 gdb/testsuite/gdb.ada/complete/aux_pck.ads
Comments
> gdb/ChangeLog:
>
> * ada-lang.c (ada_resolve_function): Do not ask the user what
> match to use when in completion mode.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.ada/complete.exp: Add "pck.ambiguous_func" to the relevant
> expected outputs. Add two testcases for completing ambiguous
> functions.
> * gdb.ada/complete/aux_pck.adb: New file.
> * gdb.ada/complete/aux_pck.ads: New file.
> * gdb.ada/complete/foo.adb: Pull Aux_Pck and call the two
> Ambiguous_Func functions.
> * gdb.ada/complete/pck.ads: Add an Ambiguous_Func function.
> * gdb.ada/complete/pck.adb: Likewise.
Thanks for the patch, Pierre-Marie.
Overall, the patch looks like definite progress as is, and I'd like
to see it pushed to avoid both the unsollicited MCQ and the ensuing
SEGV. However, I'm wondering if it might still be leaving the hole
in the completion.
In particular, what happens to the completion of "p ambig" if
you add a second function called, say "Aux_Pck.Ambiguous_Prog"?
In other words, I am wondering if the fact that we're returning
zero when parsing for completion might not unexpectedly filter out
some matches. Regardless of the answer, that's probably a worthwhile
adjustment to the tests you propose.
So, here's the plan I propose: We adjust the very minor suggestions
I made below, adjust the testcase as suggested above, do a quick
second round of review and then push the patch. If if turns out
there is a hole (I am not sure), then we can put that on the list
of things to improve and decide when we want to look at it.
> + /* If we got multiple matches, ask the user which one to use. Don't do this
> + interactive thing during completion, though: it is irritating to get a
> + prompt in this context. */
I wouldn't say "irritating" as, to me, this is a bug, not an
inconvenience. So, I would change the comment to say something like:
/* If we got multiple matches, ask the user which one to use. Don't do this
interactive thing during completion, though, as the purpose of the
completion is providing a list of all possible matches. Prompting the
user to filter it down would be completely unexpected in this case. */
> +# Usually, parsing a function name that is ambiguous yields a menu through
> +# which users can select a specific function. This should not happen during
> +# completion, though.
Missing second space after a period.
> +test_gdb_complete "ambig" \
> + "p ambiguous_func"
> +test_gdb_complete "ambiguous_func" \
> + "p ambiguous_func"
> diff --git a/gdb/testsuite/gdb.ada/complete/aux_pck.adb b/gdb/testsuite/gdb.ada/complete/aux_pck.adb
> new file mode 100644
> index 0000000..18e060e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/complete/aux_pck.adb
> @@ -0,0 +1,23 @@
> +-- Copyright 2008-2015 Free Software Foundation, Inc.
I *think* you want to only list 2015 in this case. Unless I missed
the fact that you copied this file from elsewhere...
> diff --git a/gdb/testsuite/gdb.ada/complete/aux_pck.ads b/gdb/testsuite/gdb.ada/complete/aux_pck.ads
> new file mode 100644
> index 0000000..36fdb6c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/complete/aux_pck.ads
> @@ -0,0 +1,20 @@
> +-- Copyright 2008-2015 Free Software Foundation, Inc.
Same here.
@@ -3655,9 +3655,12 @@ ada_resolve_function (struct block_symbol syms[],
}
}
+ /* If we got multiple matches, ask the user which one to use. Don't do this
+ interactive thing during completion, though: it is irritating to get a
+ prompt in this context. */
if (m == 0)
return -1;
- else if (m > 1)
+ else if (m > 1 && !parse_completion)
{
printf_filtered (_("Multiple matches for %s\n"), name);
user_select_syms (syms, m, 1);
@@ -140,6 +140,7 @@ test_gdb_complete "external_ident" \
test_gdb_complete "pck" \
[multi_line "(p pck\\.ad\[sb\])?" \
"(p pck\\.ad\[sb\])?" \
+ "p pck.ambiguous_func" \
"p pck.external_identical_one" \
"p pck.inner.inside_variable" \
"p pck.local_identical_one" \
@@ -151,6 +152,7 @@ test_gdb_complete "pck" \
test_gdb_complete "pck." \
[multi_line "(p pck\\.ad\[sb\])?" \
"(p pck\\.ad\[sb\])?" \
+ "p pck.ambiguous_func" \
"p pck.external_identical_one" \
"p pck.inner.inside_variable" \
"p pck.local_identical_one" \
@@ -181,3 +183,11 @@ if { [readline_is_used] } {
}
}
}
+
+# Usually, parsing a function name that is ambiguous yields a menu through
+# which users can select a specific function. This should not happen during
+# completion, though.
+test_gdb_complete "ambig" \
+ "p ambiguous_func"
+test_gdb_complete "ambiguous_func" \
+ "p ambiguous_func"
new file mode 100644
@@ -0,0 +1,23 @@
+-- Copyright 2008-2015 Free Software Foundation, Inc.
+--
+-- This program is free software; you can redistribute it and/or modify
+-- it under the terms of the GNU General Public License as published by
+-- the Free Software Foundation; either version 3 of the License, or
+-- (at your option) any later version.
+--
+-- This program is distributed in the hope that it will be useful,
+-- but WITHOUT ANY WARRANTY; without even the implied warranty of
+-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+-- GNU General Public License for more details.
+--
+-- You should have received a copy of the GNU General Public License
+-- along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+package body Aux_Pck is
+
+ procedure Ambiguous_Func is
+ begin
+ null;
+ end Ambiguous_Func;
+
+end Aux_Pck;
new file mode 100644
@@ -0,0 +1,20 @@
+-- Copyright 2008-2015 Free Software Foundation, Inc.
+--
+-- This program is free software; you can redistribute it and/or modify
+-- it under the terms of the GNU General Public License as published by
+-- the Free Software Foundation; either version 3 of the License, or
+-- (at your option) any later version.
+--
+-- This program is distributed in the hope that it will be useful,
+-- but WITHOUT ANY WARRANTY; without even the implied warranty of
+-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+-- GNU General Public License for more details.
+--
+-- You should have received a copy of the GNU General Public License
+-- along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+package Aux_Pck is
+
+ procedure Ambiguous_Func;
+
+end Aux_Pck;
@@ -13,7 +13,8 @@
-- You should have received a copy of the GNU General Public License
-- along with this program. If not, see <http://www.gnu.org/licenses/>.
-with Pck; use Pck;
+with Aux_Pck; use Aux_Pck;
+with Pck; use Pck;
procedure Foo is
Some_Local_Variable : Integer := 1;
@@ -21,5 +22,7 @@ procedure Foo is
begin
My_Global_Variable := Some_Local_Variable + 1; -- START
Proc (External_Identical_Two);
+ Aux_Pck.Ambiguous_Func;
+ Pck.Ambiguous_Func;
end Foo;
@@ -21,4 +21,9 @@ package body Pck is
Inner.Inside_Variable := Not_In_Scope + I;
end Proc;
+ procedure Ambiguous_Func is
+ begin
+ null;
+ end Ambiguous_Func;
+
end Pck;
@@ -31,4 +31,6 @@ package Pck is
procedure Proc (I : Integer);
+ procedure Ambiguous_Func;
+
end Pck;