Re: [PATCH] Throw error if ambiguous name is used in gdb.parameter

[Andrew Burgess <aburgess@redhat.com>]

Thanks for addressing this.

Hannes Domani <ssbssa@yahoo.de> writes:

> Currently gdb.parameter doesn't throw an error if an ambiguous

In the subject line and again here you talk about thowing an error.  I
think the correct Python terminology would be raising an exception.

> name is used, it instead returns the value of the last partly
> matching parameter:
> ```
> (gdb) show print sym
> Ambiguous show print command "sym": symbol, symbol-filename, symbol-loading.
> (gdb) show print symbol-loading
> Printing of symbol loading messages is "full".
> (gdb) py print(gdb.parameter("print sym"))
> full
> ```
>
> It's because lookup_cmd_composition_1 tries to detect
> ambigous names by checking the return value of find_cmd
> for CMD_LIST_AMBIGUOUS, which never happens, since only
> lookup_cmd_1 returns CMD_LIST_AMBIGUOUS.
> Instead the nfound argument contains the number of found
> matches.
>
> By using it instead, it's working/failing as expected:
> ```
> (gdb) py print(gdb.parameter("print sym"))
> Traceback (most recent call last):
>   File "<string>", line 1, in <module>
> RuntimeError: Could not find parameter `print sym'.
> Error while executing Python code.

It might be nice if this error could indicate that the named parameter
was ambiguous rather than not found.  Similar to how on the command line
when I do this:

  (gdb) set print sym
  Ambiguous set print command "sym": symbol, symbol-filename, symbol-loading.

But I looked at how this might be done, and it looks like it would need
some larger restructuring, so I don't think that should prevent your
patch going in.

> (gdb) py print(gdb.parameter("print symbol"))
> True
> (gdb) py print(gdb.parameter("print symbol-"))
> Traceback (most recent call last):
>   File "<string>", line 1, in <module>
> RuntimeError: Could not find parameter `print symbol-'.
> Error while executing Python code.
> (gdb) py print(gdb.parameter("print symbol-load"))
> full
> ```
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=14639
> ---
>  gdb/cli/cli-decode.c                      |  2 +-
>  gdb/testsuite/gdb.python/py-parameter.exp | 32 +++++++++++++++++++++++
>  2 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
> index 940cd6a2c8e..12aa152d0c4 100644
> --- a/gdb/cli/cli-decode.c
> +++ b/gdb/cli/cli-decode.c
> @@ -2589,7 +2589,7 @@ lookup_cmd_composition_1 (const char *text,
>        *cmd = find_cmd (command.c_str (), len, cur_list, 1, &nfound);
>  
>        /* We only handle the case where a single command was found.  */
> -      if (*cmd == CMD_LIST_AMBIGUOUS || *cmd == nullptr)
> +      if (nfound > 1 || *cmd == nullptr)

I wonder if `find_cmd` should return nullptr if `nfound != 1`?  It
doesn't seem like returning what is basically a random setting is ever
going to be useful...

But I think it's optional if you want to make that change.  I'm happy
with this as it is:

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


>  	return 0;
>        else
>  	{
> diff --git a/gdb/testsuite/gdb.python/py-parameter.exp b/gdb/testsuite/gdb.python/py-parameter.exp
> index c1f8a80c6b7..df1b93028ad 100644
> --- a/gdb/testsuite/gdb.python/py-parameter.exp
> +++ b/gdb/testsuite/gdb.python/py-parameter.exp
> @@ -588,6 +588,37 @@ proc_with_prefix test_language {} {
>      gdb_test_no_output "set lang auto"
>  }
>  
> +proc_with_prefix test_ambiguous_parameter {} {
> +    gdb_test_multiline "create parameter" \
> +	"python" "" \
> +	"class TestAmbiguousParam (gdb.Parameter):" "" \
> +	"   def __init__ (self, name, value):" "" \
> +	"      super (TestAmbiguousParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_INTEGER)" "" \
> +	"      self.value = value" "" \
> +	"end"
> +
> +    # Create parameters.
> +    gdb_test "python TestAmbiguousParam('test-ambiguous-value-1', 1)" ""
> +    gdb_test "python TestAmbiguousParam('test-ambiguous-value-2-extra', 2)" ""
> +    gdb_test "python TestAmbiguousParam('test-ambiguous', 3)" ""
> +
> +    # Test unambiguous matches.
> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-1'))" "1"
> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-2-extra'))" "2"
> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-2'))" "2"
> +    gdb_test "python print(gdb.parameter('test-ambiguous'))" "3"
> +
> +    # Test ambiguous names.
> +    gdb_test "python print(gdb.parameter('test-ambiguou'))" \
> +	"Could not find parameter.*Error while executing Python code."
> +    gdb_test "python print(gdb.parameter('test-ambiguous-'))" \
> +	"Could not find parameter.*Error while executing Python code."
> +    gdb_test "python print(gdb.parameter('test-ambiguous-v'))" \
> +	"Could not find parameter.*Error while executing Python code."
> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-1a'))" \
> +	"Could not find parameter.*Error while executing Python code."
> +}
> +
>  test_directories
>  test_data_directory
>  test_boolean_parameter
> @@ -600,5 +631,6 @@ test_gdb_parameter
>  test_integer_parameter
>  test_throwing_parameter
>  test_language
> +test_ambiguous_parameter
>  
>  rename py_param_test_maybe_no_output ""
> -- 
> 2.35.1