[PATCH 0/2, libcpp] Fix virtual location expansion and macro map lookup

[PATCH 0/2, libcpp] Fix virtual location expansion and macro map lookup

[Dodji Seketeli]

Hello,

The following two patches fixes virtual locations handling to progress
toward bootstrapping the compiler with CFLAGS=-ftrack-macro-expansion
CXXFLAGS=-ftrack-macro-expansion

After these two patches I'll still need to update many c++ test cases
and probably exercise some dg-prune-fu to cope with the output change
incurred by using that option.  This is because the c++ test suite
inherits the CXXFLAGS used to build the compiler and uses to run the
test cases.

[PATCH 1/2, libcpp] Support expansion of reserved locations wrapped in virtual locations

[Dodji Seketeli]

Hello,

A virtual location can now "wrap" a reserved location.  By reserved
location, I mean a location that is less than RESERVED_LOCATION_COUNT
(i.e UNKNOWN_LOCATION or BUILTINS_LOCATION).

When such a virtual location is passed to linemap_resolve_location,
that function returns the bare reserved location, along with a NULL
map, signifying that the reserved location wasn't encoded by any map.

When such a virtual location is passed to linemap_expand_location_full
then we have a problem because linemap_expand_location is given the
result of linemap_resolve_location with is a reserved location and a
NULL map.  And linemap_expand_location expects neither.  Oops.

Transitively, we have the issue when passing such a virtual location
to expand_location as well.

The patch below teaches linemap_expand_location to expand a reserved
location into a zeroed expanded location, relaxes
linemap_resolve_location to behave when passed a reserved location and
then makes linemap_expand_location_full be amenable to any kind of
location, or so I expect.

It also makes linemap_expand_location_full to return the location it
resolved to.  This helps expand_location delegate the handling of
reserved locations to linemap_expand_location_full while retaining the
ability to choose how to label a given reserved location that has a
GCC-specific meaning, for printing purposes.

Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk.

libcpp/

	* include/line-map.h (linemap_expand_location)
	(linemap_resolve_location): Update comment.
	(linemap_expand_location_full): Add output parameter for resolved
	location.
	* line-map.c (linemap_resolve_location):  Handle reserved
	locations; return a NULL map in those cases.
	(linemap_expand_location): If location is reserved, return a
	zeroed expanded location.  Update comment.
	(linemap_expand_location_full): Take a new output parameter to
	return the resolved location.
	(linemap_dump_location): Handle the fact that
	linemap_resolve_location can return NULL line maps when the
	location resolves to a reserved location.

gcc/
	* input.c (expand_location): Let linemap_expand_location_full do
	the expansion even for reserved locations.  Add a comment.
---
 gcc/input.c               |   23 ++++++-----
 libcpp/include/line-map.h |   17 ++++++--
 libcpp/line-map.c         |   95 +++++++++++++++++++++++++++++++-------------
 3 files changed, 92 insertions(+), 43 deletions(-)

diff --git a/gcc/input.c b/gcc/input.c
index a780f5c..78565d6 100644
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -30,20 +30,23 @@ location_t input_location;
 
 struct line_maps *line_table;
 
+/* Expand the source location LOC into a human readable location.  If
+   LOC resolves to a builtin location, the file name of the readable
+   location is set to the string "<built-in>".  */
+
 expanded_location
 expand_location (source_location loc)
 {
+  source_location resolved_location = 0;
   expanded_location xloc;
-  if (loc <= BUILTINS_LOCATION)
-    {
-      xloc.file = loc == UNKNOWN_LOCATION ? NULL : _("<built-in>");
-      xloc.line = 0;
-      xloc.column = 0;
-      xloc.sysp = 0;
-    }
-  else
-    xloc = linemap_expand_location_full (line_table, loc,
-					 LRK_SPELLING_LOCATION);
+
+  xloc = linemap_expand_location_full (line_table, loc,
+				       LRK_SPELLING_LOCATION,
+				       &resolved_location);
+
+  if (resolved_location <= BUILTINS_LOCATION)
+    xloc.file = resolved_location == UNKNOWN_LOCATION ? NULL : _("<built-in>");
+
   return xloc;
 }
 
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index ef98f59..d5d3831 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -651,7 +651,10 @@ enum location_resolution_kind
    LRK_SPELLING_LOCATION.
 
    If LOC_MAP is not NULL, *LOC_MAP is set to the map encoding the
-   returned location.  */
+   returned location.  Note that if the resturned location wasn't originally
+   encoded by a map, the *MAP is set to NULL.  This can happen if LOC
+   resolves to a location reserved for the client code, like
+   UNKNOWN_LOCATION or BUILTINS_LOCATION in GCC.  */
 
 source_location linemap_resolve_location (struct line_maps *,
 					  source_location loc,
@@ -670,18 +673,22 @@ source_location linemap_unwind_toward_expansion (struct line_maps *,
 						 const struct line_map **loc_map);
 
 /* Expand source code location LOC and return a user readable source
-   code location.  LOC must be a spelling (non-virtual) location.  */
-
+   code location.  LOC must be a spelling (non-virtual) location.  If
+   it's a location < RESERVED_LOCATION_COUNT a zeroed expanded source
+   location is returned.  */
 expanded_location linemap_expand_location (const struct line_map *,
 					   source_location loc);
 
 /* Expand source code location LOC and return a user readable source
    code location.  LOC can be a virtual location.  The LRK parameter
-   is the same as for linemap_resolve_location.  */
+   is the same as for linemap_resolve_location.  If RESOLVED_LOCATION
+   is non-NULL, *RESOLVED_LOCATION is set to the result of
+   linemap_resolve_location.  */
 
 expanded_location linemap_expand_location_full (struct line_maps *,
 						source_location loc,
-						enum location_resolution_kind lrk);
+						enum location_resolution_kind lrk,
+						source_location *resolved_location);
 
 /* Statistics about maps allocation and usage as returned by
    linemap_get_statistics.  */
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index fb3be3a..352d697 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -755,12 +755,12 @@ linemap_location_in_system_header_p (struct line_maps *set,
 {
   const struct line_map *map = NULL;
 
-  if (location < RESERVED_LOCATION_COUNT)
-    return false;
-
   location =
     linemap_resolve_location (set, location, LRK_SPELLING_LOCATION, &map);
 
+  if (location < RESERVED_LOCATION_COUNT)
+    return false;
+
   return LINEMAP_SYSP (map);
 }
 
@@ -1039,7 +1039,10 @@ linemap_macro_loc_to_exp_point (struct line_maps *set,
    LRK_SPELLING_LOCATION.
 
    If MAP is non-NULL, *MAP is set to the map of the resolved
-   location.  */
+   location.  Note that if the resturned location wasn't originally
+   encoded by a map, the *MAP is set to NULL.  This can happen if LOC
+   resolves to a location reserved for the client code, like
+   UNKNOWN_LOCATION or BUILTINS_LOCATION in GCC.  */
 
 source_location
 linemap_resolve_location (struct line_maps *set,
@@ -1047,7 +1050,17 @@ linemap_resolve_location (struct line_maps *set,
 			  enum location_resolution_kind lrk,
 			  const struct line_map **map)
 {
-  linemap_assert (set && loc >= RESERVED_LOCATION_COUNT);
+  linemap_assert (set);
+
+  if (loc < RESERVED_LOCATION_COUNT)
+    {
+      /* A reserved location wasn't encoded in a map.  Let's return a
+	 NULL map here, just like what linemap_ordinary_map_lookup
+	 does.  */
+      if (map)
+	*map = NULL;
+      return loc;
+    }
 
   switch (lrk)
     {
@@ -1101,7 +1114,9 @@ linemap_unwind_toward_expansion (struct line_maps *set,
 }
 
 /* Expand source code location LOC and return a user readable source
-   code location.  LOC must be a spelling (non-virtual) location.  */
+   code location.  LOC must be a spelling (non-virtual) location.  If
+   it's a location < RESERVED_LOCATION_COUNT a zeroed expanded source
+   location is returned.  */
 
 expanded_location
 linemap_expand_location (const struct line_map *map,
@@ -1110,28 +1125,47 @@ linemap_expand_location (const struct line_map *map,
 {
   expanded_location xloc;
 
-  xloc.file = LINEMAP_FILE (map);
-  xloc.line = SOURCE_LINE (map, loc);
-  xloc.column = SOURCE_COLUMN (map, loc);
-  xloc.sysp = LINEMAP_SYSP (map) != 0;
+  memset (&xloc, 0, sizeof (xloc));
+
+  if (loc < RESERVED_LOCATION_COUNT)
+    /* The location for this token wasn't generated from a line
+       map.  It was probably a location for a builtin token,
+       choosen by some client code.  Let's not try to expand the
+       location in that case.  */;
+  else if (map == NULL)
+    /* We shouldn't be getting a NULL map with a location that is not
+       reserved by the client code.  */
+    abort ();
+  else
+    {
+      xloc.file = LINEMAP_FILE (map);
+      xloc.line = SOURCE_LINE (map, loc);
+      xloc.column = SOURCE_COLUMN (map, loc);
+      xloc.sysp = LINEMAP_SYSP (map) != 0;
+    }
 
   return xloc;
 }
 
 /* Expand source code location LOC and return a user readable source
    code location.  LOC can be a virtual location.  The LRK parameter
-   is the same as for linemap_resolve_location.  */
+   is the same as for linemap_resolve_location.  If RESOLVED_LOCATION
+   is non-NULL, *RESOLVED_LOCATION is set to the result of
+   linemap_resolve_location.  */
 
 expanded_location
 linemap_expand_location_full (struct line_maps *set,
 			      source_location loc,
-			      enum location_resolution_kind lrk)
+			      enum location_resolution_kind lrk,
+			      source_location *resolved_location)
 {
   const struct line_map *map;
   expanded_location xloc;
 
   loc = linemap_resolve_location (set, loc, lrk, &map);
   xloc = linemap_expand_location (map, loc);
+  if (resolved_location != NULL)
+    *resolved_location = loc;
   return xloc;
 }
 
@@ -1145,32 +1179,37 @@ linemap_dump_location (struct line_maps *set,
 {
   const struct line_map *map;
   source_location location;
-  const char *path, *from;
-  int l,c,s,e;
+  const char *path = "", *from = "";
+  int l = -1, c = -1, s = -1, e = -1;
 
   if (loc == 0)
     return;
 
   location =
     linemap_resolve_location (set, loc, LRK_MACRO_DEFINITION_LOCATION, &map);
-  path = LINEMAP_FILE (map);
 
-  l = SOURCE_LINE (map, location);
-  c = SOURCE_COLUMN (map, location);
-  s = LINEMAP_SYSP (map) != 0;
-  e = location != loc;
-
-  if (e)
-    from = "N/A";
+  if (map == NULL)
+    /* Only reserved locations can be tolerated in this case.  */
+    linemap_assert (location < RESERVED_LOCATION_COUNT);
   else
-    from = (INCLUDED_FROM (set, map))
-      ? LINEMAP_FILE (INCLUDED_FROM (set, map))
-      : "<NULL>";
+    {
+      path = LINEMAP_FILE (map);
+      l = SOURCE_LINE (map, location);
+      c = SOURCE_COLUMN (map, location);
+      s = LINEMAP_SYSP (map) != 0;
+      e = location != loc;
+      if (e)
+	from = "N/A";
+      else
+	from = (INCLUDED_FROM (set, map))
+	  ? LINEMAP_FILE (INCLUDED_FROM (set, map))
+	  : "<NULL>";
+    }
 
   /* P: path, L: line, C: column, S: in-system-header, M: map address,
-     E: macro expansion?.   */
-  fprintf (stream, "{P:%s;F:%s;L:%d;C:%d;S:%d;M:%p;E:%d,LOC:%d}",
-	   path, from, l, c, s, (void*)map, e, loc);
+     E: macro expansion?, LOC: original location, R: resolved location   */
+  fprintf (stream, "{P:%s;F:%s;L:%d;C:%d;S:%d;M:%p;E:%d,LOC:%d,R:%d}",
+	   path, from, l, c, s, (void*)map, e, loc, location);
 }
 
 /* Compute and return statistics about the memory consumption of some
-- 
1.7.6.4

[PATCH 2/2, libcpp] Fix lookup of macro maps

[Dodji Seketeli]

Hello,

I noticed that linemap_macro_map_lookup can yield the wrong map when
at the end of the lookup, we end up with a pair of maps whose indexes
are (mn,mx) with mx-mn == 1.  In that case, if the map we want is mn,
we'd wrongly yield mx.

Fixed thus, bootstrapped and tested on x86_64-unknown-linux-gnu
against trunk.

	* line-map.c (linemap_macro_map_lookup): Make sure to always pick
	the map with the highest MAP_START_LOCATION.
---
 libcpp/line-map.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 352d697..a94bf0c 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -588,14 +588,19 @@ linemap_macro_map_lookup (struct line_maps *set, source_location line)
       mn = 0;
     }
 
-  do 
+  while (mx - mn > 1)
     {
       md = (mx + mn) / 2;
       if (MAP_START_LOCATION (LINEMAPS_MACRO_MAP_AT (set, md)) > line)
 	mn = md;
       else
 	mx = md;
-    } while (mx - mn > 1);
+    }
+
+  /* There are cases where mx - mn = 1 and where the map we want is
+     mn.  Let's not miss it.  */
+  if (MAP_START_LOCATION (LINEMAPS_MACRO_MAP_AT (set, mn)) >= line)
+      mx = mn;
 
   LINEMAPS_MACRO_CACHE (set) = mx;
   result = LINEMAPS_MACRO_MAP_AT (set, LINEMAPS_MACRO_CACHE (set));
-- 
1.7.6.4

Re: [PATCH 2/2, libcpp] Fix lookup of macro maps

[Dodji Seketeli]

This is the right patch that I bootstrapped and tested.

	* line-map.c (linemap_macro_map_lookup): Make sure to always pick
	the map with the highest MAP_START_LOCATION.
---
 libcpp/line-map.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 352d697..ecfcfd0 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -588,14 +588,19 @@ linemap_macro_map_lookup (struct line_maps *set, source_location line)
       mn = 0;
     }
 
-  do 
+  while (mx - mn > 1)
     {
       md = (mx + mn) / 2;
       if (MAP_START_LOCATION (LINEMAPS_MACRO_MAP_AT (set, md)) > line)
 	mn = md;
       else
 	mx = md;
-    } while (mx - mn > 1);
+    }
+
+  /* There are cases where mx - mn = 1 and where the map we want is
+     mn.  Let's not miss it.  */
+  if (MAP_START_LOCATION (LINEMAPS_MACRO_MAP_AT (set, mn)) <= line)
+      mx = mn;
 
   LINEMAPS_MACRO_CACHE (set) = mx;
   result = LINEMAPS_MACRO_MAP_AT (set, LINEMAPS_MACRO_CACHE (set));
-- 
1.7.6.4


-- 
		Dodji

Re: [PATCH 2/2, libcpp] Fix lookup of macro maps

[Jason Merrill]

On 10/21/2011 07:53 PM, Dodji Seketeli wrote:
> -  do
> +  while (mx - mn>  1)
>       {
>         md = (mx + mn) / 2;
>         if (MAP_START_LOCATION (LINEMAPS_MACRO_MAP_AT (set, md))>  line)
>   	mn = md;
>         else
>   	mx = md;
> -    } while (mx - mn>  1);
> +    }
> +
> +  /* There are cases where mx - mn = 1 and where the map we want is
> +     mn.  Let's not miss it.  */
> +  if (MAP_START_LOCATION (LINEMAPS_MACRO_MAP_AT (set, mn))<= line)
> +      mx = mn;

I think a better fix to your binary search algorithm would be to change

   mn = md;

to be

   mn = md + 1;

since you've eliminated md as a possibility.  And then change the test to

  (mn < mx).

Jason

Re: [PATCH 1/2, libcpp] Support expansion of reserved locations wrapped in virtual locations

[Jason Merrill]

On 10/21/2011 07:37 PM, Dodji Seketeli wrote:
> It also makes linemap_expand_location_full to return the location it
> resolved to.

I think I'd prefer to have expand_location call linemap_resolve_location 
and then linemap_expand_location, and perhaps remove 
linemap_expand_location_full.

Incidentally, I notice that there's no assert to enforce the requirement 
that linemap_expand_location only takes spelling locations.

Jason

Re: [PATCH 1/2, libcpp] Support expansion of reserved locations wrapped in virtual locations

[Dodji Seketeli]

Jason Merrill <jason@redhat.com> writes:

> On 10/21/2011 07:37 PM, Dodji Seketeli wrote:
>> It also makes linemap_expand_location_full to return the location it
>> resolved to.
>
> I think I'd prefer to have expand_location call
> linemap_resolve_location and then linemap_expand_location, and perhaps
> remove linemap_expand_location_full.

OK.

>
> Incidentally, I notice that there's no assert to enforce the
> requirement that linemap_expand_location only takes spelling
> locations.

Done.

Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk.

From: Dodji Seketeli <dodji@redhat.com>
Date: Wed, 19 Oct 2011 15:34:51 +0200
Subject: [PATCH 1/2] Support expansion of reserved locations wrapped in
 virtual locations

libcpp/

	* include/line-map.h (linemap_expand_location): Take a line table
	parameter.  Update comment.
	(linemap_resolve_location): Update comment.
	(linemap_expand_location_full): Remove.
	* line-map.c (linemap_resolve_location):  Handle reserved
	locations; return a NULL map in those cases.
	(linemap_expand_location): If location is reserved, return a
	zeroed expanded location.  Update comment.  Take a line table to
	assert that the function takes non-virtual locations only.
	(linemap_expand_location_full): remove.
	(linemap_dump_location): Handle the fact that
	linemap_resolve_location can return NULL line maps when the
	location resolves to a reserved location.

gcc/
	* input.c (expand_location): Rewrite using
	linemap_resolve_location and linemap_expand_location.  Add a
	comment.
---
 gcc/input.c               |   21 +++++----
 libcpp/include/line-map.h |   21 ++++-----
 libcpp/line-map.c         |  109 +++++++++++++++++++++++++++------------------
 3 files changed, 87 insertions(+), 64 deletions(-)

diff --git a/gcc/input.c b/gcc/input.c
index a780f5c..4077f9e 100644
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -30,20 +30,23 @@ location_t input_location;
 
 struct line_maps *line_table;
 
+/* Expand the source location LOC into a human readable location.  If
+   LOC resolves to a builtin location, the file name of the readable
+   location is set to the string "<built-in>".  */
+
 expanded_location
 expand_location (source_location loc)
 {
   expanded_location xloc;
+  const struct line_map *map;
+
+  loc = linemap_resolve_location (line_table, loc,
+				  LRK_SPELLING_LOCATION, &map);
+  xloc = linemap_expand_location (line_table, map, loc);
+
   if (loc <= BUILTINS_LOCATION)
-    {
-      xloc.file = loc == UNKNOWN_LOCATION ? NULL : _("<built-in>");
-      xloc.line = 0;
-      xloc.column = 0;
-      xloc.sysp = 0;
-    }
-  else
-    xloc = linemap_expand_location_full (line_table, loc,
-					 LRK_SPELLING_LOCATION);
+    xloc.file = loc == UNKNOWN_LOCATION ? NULL : _("<built-in>");
+
   return xloc;
 }
 
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index ef98f59..112bc02 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -651,7 +651,10 @@ enum location_resolution_kind
    LRK_SPELLING_LOCATION.
 
    If LOC_MAP is not NULL, *LOC_MAP is set to the map encoding the
-   returned location.  */
+   returned location.  Note that if the resturned location wasn't originally
+   encoded by a map, the *MAP is set to NULL.  This can happen if LOC
+   resolves to a location reserved for the client code, like
+   UNKNOWN_LOCATION or BUILTINS_LOCATION in GCC.  */
 
 source_location linemap_resolve_location (struct line_maps *,
 					  source_location loc,
@@ -670,19 +673,13 @@ source_location linemap_unwind_toward_expansion (struct line_maps *,
 						 const struct line_map **loc_map);
 
 /* Expand source code location LOC and return a user readable source
-   code location.  LOC must be a spelling (non-virtual) location.  */
-
-expanded_location linemap_expand_location (const struct line_map *,
+   code location.  LOC must be a spelling (non-virtual) location.  If
+   it's a location < RESERVED_LOCATION_COUNT a zeroed expanded source
+   location is returned.  */
+expanded_location linemap_expand_location (struct line_maps *,
+					   const struct line_map *,
 					   source_location loc);
 
-/* Expand source code location LOC and return a user readable source
-   code location.  LOC can be a virtual location.  The LRK parameter
-   is the same as for linemap_resolve_location.  */
-
-expanded_location linemap_expand_location_full (struct line_maps *,
-						source_location loc,
-						enum location_resolution_kind lrk);
-
 /* Statistics about maps allocation and usage as returned by
    linemap_get_statistics.  */
 struct linemap_stats
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index fb3be3a..4af3782 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -755,12 +755,12 @@ linemap_location_in_system_header_p (struct line_maps *set,
 {
   const struct line_map *map = NULL;
 
-  if (location < RESERVED_LOCATION_COUNT)
-    return false;
-
   location =
     linemap_resolve_location (set, location, LRK_SPELLING_LOCATION, &map);
 
+  if (location < RESERVED_LOCATION_COUNT)
+    return false;
+
   return LINEMAP_SYSP (map);
 }
 
@@ -1039,7 +1039,10 @@ linemap_macro_loc_to_exp_point (struct line_maps *set,
    LRK_SPELLING_LOCATION.
 
    If MAP is non-NULL, *MAP is set to the map of the resolved
-   location.  */
+   location.  Note that if the resturned location wasn't originally
+   encoded by a map, the *MAP is set to NULL.  This can happen if LOC
+   resolves to a location reserved for the client code, like
+   UNKNOWN_LOCATION or BUILTINS_LOCATION in GCC.  */
 
 source_location
 linemap_resolve_location (struct line_maps *set,
@@ -1047,7 +1050,15 @@ linemap_resolve_location (struct line_maps *set,
 			  enum location_resolution_kind lrk,
 			  const struct line_map **map)
 {
-  linemap_assert (set && loc >= RESERVED_LOCATION_COUNT);
+  if (loc < RESERVED_LOCATION_COUNT)
+    {
+      /* A reserved location wasn't encoded in a map.  Let's return a
+	 NULL map here, just like what linemap_ordinary_map_lookup
+	 does.  */
+      if (map)
+	*map = NULL;
+      return loc;
+    }
 
   switch (lrk)
     {
@@ -1101,37 +1112,44 @@ linemap_unwind_toward_expansion (struct line_maps *set,
 }
 
 /* Expand source code location LOC and return a user readable source
-   code location.  LOC must be a spelling (non-virtual) location.  */
+   code location.  LOC must be a spelling (non-virtual) location.  If
+   it's a location < RESERVED_LOCATION_COUNT a zeroed expanded source
+   location is returned.  */
 
 expanded_location
-linemap_expand_location (const struct line_map *map,
+linemap_expand_location (struct line_maps *set,
+			 const struct line_map *map,
 			 source_location loc)
 
 {
   expanded_location xloc;
 
-  xloc.file = LINEMAP_FILE (map);
-  xloc.line = SOURCE_LINE (map, loc);
-  xloc.column = SOURCE_COLUMN (map, loc);
-  xloc.sysp = LINEMAP_SYSP (map) != 0;
-
-  return xloc;
-}
-
-/* Expand source code location LOC and return a user readable source
-   code location.  LOC can be a virtual location.  The LRK parameter
-   is the same as for linemap_resolve_location.  */
+  memset (&xloc, 0, sizeof (xloc));
+
+  if (loc < RESERVED_LOCATION_COUNT)
+    /* The location for this token wasn't generated from a line
+       map.  It was probably a location for a builtin token,
+       choosen by some client code.  Let's not try to expand the
+       location in that case.  */;
+  else if (map == NULL)
+    /* We shouldn't be getting a NULL map with a location that is not
+       reserved by the client code.  */
+    abort ();
+  else
+    {
+      /* MAP must be an ordinary map and LOC must be non-virtual,
+	 encoded into this map, obviously; the accessors used on MAP
+	 below ensure it is ordinary.  Let's just assert the
+	 non-virtualness of LOC here.  */
+      if (linemap_location_from_macro_expansion_p (set, loc))
+	abort ();
 
-expanded_location
-linemap_expand_location_full (struct line_maps *set,
-			      source_location loc,
-			      enum location_resolution_kind lrk)
-{
-  const struct line_map *map;
-  expanded_location xloc;
+      xloc.file = LINEMAP_FILE (map);
+      xloc.line = SOURCE_LINE (map, loc);
+      xloc.column = SOURCE_COLUMN (map, loc);
+      xloc.sysp = LINEMAP_SYSP (map) != 0;
+    }
 
-  loc = linemap_resolve_location (set, loc, lrk, &map);
-  xloc = linemap_expand_location (map, loc);
   return xloc;
 }
 
@@ -1145,32 +1163,37 @@ linemap_dump_location (struct line_maps *set,
 {
   const struct line_map *map;
   source_location location;
-  const char *path, *from;
-  int l,c,s,e;
+  const char *path = "", *from = "";
+  int l = -1, c = -1, s = -1, e = -1;
 
   if (loc == 0)
     return;
 
   location =
     linemap_resolve_location (set, loc, LRK_MACRO_DEFINITION_LOCATION, &map);
-  path = LINEMAP_FILE (map);
-
-  l = SOURCE_LINE (map, location);
-  c = SOURCE_COLUMN (map, location);
-  s = LINEMAP_SYSP (map) != 0;
-  e = location != loc;
 
-  if (e)
-    from = "N/A";
+  if (map == NULL)
+    /* Only reserved locations can be tolerated in this case.  */
+    linemap_assert (location < RESERVED_LOCATION_COUNT);
   else
-    from = (INCLUDED_FROM (set, map))
-      ? LINEMAP_FILE (INCLUDED_FROM (set, map))
-      : "<NULL>";
+    {
+      path = LINEMAP_FILE (map);
+      l = SOURCE_LINE (map, location);
+      c = SOURCE_COLUMN (map, location);
+      s = LINEMAP_SYSP (map) != 0;
+      e = location != loc;
+      if (e)
+	from = "N/A";
+      else
+	from = (INCLUDED_FROM (set, map))
+	  ? LINEMAP_FILE (INCLUDED_FROM (set, map))
+	  : "<NULL>";
+    }
 
   /* P: path, L: line, C: column, S: in-system-header, M: map address,
-     E: macro expansion?.   */
-  fprintf (stream, "{P:%s;F:%s;L:%d;C:%d;S:%d;M:%p;E:%d,LOC:%d}",
-	   path, from, l, c, s, (void*)map, e, loc);
+     E: macro expansion?, LOC: original location, R: resolved location   */
+  fprintf (stream, "{P:%s;F:%s;L:%d;C:%d;S:%d;M:%p;E:%d,LOC:%d,R:%d}",
+	   path, from, l, c, s, (void*)map, e, loc, location);
 }
 
 /* Compute and return statistics about the memory consumption of some
-- 
1.7.6.4


-- 
		Dodji

Re: [PATCH 2/2, libcpp] Fix lookup of macro maps

[Dodji Seketeli]

Jason Merrill <jason@redhat.com> writes:


> I think a better fix to your binary search algorithm would be to change
>
>   mn = md;
>
> to be
>
>   mn = md + 1;
>
> since you've eliminated md as a possibility.  And then change the test to
>
>  (mn < mx).
>

Right, thanks.

Here the updated patch, bootstrapped and tested on
x86_64-unknown-linux-gnu against trunk.

From: Dodji Seketeli <dodji@redhat.com>
Date: Fri, 21 Oct 2011 16:47:07 +0200
Subject: [PATCH 2/2] Fix lookup of macro maps

	* line-map.c (linemap_macro_map_lookup): Fix logic.
---
 libcpp/line-map.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 4af3782..97075e1 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -588,14 +588,14 @@ linemap_macro_map_lookup (struct line_maps *set, source_location line)
       mn = 0;
     }
 
-  do 
+  while (mn < mx)
     {
       md = (mx + mn) / 2;
       if (MAP_START_LOCATION (LINEMAPS_MACRO_MAP_AT (set, md)) > line)
-	mn = md;
+	mn = md + 1;
       else
 	mx = md;
-    } while (mx - mn > 1);
+    }
 
   LINEMAPS_MACRO_CACHE (set) = mx;
   result = LINEMAPS_MACRO_MAP_AT (set, LINEMAPS_MACRO_CACHE (set));
-- 
1.7.6.4


-- 
		Dodji

Re: [PATCH 1/2, libcpp] Support expansion of reserved locations wrapped in virtual locations

[Jason Merrill]

OK.

Jason

Re: [PATCH 2/2, libcpp] Fix lookup of macro maps

[Jason Merrill]

OK.

Jason