Re: [PATCH v6 9/9] Add documentation for new record Python bindings.

[Yao Qi <qiyaoltc@gmail.com>]

"Wiederhake, Tim" <tim.wiederhake@intel.com> writes:

> For the sake of the argument, let's assume that "number", "pc" and "sal" are not
> unique to btrace and that support for "full" is implemented in the way that I
> suggested: "FullInstruction" has some functions / data members that happen to
> have the same names as their "BtraceInstruction" counterparts but these python
> classes are not related in any way in terms of class inheritance. The user now
> wants to record with method "full" and "instruction_history" returns not a list
> of BtraceInstruction objects but a list of FullInstruction objects; the input
> looks like this:

That is not the use case of OOP.  I, as a user, write a python script to
get instructions (which have attributes "number", "pc", and "sal"), I
don't have to know which record method is used.  Just start record,
want gdb gives me a list of instructions, and iterate them to parse
these attributes of each instruction.

>
> (gdb) record full
> [RECORD SOME PROGRAM FLOW]
> (gdb) python-interactive
>>>> recorded_instructions = gdb.current_recording().instruction_history
>>>> for instruction in recorded_instructions:
> ...   print(i.number, hex(i.pc), i.sal)
> ...
> [OUTPUT]
>
> It is exactly the same. Now if we were to have some Python instruction base
> class, let's call it "RecordInstruction", and two Python sub classes,
> "BtraceInstruction" and "FullInstruction", the example would still look exactly
> like this:
>
> (gdb) record [full or btrace]
> [RECORD SOME PROGRAM FLOW]
> (gdb) python-interactive
>>>> recorded_instructions = gdb.current_recording().instruction_history
>>>> for instruction in recorded_instructions:
> ...   print(i.number, hex(i.pc), i.sal)
> ...
> [OUTPUT]
>
> The user has no benefit from the existance of a python base class. They never
> receive an instance of this base class to see what the common interface is. The
> pythonic way -- as far as I understand it -- is to rely on the existance of
> functions / data members of the objects, not on the actual type of the
> object.

The benefit is that we can easily enforce the instruction attributes for
new added record methods.  If I want to add "FullInstruction",
"BarInstruction", or "FooInstruction", how can we make sure these
classes have these mandatory or basic attributes "number", "pc", and
"sal" so that the following python script can still be used with new
record format?

recorded_instructions = gdb.current_recording().instruction_history
for instruction in recorded_instructions:
   print(i.number, hex(i.pc), i.sal)

Secondly, the document of these interfaces becomes simpler with class
inheritance.  We only need to document these mandatory attributes in
base class once.  In your current approach, we need to document these
attributes separately in each class.

> That is what I meant with "duck typing". Both BtraceInstruction and
> FullInstruction behave in the same way for a couple of functions / data members,
> they "quack like a duck" and therefore "are a duck", no matter that they are
> instances of two unrelated classes.
>

This is from consumer's point of view.  I am looking at this from the
producer's or interface provider's point of view.

> What exactly forms this common interface has yet to defined and codified in the
> documentation. But this is in my opinion the beauty of this approach: All
> functions / data members of BtraceInstruction that are not part of this common
> interface automatically are "just there" and "additional functionality".
>
> The approach of having a common base class /would/ have some benefit, if all
> three of "RecordInstruction", "BtraceInstruction" and "FullInstruction" were
> native Python classes: Less code in BtraceInstruction and FullInstruction. But
> BtraceInstruction and eventually FullInstruction are "Built-In"s from Python's
> perspective. That is, not real Python classes but some C code that interfaces
> with the Python interpreter.
>
> Let's assume that "number", "pc" and "sal" were to be the common interface and
> therefore data members of "RecordInstruction". The C code for this Python class
> now needs to know which recording is currently active, since there is no
>
> current_target.to_get_pc_for_recorded_instruction_by_number(struct target_ops*,
> 							unsigned int);
>

We can change GDB C code, like target_ops, whenever we want, however, we
can't change external python interface.  I am focusing on the external
python interface now.

>
> From my point of view, there is no benefit for the user from the common base
> class approach. Both cases exhibit the same functionality but the one with the
> common base class adds lots of complexity and added code to maintain.
>
> So, no "if (method = ...)" in python code; the manual tells you how write
> (python) code that works regardless of the recording method in a yet to be
> written and to be decided upon paragraph about the "common interface"; and the
> python API itself is actually not at all btrace specific, as one could argue
> that all implemented functions and data members are currently just "added
> functionality for the btrace case".

I'll do some experimental hacking to help me fully understanding whether
my suggestion is a good idea or bad idea.

-- 
Yao (齐尧)