This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH column number support


Richard Henderson wrote:

On Tue, Dec 02, 2003 at 12:17:48AM -0800, Per Bothner wrote:

Your API still falls into the trap of exposing the low bits as the
column number.

(Actually, it not the low bits, but the low bits after subtracting the current map's start_location.)

Use an API wherein you must use a function to set
the column number and you don't have that problem:

Here's an API that is closer to what you're asking for. I've modified cpplib to use the new api and it builds, but there is a stage3 comparison failure and some regression failures, so it isn't ready yet.

However, my guess is the remaining bugs may be in cpplib
itself, rather than the line-map.[ch], and in any case it's
useful to get feedback on the API.

Some "regressions" are because some error messages now
include a column number where they didn't before.

One implementation detail:  I still use shifts and masks to
separate out the column number.  An alternative might be
to use division ('/' and '%' instead).  E.g. instead of:

(loc - map->start_location) >> map->column_bits

do

(loc - map->start_location) / map->column_limit

where column_limit replaces (1 << column_bits).  Doing a division
makes the code a bit cleaner, plus we aren't limited to powers of 2.

On modern processors my assumption is that division time
is irrelevant - we're limited by memory bandwidth, but on older CPUs
division is expensive.  More importantly if we use a shift, the
column_bits only needs 5-6 bits, but its not obvious how much
space to allocate for column_limit.  16 bits?  32 bits?  So my
inclination is to keep it as a shift count.

But note this is only an implementation detail now; the column_bits
field is not referenced except in line-map.[ch].
--
	--Per Bothner
per@bothner.com   http://per.bothner.com/

Index: line-map.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/line-map.h,v
retrieving revision 1.9.18.4
diff -u -p -r1.9.18.4 line-map.h
--- line-map.h	28 Sep 2003 06:06:29 -0000	1.9.18.4
+++ line-map.h	4 Dec 2003 20:28:16 -0000
@@ -1,5 +1,5 @@
 /* Map logical line numbers to (source file, line number) pairs.
-   Copyright (C) 2001
+   Copyright (C) 2001, 2003
    Free Software Foundation, Inc.
 
 This program is free software; you can redistribute it and/or modify it
@@ -30,27 +30,34 @@ Foundation, 59 Temple Place - Suite 330,
    (e.g. a #line directive in C).  */
 enum lc_reason {LC_ENTER = 0, LC_LEAVE, LC_RENAME};
 
-/* A logical line number, i,e, an "index" into a line_map.  */
+/* A logical line/column number, i,e, an "index" into a line_map.  */
 /* Long-term, we want to use this to replace struct location_s (in input.h),
-   and effectively typedef fileline location_t.  */
-typedef unsigned int fileline;
-
-/* The logical line FROM_LINE maps to physical source file TO_FILE at
-   line TO_LINE, and subsequently one-to-one until the next line_map
-   structure in the set.  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.  */
+   and effectively typedef source_location location_t.  */
+typedef unsigned int source_location;
+typedef source_location fileline; /* deprecated name */
+
+/* Physical source file TO_FILE at line TO_LINE at column 0 is represented
+   by the logical START_LOCATION.  TO_LINE+L at column C is represented by
+   START_LOCATION+(L*(1<<column_bits))+C, as long as C<(1<<column_bits),
+   and the result_location is less than the next line_map's start_location.
+   (The top line is line 1 and the leftmost column is column 1; line/column 0
+   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 line_map
 {
   const char *to_file;
   unsigned int to_line;
-  fileline from_line;
+  source_location start_location;
   int included_from;
   ENUM_BITFIELD (lc_reason) reason : CHAR_BIT;
   unsigned char sysp;
+  /* Number of the low-order source_location bits used for a column number. */
+  unsigned int column_bits : 8;
 };
 
 /* A set of chronological line_map structures.  */
@@ -70,6 +77,11 @@ struct line_maps
 
   /* If true, prints an include trace a la -H.  */
   bool trace_includes;
+
+  /* Highest source_location "given out". */
+  source_location highest_location;
+
+  unsigned int max_column_hint;
 };
 
 /* Initialize a line map set.  */
@@ -78,6 +90,17 @@ extern void linemap_init (struct line_ma
 /* Free a line map set.  */
 extern void linemap_free (struct line_maps *);
 
+extern void linemap_check_files_exited (struct line_maps *);
+
+/* Return a source_location for the start (i.e. column==0) of
+   (physical) line TO_LINE in the current source file (as in the
+   most recent linemap_add).   MAX_COLUMN_HINT is the highest column
+   number we expect to use in this line (but it does not change
+   the highest_location). */
+
+extern source_location linemap_line_start
+(struct line_maps *, unsigned int,  unsigned int);
+
 /* Add a mapping of logical source line to physical source file and
    line number.
 
@@ -87,16 +110,17 @@ extern void linemap_free (struct line_ma
    TO_FILE is NULL, then TO_FILE, TO_LINE and SYSP are given their
    natural values considering the file we are returning to.
 
-   FROM_LINE should be monotonic increasing across calls to this
+   START_LOCATION should be monotonic increasing across calls to this
    function.  A call to this function can relocate the previous set of
    maps, so any stored line_map pointers should not be used.  */
 extern const struct line_map *linemap_add
   (struct line_maps *, enum lc_reason, unsigned int sysp,
-   fileline from_line, const char *to_file, unsigned int to_line);
+   const char *to_file, unsigned int to_line);
 
 /* Given a logical line, returns the map from which the corresponding
    (source file, line) pair can be deduced.  */
-extern const struct line_map *linemap_lookup (struct line_maps *, fileline);
+extern const struct line_map *linemap_lookup
+  (struct line_maps *, source_location);
 
 /* Print the file names and line numbers of the #include commands
    which led to the map MAP, if any, to stderr.  Nothing is output if
@@ -104,12 +128,17 @@ extern const struct line_map *linemap_lo
 extern void linemap_print_containing_files (struct line_maps *,
 					    const struct line_map *);
 
-/* Converts a map and logical line to source line.  */
-#define SOURCE_LINE(MAP, LINE) ((LINE) + (MAP)->to_line - (MAP)->from_line)
+/* Converts a map and a source_location to source line.  */
+#define SOURCE_LINE(MAP, LINE) \
+  ((((LINE) - (MAP)->start_location) >> (MAP)->column_bits) + (MAP)->to_line)
+
+#define SOURCE_COLUMN(MAP, LINE) \
+  (((LINE) - (MAP)->start_location) & ((1 << (MAP)->column_bits) - 1))
 
 /* Returns the last source line within a map.  This is the (last) line
    of the #include, or other directive, that caused a map change.  */
-#define LAST_SOURCE_LINE(MAP) SOURCE_LINE ((MAP), (MAP)[1].from_line - 1)
+#define LAST_SOURCE_LINE(MAP) \
+  SOURCE_LINE ((MAP), (MAP)[1].start_location - (1 << (MAP)->column_bits))
 
 /* Returns the map a given map was included from.  */
 #define INCLUDED_FROM(SET, MAP) (&(SET)->maps[(MAP)->included_from])
@@ -117,8 +146,30 @@ extern void linemap_print_containing_fil
 /* Nonzero if the map is at the bottom of the include stack.  */
 #define MAIN_FILE_P(MAP) ((MAP)->included_from < 0)
 
-/* The current line map.  Saves a call to lookup_line if the caller is
-   sure he is in the scope of the current map.  */
-#define CURRENT_LINE_MAP(MAPS) ((MAPS)->maps + (MAPS)->used - 1)
+/* Get a source position that for the same line as the most recent
+   linemap_line_start, but with the specified TO_COLUMN column number. */
 
+static inline source_location
+linemap_position_for_column (struct line_maps *set, unsigned int to_column)
+{
+  struct line_map *map = &set->maps[set->used - 1];
+  source_location r = set->highest_location;
+  if (__builtin_expect (to_column > set->max_column_hint, 0))
+    {
+      if (r >= 0xC000000 || to_column > 1000000) /* FIXME */
+	{
+	  /* Running low on source_locations - disable column numbers. */
+	  return r - SOURCE_COLUMN (map, r);
+	}
+      else
+	{
+	  r = linemap_line_start (set, SOURCE_LINE (map, r), to_column + 50);
+	}
+    }
+  r = r - SOURCE_COLUMN (map, r) + to_column;
+  if (r >= set->highest_location)
+    set->highest_location = r;
+  return r;
+}
+						  
 #endif /* !GCC_LINE_MAP_H  */
Index: line-map.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/line-map.c,v
retrieving revision 1.6.18.5
diff -u -p -r1.6.18.5 line-map.c
--- line-map.c	20 Aug 2003 20:44:25 -0000	1.6.18.5
+++ line-map.c	4 Dec 2003 20:28:16 -0000
@@ -1,5 +1,5 @@
 /* Map logical line numbers to (source file, line number) pairs.
-   Copyright (C) 2001
+   Copyright (C) 2001, 2003
    Free Software Foundation, Inc.
 
 This program is free software; you can redistribute it and/or modify it
@@ -38,6 +38,20 @@ linemap_init (struct line_maps *set)
   set->last_listed = -1;
   set->trace_includes = false;
   set->depth = 0;
+  set->highest_location = 0;
+  set->max_column_hint = 0;
+}
+
+void
+linemap_check_files_exited (struct line_maps *set)
+{
+  struct line_map *map;
+  /* Depending upon whether we are handling preprocessed input or
+     not, this can be a user error or an ICE.  */
+  for (map = &set->maps[set->used - 1]; ! MAIN_FILE_P (map);
+       map = INCLUDED_FROM (set, map))
+    fprintf (stderr, "line-map.c: file \"%s\" entered but not left\n",
+	     map->to_file);
 }
 
 /* Free a line map set.  */
@@ -47,14 +61,7 @@ linemap_free (struct line_maps *set)
 {
   if (set->maps)
     {
-      struct line_map *map;
-
-      /* Depending upon whether we are handling preprocessed input or
-	 not, this can be a user error or an ICE.  */
-      for (map = CURRENT_LINE_MAP (set); ! MAIN_FILE_P (map);
-	   map = INCLUDED_FROM (set, map))
-	fprintf (stderr, "line-map.c: file \"%s\" entered but not left\n",
-		 map->to_file);
+      linemap_check_files_exited (set);
 
       free (set->maps);
     }
@@ -69,18 +76,16 @@ linemap_free (struct line_maps *set)
    TO_FILE is NULL, then TO_FILE, TO_LINE and SYSP are given their
    natural values considering the file we are returning to.
 
-   FROM_LINE should be monotonic increasing across calls to this
-   function.  A call to this function can relocate the previous set of
+   A call to this function can relocate the previous set of
    maps, so any stored line_map pointers should not be used.  */
 
 const struct line_map *
 linemap_add (struct line_maps *set, enum lc_reason reason,
-	     unsigned int sysp, unsigned int from_line,
-	     const char *to_file, unsigned int to_line)
+	     unsigned int sysp, const char *to_file, unsigned int to_line)
 {
   struct line_map *map;
-
-  if (set->used && from_line < set->maps[set->used - 1].from_line)
+  source_location start_location = set->highest_location + 1;
+  if (set->used && start_location < set->maps[set->used - 1].start_location)
     abort ();
 
   if (set->used == set->allocated)
@@ -131,16 +136,19 @@ linemap_add (struct line_maps *set, enum
       if (error || to_file == NULL)
 	{
 	  to_file = from->to_file;
-	  to_line = LAST_SOURCE_LINE (from) + 1;
+	  to_line = SOURCE_LINE (from, from[1].start_location);
 	  sysp = from->sysp;
 	}
     }
 
   map->reason = reason;
   map->sysp = sysp;
-  map->from_line = from_line;
+  map->start_location = start_location;
   map->to_file = to_file;
   map->to_line = to_line;
+  map->column_bits = 0;
+  set->highest_location = start_location;
+  set->max_column_hint = 0;
 
   if (reason == LC_ENTER)
     {
@@ -160,13 +168,74 @@ linemap_add (struct line_maps *set, enum
   return map;
 }
 
+source_location
+linemap_line_start (struct line_maps *set, unsigned int to_line,
+		    unsigned int max_column_hint)
+{
+  struct line_map *map = &set->maps[set->used - 1];
+  source_location highest = set->highest_location;
+  source_location r;
+  unsigned int last_line = SOURCE_LINE (map, highest);
+  int line_delta = to_line - last_line;
+  bool add_map = false;
+  if (line_delta < 0)
+    {
+      add_map = true;
+    }
+  else if (line_delta > 10 && line_delta * map->column_bits > 1000)
+    {
+      add_map = true;
+    }
+  else if (max_column_hint >= (1U << map->column_bits))
+    {
+      add_map = true;
+    }
+  else if (max_column_hint <= 80 && map->column_bits >= 10)
+    {
+      add_map = true;
+    }
+  else
+    max_column_hint = set->max_column_hint;
+  if (add_map)
+    {
+      int column_bits;
+      if (max_column_hint > 1000000 || highest > 0xC0000000)
+	{
+	  max_column_hint = 0;
+	  if (highest >0xF0000000)
+	    return 0;
+	  column_bits = 0;
+	}
+      else
+	{
+	  column_bits = 7;
+	  while (max_column_hint >= (1U << column_bits))
+	    column_bits++;
+	  max_column_hint = 1U << column_bits;
+	}
+      if (line_delta > 0
+	  || highest - SOURCE_COLUMN (map, highest) != map->start_location)
+	map = (struct line_map*) linemap_add (set, LC_RENAME, map->sysp,
+					      map->to_file, to_line);
+      map->column_bits = column_bits;
+      r = map->start_location;
+    }
+  else
+    r = highest - SOURCE_COLUMN (map, highest)
+      + (line_delta << map->column_bits);
+  if (r > set->highest_location)
+    set->highest_location = r;
+  set->max_column_hint = max_column_hint;
+  return r;
+}
+
 /* Given a logical line, returns the map from which the corresponding
    (source file, line) pair can be deduced.  Since the set is built
    chronologically, the logical lines are monotonic increasing, and so
    the list is sorted and we can use a binary search.  */
 
 const struct line_map *
-linemap_lookup (struct line_maps *set, unsigned int line)
+linemap_lookup (struct line_maps *set, source_location line)
 {
   unsigned int md, mn = 0, mx = set->used;
 
@@ -176,7 +245,7 @@ linemap_lookup (struct line_maps *set, u
   while (mx - mn > 1)
     {
       md = (mn + mx) / 2;
-      if (set->maps[md].from_line > line)
+      if (set->maps[md].start_location > line)
 	mx = md;
       else
 	mn = md;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]