From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24201 invoked by alias); 6 Jan 2011 16:25:04 -0000 Received: (qmail 24176 invoked by uid 22791); 6 Jan 2011 16:25:02 -0000 X-SWARE-Spam-Status: No, hits=-6.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD 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; Thu, 06 Jan 2011 16:24:50 +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.13.8/8.13.8) with ESMTP id p06GOfvl005205 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 6 Jan 2011 11:24:41 -0500 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 p06GOe5V005181; Thu, 6 Jan 2011 11:24:41 -0500 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id p06GOd1l009532; Thu, 6 Jan 2011 11:24:40 -0500 Received: by opsy.redhat.com (Postfix, from userid 500) id A919937848D; Thu, 6 Jan 2011 09:24:39 -0700 (MST) From: Tom Tromey To: Dodji Seketeli Cc: gcc-patches@gcc.gnu.org, joseph@codesourcery.com, gdr@integrable-solutions.net, lopezibanez@gmail.com Subject: Re: [PATCH 1/6] Linemap infrastructure for virtual locations References: <1291979498-1604-1-git-send-email-dodji@redhat.com> <1291979498-1604-3-git-send-email-dodji@redhat.com> Date: Thu, 06 Jan 2011 16:48:00 -0000 In-Reply-To: <1291979498-1604-3-git-send-email-dodji@redhat.com> (Dodji Seketeli's message of "Fri, 10 Dec 2010 12:11:33 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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-01/txt/msg00305.txt.bz2 >>>>> "Dodji" == Dodji Seketeli writes: Dodji> This is the first instalment of a set which goal is to track locations Dodji> of tokens across macro expansions. Tom Tromey did the original work Dodji> and attached the patch to PR preprocessor/7263. This opus is a Dodji> derivative of that original work. Thank you very much for doing this. I think the accessor macros and changes to use them could go in as a separate patch. It doesn't matter too much now, but I think this would have made this patch more readable. Dodji> expanded_location xloc; Dodji> if (loc <= BUILTINS_LOCATION) Dodji> - { Dodji> - xloc.file = loc == UNKNOWN_LOCATION ? NULL : _(""); Dodji> + { Dodji> + /* If LOC is UNKNOWN_LOCATION and there was a map allocated for Dodji> + it then XLOC.FILE should be the file path of the map, Dodji> + i.e. the file path of the main file being compiled. Dodji> + Otherwise [if we are being called before any line map has Dodji> + been allocated, e.g. during parsing of commande line Dodji> + options] if no line map has been allocated yet, then Dodji> + XLOC.FILE should just be NULL. */ Dodji> + if (loc == UNKNOWN_LOCATION) Dodji> + { Dodji> + const struct line_map *map = Dodji> + linemap_lookup (line_table, UNKNOWN_LOCATION); Dodji> + xloc.file = map != NULL ? LINEMAP_FILE (map) : NULL; Dodji> + } I don't understand this. I thought that UNKNOWN_LOCATION should never have an associated file. This is handled in a funny way: each front end is responsible for ensuring that this holds true (this could be cleaned up...). Am I mistaken about this? Or is something weird happening after this patch? Dodji> @@ -883,14 +884,14 @@ static void Dodji> do_line (cpp_reader *pfile) Dodji> { Dodji> const struct line_maps *line_table = pfile->line_table; Dodji> - const struct line_map *map = &line_table->maps[line_table->used - 1]; Dodji> + const struct line_map *map = LINEMAPS_LAST_ORDINARY_MAP (line_table); Dodji> /* skip_rest_of_line() may cause line table to be realloc()ed so note down Dodji> sysp right now. */ Dodji> - unsigned char map_sysp = map->sysp; Dodji> + unsigned char map_sysp = map->d.ordinary.sysp; Dodji> const cpp_token *token; Dodji> - const char *new_file = map->to_file; Dodji> + const char *new_file = map->d.ordinary.to_file; Did you want to use the accessor macros here? If not, why not? Dodji> /* C99 raised the minimum limit on #line numbers. */ Dodji> @@ -945,11 +946,11 @@ static void Dodji> do_linemarker (cpp_reader *pfile) Dodji> { Dodji> const struct line_maps *line_table = pfile->line_table; Dodji> - const struct line_map *map = &line_table->maps[line_table->used - 1]; Dodji> + const struct line_map *map = LINEMAPS_LAST_ORDINARY_MAP (line_table); Dodji> const cpp_token *token; Dodji> - const char *new_file = map->to_file; Dodji> + const char *new_file = map->d.ordinary.to_file; Dodji> linenum_type new_lineno; Dodji> - unsigned int new_sysp = map->sysp; Dodji> + unsigned int new_sysp = map->d.ordinary.sysp; Likewise. Dodji> if (map != NULL) Dodji> - linemap_line_start (pfile->line_table, map->to_line, 127); Dodji> + linemap_line_start (pfile->line_table, map->d.ordinary.to_line, 127); Likewise. Dodji> + int sysp = (LINEMAPS_ORDINARY_HIGHEST_LINE (pfile->line_table) > 1) Dodji> + && (pfile->buffer ? pfile->buffer->sysp : 0); The indentation is off here. Put another paren after the '=' and emacs will DTRT. Dodji> + physicall source locations are called "spelling locations". Typo, should be "physical". Dodji> +/* A set of chronological line_map structures. */ Dodji> +struct GTY(()) line_maps { Dodji> + Dodji> + struct maps_info info_ordinary; Dodji> + Dodji> + struct maps_info info_macro; One thing to note is that there may be some code that assumes an ordering of location values. If you hand out ordinary and macro locations separately (which I think is what is going on), then code doing this may break. My recollection is that maybe the diagnostic code did this? Or maybe it was someplace more obscure :-( Did you happen to run across this while working on this patch? Dodji> - fname = pfile->line_table->maps[pfile->line_table->used-1].to_file; Dodji> + fname = (LINEMAPS_LAST_ORDINARY_MAP (pfile->line_table))->d.ordinary.to_file; Accessor? Dodji> +/* Add a mapping of logical source line to physical source file and Dodji> + line number. This function creates an "ordinary map", which is a Dodji> + map that records locations of tokens that are not part of macro Dodji> + replacement-lists present at a macro expansion point. Dodji> + Dodji> + The text pointed to by TO_FILE must have a lifetime Dodji> + at least as long as the lifetime of SET. An empty Dodji> + TO_FILE means standard input. If reason is LC_LEAVE, and Dodji> + TO_FILE is NULL, then TO_FILE, TO_LINE and SYSP are given their Dodji> + natural values considering the file we are returning to. Dodji> + Dodji> + A call to this function can relocate the previous set of Dodji> + maps, so any stored line_map pointers should not be used. */ Dodji> + Dodji> +const struct line_map * Dodji> +linemap_add (struct line_maps *set, enum lc_reason reason, Dodji> + unsigned int sysp, const char *to_file, linenum_type to_line) I would generally prefer for explanatory comments for public functions to be in the header, not by the implementation. This is more in keeping with the current style of the line map code, but I also like it because it provides a single text for users of the library to read. Nice job on the comments, btw. Tom