From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30075 invoked by alias); 19 Sep 2011 20:59:20 -0000 Received: (qmail 30067 invoked by uid 22791); 19 Sep 2011 20:59:19 -0000 X-SWARE-Spam-Status: No, hits=-6.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 19 Sep 2011 20:59:06 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p8JKwfkO003323 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 19 Sep 2011 16:58:41 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p8JKweCj004943; Mon, 19 Sep 2011 16:58:41 -0400 Received: from [0.0.0.0] (ovpn-113-145.phx2.redhat.com [10.3.113.145]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id p8JKwcxj000583; Mon, 19 Sep 2011 16:58:39 -0400 Message-ID: <4E77ACFE.2080805@redhat.com> Date: Mon, 19 Sep 2011 22:31:00 -0000 From: Jason Merrill User-Agent: Mozilla/5.0 (X11; Linux i686; rv:6.0.2) Gecko/20110906 Thunderbird/6.0.2 MIME-Version: 1.0 To: Dodji Seketeli CC: gcc-patches@gcc.gnu.org, tromey@redhat.com, gdr@integrable-solutions.net, joseph@codesourcery.com, burnus@net-b.de, charlet@act-europe.fr, bonzini@gnu.org Subject: Re: [PATCH 2/7] Generate virtual locations for tokens References: <1291979498-1604-1-git-send-email-dodji@redhat.com> <4E6E65B2.2060909@redhat.com> <4E711F42.80802@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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: 2011-09/txt/msg01113.txt.bz2 On 09/18/2011 06:47 AM, Dodji Seketeli wrote: > + cpp_hashnode *macro; > + > + }c; Extra blank line, missing space. >>> +/* Appends a token to the end of the token buffer BUFFER. Note that >>> + this function doesn't enlarge BUFFER; it overwrite the last memory >>> + location of BUFFER that holds a token. >> That doesn't sound like appending. > I have changed the name into tokens_buff_add_token and I updated the > comments accordingly. Hmm, it seems that what the callers are looking for is appending, so the name wasn't really the problem (though the new name is fine too). And the new comment: > +/* Adds a token at the end of the tokens contained in BUFFER. Note > + that this function doesn't enlarge BUFFER when the number of tokens > + reaches BUFFER's size; it then overwrites the last memory location > + of BUFFER that holds a token. clarifies the text that was confusing me so that it does sound more like appending except when the buffer is full. But clobbering the last token when the buffer is full sounds like it's unlikely to be what the caller wants; should we abort instead? > + size_t expanded_capacity; /* total size of expanded array. */ > + source_location *virt_locs; /* Where virtual locations for > + unexpanded tokens are stored. */ > + unsigned virt_locs_capacity; /* Total size of virtual locations > + array. */ > + source_location *expanded_virt_locs; /* Where virtual locations for > + expanded tokens are > + stored. */ Why do we need the capacity fields, when we didn't before? > + if (CPP_OPTION (pfile, track_macro_expansion)) > + { > + unsigned int i, count = macro->count; > + const cpp_token *src = macro->exp.tokens; > + const struct line_map *map; > + source_location *virt_locs = NULL; > + _cpp_buff *macro_tokens = > + tokens_buff_new (pfile, count, &virt_locs); This looks a lot like cloning tokens. I thought you were storing virtual locations separately so you wouldn't have to do that? Jason