Re: [PATCH v4 9/9] enable target-async

[Pedro Alves <palves@redhat.com>]

Hi Tom,

still not a full review, but I thought I'd send out a
couple comments as I have them.

On 10/22/2013 06:59 PM, Tom Tromey wrote:
>  /* Never display the default GDB prompt in MI case.  */
>  
>  static int
>  mi_interpreter_prompt_p (void *data)
>  {

Looks quite odd for a predicate function to actually have
side effects.  I guess this is the hack you mentioned?
I think this is missing a comment explaining what is
going on.  It's not obvious at all to me.

> +  if (!interp_quiet_p (NULL))
> +    {
> +      if (!target_is_async_p ()
> +	  || (!sync_execution
> +	      && (!target_async_permitted
> +		  || iterate_over_threads (thread_command_not_mi,
> +					   NULL) == NULL)))
> +	{
> +	  fputs_unfiltered ("(gdb) \n", raw_stdout);
> +	  gdb_flush (raw_stdout);
> +	}
> +    }
> +
>    return 0;
>  }


> @@ -1837,7 +1851,7 @@ mi_cmd_list_target_features (char *command, char **argv, int argc)
>        struct ui_out *uiout = current_uiout;
>
>        cleanup = make_cleanup_ui_out_list_begin_end (uiout, "features");
> -      if (target_can_async_p ())
> +      if (mi_target_can_async_p ())
>  	ui_out_field_string (uiout, NULL, "async");
>        if (target_can_execute_reverse)
>  	ui_out_field_string (uiout, NULL, "reverse");

Hmm, not sure this is right.  This supposedly returns the set of
supported features.  But mi_target_can_async_p returns false
until the frontend enables target-async.  So this change creates
a sort of chicken and egg situation.


> --- a/gdb/testsuite/gdb.mi/mi-cli.exp
> +++ b/gdb/testsuite/gdb.mi/mi-cli.exp
> @@ -134,20 +134,7 @@ mi_gdb_test "500-stack-select-frame 0" \
>    {500\^done} \
>    "-stack-select-frame 0"
>
> -# When a CLI command is entered in MI session, the respose is different in
> -# sync and async modes. In sync mode normal_stop is called when current
> -# interpreter is CLI. So:
> -#   - print_stop_reason prints stop reason in CLI uiout, and we don't show it
> -#     in MI
> -#   - The stop position is printed, and appears in MI 'console' channel.
> -#
> -# In async mode the stop event is processed when we're back to MI interpreter,
> -# so the stop reason is printed into MI uiout an.
> -if {$async} {
> -    set reason "end-stepping-range"
> -} else {
> -    set reason ""
> -}
> +set reason "end-stepping-range"

I'm a little confused by this one.  Isn't it still necessary
for targets that don't do async?

-- 
Pedro Alves