From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 80620 invoked by alias); 29 Jun 2017 15:24:45 -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 63226 invoked by uid 89); 29 Jun 2017 15:24:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=H*r:sk:static., Inheritance, c X-HELO: mail-it0-f41.google.com Received: from mail-it0-f41.google.com (HELO mail-it0-f41.google.com) (209.85.214.41) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 29 Jun 2017 15:24:22 +0000 Received: by mail-it0-f41.google.com with SMTP id v202so47223554itb.0 for ; Thu, 29 Jun 2017 08:24:22 -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=Vt0pHz8/UoT7u1yQpMyatXRa7/p+ccnXfpgRDaVyp8U=; b=Q/pYdS4VVnPG6CwEAZBTxvyREtJBDVaBCSGwJoaUKQe9ZmCk2RQpZ7fXbgtg/SIa3X hXXZpEfI5tBjguXIayja+I3JIyocjw7euDEpZ2Y+N4+jFfnYcRfXE3LGhbfOOphZx8ec xDihpCNqgers4APvvYZElvStaQAvD7VyVDDCL8F1EGyNG9t7mqyxHB3ItDcLHWCPE4e+ ukwHeRw0vM88qGtoDlgq+vF2ZegWg50bakYkUe49vPATOACdUThUs00OGzcFghtQkykW otOCkDuy4ln3YFu1iEhjglYU0IQKETBhzgvkmbu4zPdJSUUACeC05ryDr35r94hAqNWd FklA== X-Gm-Message-State: AKS2vOxgcFe0BpHzxXAh9eJBdaoCbv3OZWjSqKCY8kkKwJr5SyoJpa2j 70j/5n6vsLq5up0Y X-Received: by 10.36.69.210 with SMTP id c79mr2508570itd.109.1498749860865; Thu, 29 Jun 2017 08:24:20 -0700 (PDT) Received: from E107787-LIN (static.42.136.251.148.clients.your-server.de. [148.251.136.42]) by smtp.gmail.com with ESMTPSA id s68sm618161ita.29.2017.06.29.08.24.19 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Thu, 29 Jun 2017 08:24:19 -0700 (PDT) From: Yao Qi To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 06/25] Generate c for feature instead of tdesc References: <1497256916-4958-1-git-send-email-yao.qi@linaro.org> <1497256916-4958-7-git-send-email-yao.qi@linaro.org> <3308812a9a2209d76f83656eeccb39bc@polymtl.ca> Date: Thu, 29 Jun 2017 15:24:00 -0000 In-Reply-To: <3308812a9a2209d76f83656eeccb39bc@polymtl.ca> (Simon Marchi's message of "Mon, 26 Jun 2017 23:38:24 +0200") Message-ID: <86injeddgg.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-06/txt/msg00793.txt.bz2 Simon Marchi writes: >> +FEATURE_CFILES =3D $(patsubst %.xml,%.c,$(FEATURE_XMLFILES)) >> + >> +cfiles: $(CFILES) $(FEATURE_CFILES) > > If we are going to have different kinds of CFILES, it would be nice to > rename CFILES (to TDESC_CFILES I suppose) to clarify the purpose of > each variable. > OK, I'll rename CFILES. >> + >> +$(CFILES): %.c: %.xml >> $(GDB) -nx -q -batch \ >> -ex "set tdesc filename $<" -ex 'maint print c-tdesc' > $@.tmp >> sh ../../move-if-change $@.tmp $@ >> >> +$(FEATURE_CFILES): %.c: %.xml.tmp >> + $(GDB) -nx -q -batch \ >> + -ex 'maint print c-tdesc $<' > $@.tmp >> + sh ../../move-if-change $@.tmp $@ >> + rm $< >> + >> +%.xml.tmp: %.xml >> + echo "" > $@ >> + echo "" >> $@ >> + echo "" >> $@ >> + echo " " >> $@ >> + if test $(findstring i386/32bit-,$@); then echo "i386" >> $@ ; fi; > > Should this be > > test -n "$(findstring ...)" > > Quotes would be important here, if findstring returns en empty string, > I update the code here, and find the "test findstring" is no longer needed. >> + echo " " >> $@ >> + echo " " >> $@ >> + echo "" >> $@ >> + > > Instead of creating this temporary file, wouldn't it be simpler to > make "maint print c-tdesc" (or a new "maint print c-feature") take > directly a path to a feature XML, and generate the C from that? Actually, I thought about adding a new command, but I find adding temp file is simpler than modifying GDB to add a new command and accept a feature XML. It is just some lines of code in Makefile, with the "test" "findstring" stuff removed, they are quite simple. >> @@ -2033,38 +2040,123 @@ public: >> printf_unfiltered ("%d, \"%s\");\n", reg->bitsize, reg->type); >> } >> >> +protected: >> + std::string m_filename_after_features; >> + >> private: >> char *m_function; >> - std::string m_filename_after_features; >> bool m_printed_field_type =3D false; >> bool m_printed_type =3D false; >> }; >> >> +class print_c_feature : public print_c_tdesc > > I am really not convinced that such an inheritance is appropriate > here. To make print_c_feature inherit from print_c_tdesc, we should > be able to say that a print_c_feature object _is_ a print_c_tdesc with > further specialization. I don't think it's the case here. If they No, there is no is-a relationship between print_c_feature and print_c_tdesc, so it is inheritance, not subtying. https://en.wikipedia.org/wiki/Inheritance_(object-oriented_programming) "Inheritance should not be confused with subtyping....in general, subtyping establishes an is-a relationship, whereas inheritance only reuses implementation and establishes a syntactic relationship..." and "In object-oriented programming, inheritance is when an object or class is based on another object (prototypal inheritance) or class (class-based inheritance), using the same implementation (...) or specifying a new implementation to maintain the same behavior (...)", > can share some code, I think it would be appropriate for them to have > a common base class though. --=20 Yao (=E9=BD=90=E5=B0=A7)