Re: [PATCH] Add call to prune_program_spaces in mi_cmd_remove_inferior

[Doug Evans <dje@google.com>]

On Thu, Sep 25, 2014 at 8:07 AM, Simon Marchi <simon.marchi@ericsson.com> wrote:
> On 2014-09-24 06:43 PM, Doug Evans wrote:
>> Hi.
>>
>> One of my pet peeves of gdb is that too much implementation logic is
>> spread throughout gdb.
>> By that I mean random bits of gdb take on the job of maintaining
>> random bits of internal gdb state,
>> instead of calling one routine (or very few) whose job it is to
>> encapsulate all that knowledge.
>>
>> It's not clear that that applies here, but I think it does.
>>
>> With that in mind the first question that comes to mind when reviewing
>> this patch is:
>> "Is there ever a time when deleting an inferior (from outside inferior.c)
>> would ever *not* want to also prune program spaces (at least by default)?"
>
> I had the same thought.
>
> I actually have another patch in the pipeline that addresses this. I think that
> the pruning approach is a bit wasteful when deleting an inferior. The only possible
> program space that we could possibly delete is the one that is tied to the deleted
> inferior. I was thinking of adding something like this in delete_inferior:
>
>   /* If this program space is rendered useless, remove it. */
>   if (pspace_empty_p (inf->pspace))
>       delete_program_space (inf->pspace);
>
> (this is done after "inf" has been removed from the inferiors list, such that
> pspace_empty_p returns true if inf was the last inferior tied to that pspace)
>
> I think this will allow to completely remove the prune_program_spaces function,
> since deleting an inferior is the only case where this is used. If you prefer,
> I can go directly with a patch like that and drop this one. I sent the current
> one first because I thought it would be a bit more obvious and require less
> discussion (and at least get the functionality right).

That would be nice.
I only stopped where I did because I didn't want to impose vetting all
callers of delete_inferior(_*) on this patch series.

I don't have a preference, and I don't want to impose too much
specifics on the patch since you're the one writing it.  OTOH, I do
want to make progress and do a bit more than your original patch.
If you're willing to go directly to always deleting unused progspaces
whenever we delete inferiors then let's do that.

---

btw, it would be nice to have an assert that we're not deleting the
last inferior.
"There's always (at least) one inferior."
This invariant should be readily visible to readers of delete_inferior(_*).

btw #2, There's also the invariant "There's always (at least) one
program space."
One is left with the question of whether they could be unrelated.
IOW could there be an inferior without a program space or a program
space without an inferior?
[At least in general. There are special cases where we do temporary
hacks to get through forks and such.]
Another cleanup could be to make this clearer.