Re: [PATCH] [Ada] Fix completion for multiple function matches

[Joel Brobecker <brobecker@adacore.com>]

> 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.

-- 
Joel