Linemap force location and remove LINEMAP_POSITION_FOR_COLUMN (issue4801090)

[gchare at google dot com (Gabriel Charette)]

There was a bug where c_finish_options would create some builtins and assign them source_locations in the linemap other than BUILTINS_LOCATION == 1.

Thus, when calling DECL_IS_BUILTIN to know if a decl is a builtin, some of them would return false as they had a source_location other than BUILTINS_LOCATION within the line_map entry that was incorrectly created in c_finish_options.

The problem to fix this is that _cpp_lex_direct just gets the location of the currently lexed token by calling linemap_position_for_column. It has no notion of whether this token is a builtin or whatever else it may be.

My solution is to add a forced_location field to the line_table which when set is returned by default by linemap_position_for_column instead of getting a new loc from the line_table.

Furthermore, this mechanism will allow us to inject locations for a similar problem we have in pph when lexing replayed pre-processor definitions (which as it is now are getting assigned new source_locations which is creating problems for us).

I'm open to other solutions if anyone sees another way to do it. We could also leave linemap_position_for_column as is and have a separate inline function, say linemap_pos_for_col_or_forced, which makes the forced_location check and simply calls linemap_position_for_column when no forced_location.

I also removed LINEMAP_POSITION_FOR_COLUMN, it did the EXACT same thing as linemap_position_for_column, so maintaining both in parallel seems like overkill to me. The only thing I can think of is that it's more optimal as it's inlined (but if that's really needed we can always make linemap_position_for_column an inline function).

Gabriel

2011-08-10  Gabriel Charette  <gchare@google.com>

	* c-opts.c (c_finish_options): Don't create built-in line_table entry;
	instead force BUILTINS_LOCATION when creating builtins.

	* include/line-map.h (struct line_maps): Add field forced_location.
	(LINEMAP_POSITION_FOR_COLUMN): Remove.
	* line-map.c (linemap_init): Init forced_location to 0.
	(linemap_position_for_column): Return forced_location by default if set

diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index 3227f7b..1af8e7b 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -1306,13 +1306,15 @@ c_finish_options (void)
     {
       size_t i;
 
-      cb_file_change (parse_in,
-		      linemap_add (line_table, LC_RENAME, 0,
-				   _("<built-in>"), 0));
+      /* Make sure all of the builtins about to be declared have
+         BUILTINS_LOCATION has their source_location.  */
+      line_table->forced_location = BUILTINS_LOCATION;
 
       cpp_init_builtins (parse_in, flag_hosted);
       c_cpp_builtins (parse_in);
 
+      line_table->forced_location = 0;
+
       /* We're about to send user input to cpplib, so make it warn for
 	 things that we previously (when we sent it internal definitions)
 	 told it to not warn.
diff --git a/gcc/go/gofrontend/lex.cc b/gcc/go/gofrontend/lex.cc
index 9f26911..167c7dd 100644
--- a/gcc/go/gofrontend/lex.cc
+++ b/gcc/go/gofrontend/lex.cc
@@ -518,9 +518,7 @@ Lex::require_line()
 source_location
 Lex::location() const
 {
-  source_location location;
-  LINEMAP_POSITION_FOR_COLUMN(location, line_table, this->lineoff_ + 1);
-  return location;
+  return linemap_position_for_column (line_table, this->lineoff_ + 1);
 }
 
 // Get a location slightly before the current one.  This is used for
@@ -529,9 +527,7 @@ Lex::location() const
 source_location
 Lex::earlier_location(int chars) const
 {
-  source_location location;
-  LINEMAP_POSITION_FOR_COLUMN(location, line_table, this->lineoff_ + 1 - chars);
-  return location;
+  return linemap_position_for_column (line_table, this->lineoff_ + 1 - chars);
 }
 
 // Get the next token.
diff --git a/libcpp/directives-only.c b/libcpp/directives-only.c
index e19f806..c6772af 100644
--- a/libcpp/directives-only.c
+++ b/libcpp/directives-only.c
@@ -142,7 +142,7 @@ _cpp_preprocess_dir_only (cpp_reader *pfile,
 	    flags |= DO_LINE_COMMENT;
 	  else if (!(flags & DO_SPECIAL))
 	    /* Mark the position for possible error reporting. */
-	    LINEMAP_POSITION_FOR_COLUMN (loc, pfile->line_table, col);
+	    loc = linemap_position_for_column (pfile->line_table, col);
 
 	  break;
 
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index f1d5bee..d14528e 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -95,6 +95,11 @@ struct GTY(()) line_maps {
   /* If non-null, the allocator to use when resizing 'maps'.  If null,
      xrealloc is used.  */
   line_map_realloc reallocator;
+
+  /* If non-zero, linemap_position_for_column automatically returns
+     the value stored at this memory location, instead of caclulating
+     a new source_location.  */
+  source_location forced_location;
 };
 
 /* Initialize a line map set.  */
@@ -165,23 +170,6 @@ extern const struct line_map *linemap_lookup
 /* Nonzero if the map is at the bottom of the include stack.  */
 #define MAIN_FILE_P(MAP) ((MAP)->included_from < 0)
 
-/* Set LOC to a source position that is the same line as the most recent
-   linemap_line_start, but with the specified TO_COLUMN column number.  */
-
-#define LINEMAP_POSITION_FOR_COLUMN(LOC, SET, TO_COLUMN) do { \
-  unsigned int to_column = (TO_COLUMN); \
-  struct line_maps *set = (SET); \
-  if (__builtin_expect (to_column >= set->max_column_hint, 0)) \
-    (LOC) = linemap_position_for_column (set, to_column); \
-  else { \
-    source_location r = set->highest_line; \
-    r = r + to_column; \
-    if (r >= set->highest_location) \
-      set->highest_location = r; \
-    (LOC) = r;			 \
-  }} while (0)
-    
-
 extern source_location
 linemap_position_for_column (struct line_maps *set, unsigned int to_column);
 
diff --git a/libcpp/lex.c b/libcpp/lex.c
index d29f36d..d460b98 100644
--- a/libcpp/lex.c
+++ b/libcpp/lex.c
@@ -1975,8 +1975,8 @@ _cpp_lex_direct (cpp_reader *pfile)
     }
   c = *buffer->cur++;
 
-  LINEMAP_POSITION_FOR_COLUMN (result->src_loc, pfile->line_table,
-			       CPP_BUF_COLUMN (buffer, buffer->cur));
+  result->src_loc = linemap_position_for_column (pfile->line_table,
+			                                    CPP_BUF_COLUMN (buffer, buffer->cur));
 
   switch (c)
     {
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index dd3f11c..31ab672 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -41,6 +41,7 @@ linemap_init (struct line_maps *set)
   set->highest_line = RESERVED_LOCATION_COUNT - 1;
   set->max_column_hint = 0;
   set->reallocator = 0;
+  set->forced_location = 0;
 }
 
 /* Check for and warn about line_maps entered but not exited.  */
@@ -245,6 +246,9 @@ linemap_line_start (struct line_maps *set, linenum_type to_line,
 source_location
 linemap_position_for_column (struct line_maps *set, unsigned int to_column)
 {
+  if (set->forced_location)
+    return set->forced_location;
+
   source_location r = set->highest_line;
   if (to_column >= set->max_column_hint)
     {

--
This patch is available for review at http://codereview.appspot.com/4801090