From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 38909 invoked by alias); 17 Mar 2017 16:50:18 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 38896 invoked by uid 89); 17 Mar 2017 16:50:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=producer's, mandatory, timwiederhakeintelcom, tim.wiederhake@intel.com X-HELO: mail-wr0-f177.google.com Received: from mail-wr0-f177.google.com (HELO mail-wr0-f177.google.com) (209.85.128.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 17 Mar 2017 16:50:15 +0000 Received: by mail-wr0-f177.google.com with SMTP id g10so55726590wrg.2 for ; Fri, 17 Mar 2017 09:50:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=ctA1AWqQurWDkp72Y7Q5I0lX1PtjzwBJwtV+MjC8neg=; b=uaCFkNLvBRFWmft8d/qxObPh6knwHZgooROuJkH5hi8CXX98y2MFbpqVr3a9MlxDew Z0eLkzb38fJzYRSQZPU+gafMcLhRiF9qwDQ4XCyUtLtvFEa3vXOJl6E0dHQpLLqx84fG olskOQ6J0K+n/1A7qxDd/+GdUbVtKpgHxKB3ZSh8IXiWUH2mHHgfXi2ifiUnmQMjpWnQ ehBmzRvmM7sadWwpNoMAEsN9eFIyh+dijBmfs/k2nG7dsmFYGb4gg3ltD4yXso9PhWFR wNBZbFkhVc49vhxjGzpqduUbSuoebBHk3xxmnlYdUqICWWM9Bvqar9zOjozD2/FGvygs Vmzg== X-Gm-Message-State: AFeK/H1p8r+sgAY1BC5A6EE8mI/7XcZaT4CJKtu9FhcgiGc2D/839l+2zxuC10r1CAOH1Q== X-Received: by 10.223.136.183 with SMTP id f52mr13210271wrf.68.1489769414464; Fri, 17 Mar 2017 09:50:14 -0700 (PDT) Received: from E107787-LIN ([194.214.185.158]) by smtp.gmail.com with ESMTPSA id h5sm3276745wmf.7.2017.03.17.09.50.13 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Fri, 17 Mar 2017 09:50:13 -0700 (PDT) From: Yao Qi To: "Wiederhake\, Tim" Cc: "gdb-patches\@sourceware.org" , "Metzger\, Markus T" , "palves\@redhat.com" , "xdje42\@gmail.com" Subject: Re: [PATCH v6 9/9] Add documentation for new record Python bindings. References: <1486989450-11313-1-git-send-email-tim.wiederhake@intel.com> <1486989450-11313-10-git-send-email-tim.wiederhake@intel.com> <86tw7apqf6.fsf@gmail.com> <9676A094AF46E14E8265E7A3F4CCE9AF942666@irsmsx105.ger.corp.intel.com> <86wpc1uwtz.fsf@gmail.com> <9676A094AF46E14E8265E7A3F4CCE9AF942A2E@irsmsx105.ger.corp.intel.com> Date: Fri, 17 Mar 2017 16:50:00 -0000 In-Reply-To: <9676A094AF46E14E8265E7A3F4CCE9AF942A2E@irsmsx105.ger.corp.intel.com> (Tim Wiederhake's message of "Tue, 7 Mar 2017 17:18:09 +0000") Message-ID: <86tw6rluf0.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2017-03/txt/msg00321.txt.bz2 "Wiederhake, Tim" 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 th= at I > suggested: "FullInstruction" has some functions / data members that happe= n to > have the same names as their "BtraceInstruction" counterparts but these p= ython > classes are not related in any way in terms of class inheritance. The use= r now > wants to record with method "full" and "instruction_history" returns not = a list > of BtraceInstruction objects but a list of FullInstruction objects; the i= nput > 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 =3D 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 ba= se > class, let's call it "RecordInstruction", and two Python sub classes, > "BtraceInstruction" and "FullInstruction", the example would still look e= xactly > like this: > > (gdb) record [full or btrace] > [RECORD SOME PROGRAM FLOW] > (gdb) python-interactive >>>> recorded_instructions =3D 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 n= ever > receive an instance of this base class to see what the common interface i= s. 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 =3D 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 m= embers, > 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 c= ommon > 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" w= ere > native Python classes: Less code in BtraceInstruction and FullInstruction= . But > BtraceInstruction and eventually FullInstruction are "Built-In"s from Pyt= hon's > perspective. That is, not real Python classes but some C code that interf= aces > with the Python interpreter. > > Let's assume that "number", "pc" and "sal" were to be the common interfac= e 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 b= ase > class approach. Both cases exhibit the same functionality but the one wit= h the > common base class adds lots of complexity and added code to maintain. > > So, no "if (method =3D ...)" in python code; the manual tells you how wri= te > (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"; an= d the > python API itself is actually not at all btrace specific, as one could ar= gue > 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. --=20 Yao (=E9=BD=90=E5=B0=A7)