Re: [PATCH v5] [PR mi/29554] New PRINT-VALUES option '--scalar-values'.

[Andrew Burgess <aburgess@redhat.com>]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gareth Rees <grees@undo.io>
>> Date: Fri, 10 Mar 2023 11:04:42 +0000
>> Cc: gdb-patches@sourceware.org
>> 
>> This also makes sense to me, but this is not a decision that I am in a
>> position to make! The experienced GDB developers -- in particular, you
>> and Eli -- need to come to an agreement about which approach is best
>> in this case: do you prefer to take the risk of a backward-
>> incompatible change (GDB/MI clients can no longer get the values for
>> references to compound types in C++ programs using --simple-values) or
>> do you prefer to accept the cost of leaving (what looks like) a
>> mistake to stand forever? I can implement whichever you think is best.
>> 
>> For the moment I will prepare a revised patch implementing solution #1
>> and addressing your other review comments. If Eli can also follow up
>> and help us reach a conclusion on the best design, that would be
>> great.
>
> The reason why I preferred #2 is simple: it avoids incompatible
> changes in the behavior of existing options.

I think we need to be careful not to paint this change as worse than it
really is.

If we picked option #1 then this would change the output in some cases,
so obviously, there could be a user out there that depends on a
particular output in a given situation, and any changes will break their
use case.  But....

... for a well written front-end, picking option #1 should be a
transparent change.  They already have to handle passing non-references
to non-scalar types.  Option #1 simply redistributes which value types
fall into which category, it doesn't do anything that we didn't do
before.


>                                               Since it was not really
> clear-cut that the previous behavior was a bug, the backward
> incompatibility could cause trouble to some application or use case
> which didn't consider it was a bug and relied on that behavior.

I agree, we will likely never know with certainty if the existing GDB
behaviour was by design, or by accident.

That said, my guess would be accident, but I also think that's likely
irrelevant here.

GDB isn't printing something that's incorrect -- that would clearly be a
bug.  And GDB isn't crashing -- that too would clearly be a bug.

So I think this becomes more a GDB development question: if we release a
miss-feature does that mean we have to guarantee to support that
miss-feature forever?  Or are we allowed to go in an make adjustments?

On one extreme we can take the position that once somethings out the
door we must NEVER break compatibility.  EVER.

On the other extreme everything could change with every release.

I think the both of these positions would not be good for GDB.
Personally, I'm more toward the conservative end.  I think stability is
good.

But I also think we do need to be open to making considered changes,
otherwise, we end up carrying around an increasing volume of technical
debt that makes future work progressively harder.

In this case my think was:

  1. The option #1 behaviour is now inline with GDB's CLI behaviour,
     i.e. hiding the values of non-scalar reference variables by default,

  2. A well written application should transparently handle this case
     anyway as we're not adding a new output format, and

  3. The current behaviour (I would say) is clearly unexpected, you ask
     GDB for simple values only, and you could get back a value of
     (almost) unlimited size.

  4. The option #1 patch already included an MI feature flag.  This
     doesn't stop a badly written MI front-end from breaking, but it
     does mean the maintainer can query the flag to figure out when to
     apply the fix-up behaviour.

> Introducing a new option is free from this problem.

But not free in maintenance cost.

If we really feel that the old behaviour is worth saving then adding a
new flag isn't the end of the world.  I'd just want to make sure we
really have considered the alternatives first.

Thanks,

Andrew