From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5336 invoked by alias); 13 Feb 2013 20:02:04 -0000 Received: (qmail 5328 invoked by uid 22791); 13 Feb 2013 20:02:03 -0000 X-SWARE-Spam-Status: No, hits=-5.8 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail-ie0-f175.google.com (HELO mail-ie0-f175.google.com) (209.85.223.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 13 Feb 2013 20:01:57 +0000 Received: by mail-ie0-f175.google.com with SMTP id c12so2220691ieb.6 for ; Wed, 13 Feb 2013 12:01:56 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type:x-gm-message-state; bh=jj/2GQNMpy7Anh0gRlNNp2aZsvdMlJVqmSZXJf4zDLw=; b=dENCQ9/QaUn4UUcbCERCSky+DlhAPCTWJelJYCTk+ctJ/rjK+exxTqomMlpvXQUmdC trp+h0pq4ZavFgRXxsj2s03ya4xLiBAGt/EAib7MFxcPHMBGXfraUSi7d/eZCuzRqYQ3 AnNb5zzdi2DcEbKbQ3qCxO1AIDM8NhBhB5s4JmzDC4Re6Fl/4NHdLet3v7HV0kPi7mub I5H6PSEWYiPLQr+hZ9xUkKbcmVbZoZrpfA48zKCJiFZV1oejpktgSwqwAKAM8W+5l5er icR3mVjMHPSFSKE/jV5cyVdor54a+oke62xAr+6cRv/8URgXBoVNJLubduNewiSCMr06 Loxg== MIME-Version: 1.0 X-Received: by 10.50.135.8 with SMTP id po8mr13528426igb.23.1360785716488; Wed, 13 Feb 2013 12:01:56 -0800 (PST) Received: by 10.231.247.66 with HTTP; Wed, 13 Feb 2013 12:01:56 -0800 (PST) In-Reply-To: References: <20130209065835.F0DCF12084A@jade.mtv.corp.google.com> Date: Wed, 13 Feb 2013 20:02:00 -0000 Message-ID: Subject: Re: [cxx-conversion] Add Record Builder Class From: Lawrence Crowl To: Diego Novillo Cc: nathan@codesourcery.com, gcc-patches List Content-Type: text/plain; charset=ISO-8859-1 X-Gm-Message-State: ALoCoQliyb5CLI1f6lb6SoMcKHpss2I/slRH2hR9AoCZ04Pk+2mtj5Rjqgv72GizLY+An3A/TL+ngekNDhp41lZPKQYOURKD7XsuScEjAPp91ul7I88lHjzOGWudR94VWgAri/byjHZyvLwioCDIy+Vv12GqIMqx8AmZHq7Ng95IP44k3AWzahLf2zkwMPB0NydWutqBEfHh X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2013-02/txt/msg00645.txt.bz2 On 2/13/13, Diego Novillo wrote: > On Tue, Feb 12, 2013 at 2:47 PM, Lawrence Crowl wrote: >> @@ -182,24 +163,9 @@ default_emutls_var_init (tree to, tree d >> static tree >> get_emutls_object_type (void) >> { >> - tree type, type_name, field; >> - >> - type = emutls_object_type; >> - if (type) >> - return type; >> - >> - emutls_object_type = type = lang_hooks.types.make_type (RECORD_TYPE); >> - type_name = NULL; >> - field = targetm.emutls.var_fields (type, &type_name); >> - if (!type_name) >> - type_name = get_identifier ("__emutls_object"); >> - type_name = build_decl (UNKNOWN_LOCATION, >> - TYPE_DECL, type_name, type); >> - TYPE_NAME (type) = type_name; >> - TYPE_FIELDS (type) = field; >> - layout_type (type); >> - >> - return type; >> + if (!emutls_object_type) >> + emutls_object_type = targetm.emutls.object_type (); >> + return emutls_object_type; > > Hm, this does not look like an idempotent change. Where did I > get lost? The responsibility for creating the type has moved into the new hook. The old hook only had responsibility for adding fields. And changing the name if it wasn't right. The new structure has a much clearer division of responsibility. > =================================================================== >> --- gcc/coverage.c (revision 195904) >> +++ gcc/coverage.c (working copy) >> @@ -121,8 +121,8 @@ static const char *const ctr_names[GCOV_ >> /* Forward declarations. */ >> static void read_counts_file (void); >> static tree build_var (tree, tree, int); >> -static void build_fn_info_type (tree, unsigned, tree); >> -static void build_info_type (tree, tree); >> +static tree build_fn_info_type (unsigned, tree); >> +static tree build_info_type (record_builder &, tree); > > I don't really like unnecessary forward declarations. But I guess > it's OK, since you are replacing existing ones. Yeah. I figure applying style changes should be a separate patch. >> -static void >> -build_fn_info_type (tree type, unsigned counters, tree gcov_info_type) >> +static tree >> +build_fn_info_type (unsigned counters, tree gcov_info_type) > > So, here you are changing the signature because the caller used > to create an empty record and now it doesn't need to? We are > going to be creating it in the constructor, right? Correct. -- Lawrence Crowl