From: Jason Merrill <jason@redhat.com>
To: Dodji Seketeli <dodji@redhat.com>
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 1/7] Linemap infrastructure for virtual locations
Date: Thu, 04 Aug 2011 21:30:00 -0000 [thread overview]
Message-ID: <4E3B0F24.90708@redhat.com> (raw)
In-Reply-To: <m3d3glyz6p.fsf@redhat.com>
On 08/04/2011 11:27 AM, Dodji Seketeli wrote:
> Yes, Tom and I thought about that, and decided that, this could be an
> optimization that could add later when the whole thing is known to
> work.
Makes sense.
> +/* This is the highest possible source location encoded within an
> + ordinary or macro map. */
> +#define MAX_SOURCE_LOCATION 0xF0000000
Why not 0xFFFFFFFF? I'm not sure what the rationale for using that
value here:
> /* If the column number is ridiculous or we've allocated a huge
> number of source_locations, give up on column numbers. */
> max_column_hint = 0;
> - if (highest >0xF0000000)
> + if (highest > MAX_SOURCE_LOCATION)
> return 0;
was, but I would think that we would be fine to use that upper range for
macro maps.
> - size_t to_file_len = strlen (map->to_file);
> + const char *file_path = LOCATION_FILE (src_loc);
> + int sysp;
> + size_t to_file_len = strlen (file_path);
> unsigned char *to_file_quoted =
> (unsigned char *) alloca (to_file_len * 4 + 1);
> unsigned char *p;
>
> - print.src_line = SOURCE_LINE (map, src_loc);
> - print.src_file = map->to_file;
> + print.src_line = LOCATION_LINE (src_loc);
> + print.src_file = LOCATION_FILE (src_loc);
It probably doesn't matter much, but you could just do
print.src_line = file_path;
to avoid an extra expand_location. Or have an expanded_location
variable here like you do in fortran/cpp.c.
> + return ((location - MAP_START_LOCATION (map))
> + >> ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map))
> + + ORDINARY_MAP_STARTING_LINE_NUMBER (map);
This should use SOURCE_LINE.
> + return (location - MAP_START_LOCATION (map))
> + & ((1 << ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map)) - 1);
And SOURCE_COLUMN.
> - means "entire file/line" or "unknown line/column" or "not applicable".)
> - INCLUDED_FROM is an index into the set that gives the line mapping
> - at whose end the current one was included. File(s) at the bottom
> - of the include stack have this set to -1. REASON is the reason for
> - creation of this line map, SYSP is one for a system header, two for
> - a C system header file that therefore needs to be extern "C"
> - protected in C++, and zero otherwise. */
> -struct GTY(()) line_map {
> + means "entire file/line" or "unknown line/column" or "not
> + applicable".)
> +
> + The highest possible source location is MAX_SOURCE_LOCATION
> +*/
Looks like you didn't need to rewrap the "entire file" line. Also, the
trailing */ goes on the last line of text, not on a line by itself.
> - most recent linemap_add). MAX_COLUMN_HINT is the highest column
> + most recent linemap_add). MAX_COLUMN_HINT is the highest column
Unnecessary reformatting.
>
> +bool
> +linemap_tracks_macro_expansion_locs_p (struct line_maps *set)
> +{
> + return LINEMAPS_MACRO_MAPS (set) != NULL;
> +}
> +
> +const struct line_map *
> +linemap_enter_macro (struct line_maps *set, struct cpp_macro *macro,
> + source_location expansion, unsigned int num_tokens)
Two of the several functions without comments. You can just copy the
comments from the header in here.
> +#define linemap_assert(EXPR) \
> + do { \
> + if (! (EXPR)) \
> + abort (); \
> + } while (0)
This should be controlled by ENABLE_CHECKING.
> +int
> +linemap_check_ordinary (const struct line_map *map)
> +{
> + linemap_assert (!linemap_macro_expansion_map_p (map));
> + /* Return any old value. */
> + return 0;
> +}
Why does this return int if we aren't interested in the value? I would
change it to a macro that returns 'map' so that it can expand to nothing
if !ENABLE_CHECKING and can be used in-line like the gcc TREE_CHECK macros.
> +bool
> +linemap_location_from_macro_expansion_p (struct line_maps *set,
> + source_location location)
> +{
> + linemap_assert (location <= MAX_SOURCE_LOCATION);
> + if (set == NULL)
> + return false;
> + return (location > set->highest_location);
> +}
We need to make sure that set->highest_location <
LINEMAPS_MACRO_LOWEST_LOCATION, either here or anywhere that changes one
of those two values.
Jason
next prev parent reply other threads:[~2011-08-04 21:30 UTC|newest]
Thread overview: 134+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-10 11:27 [PATCH 0/6] Tracking locations of tokens resulting from macro expansion Dodji Seketeli
2010-12-10 11:16 ` [PATCH 0/6] *** SUBJECT HERE *** Dodji Seketeli
2010-12-10 12:56 ` Dave Korn
2010-12-10 11:16 ` [PATCH 4/6] Support -fdebug-cpp option Dodji Seketeli
2010-12-10 11:27 ` [PATCH 3/6] Emit macro expansion related diagnostics Dodji Seketeli
2010-12-13 15:25 ` Paolo Bonzini
2010-12-13 15:38 ` Paolo Bonzini
2010-12-13 16:30 ` Manuel López-Ibáñez
2010-12-14 7:24 ` Dodji Seketeli
2010-12-14 7:28 ` Gabriel Dos Reis
2010-12-14 8:40 ` Dodji Seketeli
2010-12-14 9:38 ` Gabriel Dos Reis
2010-12-14 9:42 ` Dodji Seketeli
2010-12-14 9:48 ` Gabriel Dos Reis
2010-12-14 7:28 ` Dodji Seketeli
2010-12-14 8:19 ` Gabriel Dos Reis
2010-12-14 8:31 ` Paolo Bonzini
2010-12-14 9:23 ` Dodji Seketeli
2010-12-10 11:27 ` [PATCH 5/6] Add line map statistics to -fmem-report output Dodji Seketeli
2010-12-21 7:30 ` Gabriel Dos Reis
2011-04-13 20:08 ` Dodji Seketeli
2010-12-10 11:53 ` [PATCH 1/6] Linemap infrastructure for virtual locations Dodji Seketeli
2011-01-06 16:48 ` Tom Tromey
2011-04-12 14:39 ` Dodji Seketeli
2011-04-14 14:46 ` Tom Tromey
2010-12-10 12:27 ` [PATCH 2/6] Generate virtual locations for tokens Dodji Seketeli
2010-12-10 12:33 ` [PATCH 6/6] Kill pedantic warnings on system headers macros Dodji Seketeli
2010-12-10 12:52 ` [PATCH 0/6] Tracking locations of tokens resulting from macro expansion Gabriel Dos Reis
2010-12-10 18:22 ` Dodji Seketeli
2010-12-10 16:59 ` Jeff Law
2010-12-10 19:00 ` Dodji Seketeli
2010-12-13 15:10 ` Jeff Law
2010-12-13 16:35 ` Gabriel Dos Reis
2010-12-14 9:24 ` Dodji Seketeli
2010-12-14 9:40 ` Gabriel Dos Reis
2010-12-14 9:45 ` Dodji Seketeli
2011-07-16 14:38 ` [PATCH 0/7] " Dodji Seketeli
[not found] ` <cover.1310824120.git.dodji@redhat.com>
2011-07-16 14:38 ` [PATCH 3/7] Emit macro expansion related diagnostics Dodji Seketeli
2011-08-04 15:32 ` Dodji Seketeli
2011-09-12 21:54 ` Jason Merrill
2011-09-16 8:19 ` Dodji Seketeli
2011-09-17 21:27 ` Jason Merrill
2011-09-19 14:37 ` Dodji Seketeli
2011-09-19 19:47 ` Jason Merrill
2011-09-19 21:27 ` Jason Merrill
2011-09-20 8:47 ` Dodji Seketeli
2011-09-20 14:12 ` Jason Merrill
2011-09-20 14:12 ` Dodji Seketeli
2011-09-21 2:51 ` Jason Merrill
2011-09-21 19:09 ` Dodji Seketeli
2011-09-22 15:32 ` Jason Merrill
2011-09-26 21:11 ` Dodji Seketeli
2011-09-26 22:30 ` Jason Merrill
2011-09-27 18:43 ` Dodji Seketeli
2011-09-29 7:05 ` Jason Merrill
2011-09-29 19:20 ` Dodji Seketeli
2011-09-29 21:25 ` Jason Merrill
2011-09-29 23:33 ` Dodji Seketeli
2011-09-30 15:56 ` Jason Merrill
2011-09-30 21:04 ` Jason Merrill
2011-10-03 22:50 ` Dodji Seketeli
2011-10-04 19:59 ` Jason Merrill
2011-10-04 20:35 ` Dodji Seketeli
2011-10-03 20:09 ` Dodji Seketeli
2011-10-04 20:03 ` Jason Merrill
2011-10-04 20:28 ` Dodji Seketeli
2011-09-20 0:08 ` Dodji Seketeli
2011-07-16 14:38 ` [PATCH 6/7] Kill pedantic warnings on system headers macros Dodji Seketeli
2011-09-12 22:09 ` Jason Merrill
2011-09-16 11:25 ` Dodji Seketeli
2011-09-17 22:34 ` Jason Merrill
2011-09-18 18:59 ` Dodji Seketeli
2011-07-16 14:39 ` [PATCH 4/7] Support -fdebug-cpp option Dodji Seketeli
2011-08-21 11:02 ` Alexandre Oliva
2011-08-21 11:40 ` Jakub Jelinek
2011-08-22 14:45 ` Tom Tromey
2011-08-22 15:22 ` Jakub Jelinek
2011-08-23 19:52 ` Dodji Seketeli
2011-08-24 15:11 ` Tom Tromey
2011-09-12 22:07 ` Jason Merrill
2011-09-16 8:23 ` Dodji Seketeli
2011-09-17 22:01 ` Jason Merrill
2011-07-16 14:39 ` [PATCH 5/7] Add line map statistics to -fmem-report output Dodji Seketeli
2011-09-12 22:07 ` Jason Merrill
2011-09-16 8:29 ` Dodji Seketeli
2011-09-17 22:05 ` Jason Merrill
2011-09-20 12:10 ` Dodji Seketeli
2011-09-20 14:08 ` Jason Merrill
2011-07-16 15:25 ` [PATCH 2/7] Generate virtual locations for tokens Dodji Seketeli
2011-08-09 15:30 ` Dodji Seketeli
2011-09-12 21:15 ` Jason Merrill
2011-09-14 10:01 ` Dodji Seketeli
2011-09-14 22:56 ` Jason Merrill
2011-09-18 13:44 ` Dodji Seketeli
2011-09-19 22:31 ` Jason Merrill
2011-09-21 14:55 ` Dodji Seketeli
2011-09-22 17:10 ` Jason Merrill
2011-09-26 14:47 ` Dodji Seketeli
2011-09-26 20:39 ` Jason Merrill
2011-09-28 3:23 ` Dodji Seketeli
2011-09-28 14:49 ` Jason Merrill
2011-09-28 21:24 ` Dodji Seketeli
2011-09-28 21:45 ` Jason Merrill
2011-09-29 5:49 ` Dodji Seketeli
2011-07-16 15:28 ` [PATCH 1/7] Linemap infrastructure for virtual locations Dodji Seketeli
2011-07-18 22:06 ` Jason Merrill
2011-07-19 10:47 ` Dodji Seketeli
2011-07-19 17:26 ` Jason Merrill
2011-07-19 18:03 ` Dodji Seketeli
2011-07-19 23:37 ` Jason Merrill
2011-07-30 6:20 ` Jason Merrill
2011-08-01 18:54 ` Dodji Seketeli
2011-08-01 4:42 ` Jason Merrill
2011-08-02 4:48 ` Jason Merrill
2011-08-04 15:28 ` Dodji Seketeli
2011-08-04 21:30 ` Jason Merrill [this message]
2011-08-05 17:12 ` Dodji Seketeli
2011-08-05 17:31 ` Jason Merrill
2011-08-09 14:56 ` Dodji Seketeli
2011-08-19 8:46 ` Jason Merrill
2011-08-19 14:43 ` Tom Tromey
2011-09-01 10:37 ` Dodji Seketeli
2011-09-07 19:26 ` Jason Merrill
2011-09-08 12:41 ` Dodji Seketeli
2011-09-09 7:45 ` Jason Merrill
2011-09-09 8:57 ` Jason Merrill
2011-07-16 15:34 ` [PATCH 7/7] Reduce memory waste due to non-power-of-2 allocs Dodji Seketeli
2011-09-12 22:25 ` Jason Merrill
2011-09-17 13:41 ` Dodji Seketeli
2011-09-17 22:22 ` Jason Merrill
2011-09-18 22:30 ` Dodji Seketeli
2011-09-19 6:51 ` Laurynas Biveinis
2011-07-16 16:47 ` [PATCH 0/7] Tracking locations of tokens resulting from macro expansion Tobias Burnus
2011-07-16 17:57 ` Dodji Seketeli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4E3B0F24.90708@redhat.com \
--to=jason@redhat.com \
--cc=bonzini@gnu.org \
--cc=burnus@net-b.de \
--cc=charlet@act-europe.fr \
--cc=dodji@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=gdr@integrable-solutions.net \
--cc=joseph@codesourcery.com \
--cc=tromey@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).