Re: [PATCH 04/21] gdb: make interp_lookup a method of struct ui

[Simon Marchi <simon.marchi@efficios.com>]

>> diff --git a/gdb/python/python.c b/gdb/python/python.c
>> index 6a978d632e9a..a1aaa29d0ce5 100644
>> --- a/gdb/python/python.c
>> +++ b/gdb/python/python.c
>> @@ -683,7 +683,7 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
>>  
>>  	/* Use the console interpreter uiout to have the same print format
>>  	   for console or MI.  */
>> -	interp = interp_lookup (current_ui, "console");
>> +	interp = current_ui->lookup_interp ("console");
> 
> Might as well take this opportunity to replace "console" with
> INTERP_CONSOLE which is what we use everywhere else.
Good find, I'll add a preparatory patch that fixes it first.

> 
>>  	current_uiout = interp->interp_ui_out ();
>>  
>>  	if (to_string)
>> diff --git a/gdb/ui.c b/gdb/ui.c
>> index 624187ac73c3..a7b81c077e9a 100644
>> --- a/gdb/ui.c
>> +++ b/gdb/ui.c
>> @@ -169,7 +169,30 @@ ui::add_interp (interp *interp)
>>    this->interp_list.push_back (*interp);
>>  }
>>  
>> -/* See top.h.  */
>> +/* See interps.h.  */
>> +
>> +interp *
>> +ui::lookup_interp (const char *name)
>> +{
>> +  if (name == nullptr || strlen (name) == 0)
>> +    return nullptr;
>> +
>> +  /* Only create each interpreter once per UI.  */
>> +  interp *interp = this->lookup_existing_interp (name);
>> +  if (interp != nullptr)
>> +    return interp;
>> +
>> +  const interp_factory *factory = find_interp_factory (name);
>> +  if (factory == nullptr)
>> +    return nullptr;
>> +
>> +  interp = factory->func (factory->name);
>> +  this->add_interp (interp);
>> +
>> +  return interp;
>> +}
> 
> Rather than exposing the interpreter factor stuff outside of interp.c, I
> think a better approach would be either move ui::lookup_interp into
> interp.c -- personally, I'm happy to see different parts of an object in
> different .c files when that makes sense.

I don't know, I really like having declarations and definitions in
matching .h and .c files.

> But if you prefer all to keep ui implementation in ui.c, rather than
> calling find_interp_factory and then factory->func, we could just have a
> new function `create_interp (name)` which creates an interpreter, or
> returns nullptr, then we can write:
> 
>   interp = create_interp (name);
>   if (interp != nullptr)
>     this->add_interp (interp);
>   return interp;
> 
> And all the factory stuff can remain private to interp.c.

Yes, that makes sense, I'll do that.

Simon