Re: [PATCH] Combine location with block using block_locations

[Dodji Seketeli <dodji at redhat dot com>]

Dehao Chen <dehao@google.com> writes:

> Index: libcpp/line-map.c

[...]

> + /* Data structure to associate an arbitrary data to a source location.  */
> + struct location_adhoc_data {
> +   source_location locus;
> +   void *data;
> + };
> +
> + /* The following data structure encodes a location with some adhoc data,
> +    and map it to a new unsigned integer, and replace it with the original

I think you should remove the words "it with".

> +    location to represent the mapping.

So it should read (so far):

    The following data structure encodes a location with some adhoc data
    and maps it to a new unsigned integer (called an adhoc location)
    that replaces the original location to represent the mapping.

> +
> +    The new adhoc_loc uses the highest bit as the enabling bit, i.e. if the
> +    highest bit is 1, then the number is adhoc_loc. Otherwise, it serves as
> +    the original location. Once identified as the adhoc_loc, the lower 31
> +    bits of the integer is used to index to the location_adhoc_data array,

s/index to/index/

> +    in which the locus and associated data is stored.  */

> + /* Combine LOCUS and DATA to a combined adhoc loc.  */
> +
> + source_location
> + get_combined_adhoc_loc (source_location locus, void *data)
> + {
> +   struct location_adhoc_data lb;
> +   struct location_adhoc_data **slot;
> +
> +   linemap_assert (data);
> +
> +   if (IS_ADHOC_LOC (locus))
> +     locus = location_adhoc_data[locus & MAX_SOURCE_LOCATION].locus;
> +   if (locus == 0 && data == NULL)
> +     return 0;
> +   lb.locus = locus;
> +   lb.data = data;
> +   slot = (struct location_adhoc_data **)
> +       htab_find_slot (location_adhoc_data_htab, &lb, INSERT);
> +   if (*slot == NULL)
> +     {
> +       *slot = location_adhoc_data + curr_adhoc_loc;
> +       location_adhoc_data[curr_adhoc_loc] = lb;
> +       if (++curr_adhoc_loc >= allocated_location_adhoc_data)
> + 	{
> + 	  char *orig_location_adhoc_data = (char *) location_adhoc_data;
> + 	  allocated_location_adhoc_data *= 2;
> + 	  location_adhoc_data = XRESIZEVEC (struct location_adhoc_data,
> + 				 	location_adhoc_data,
> + 					allocated_location_adhoc_data);
> + 	  htab_traverse (location_adhoc_data_htab, location_adhoc_data_update,
> + 			 orig_location_adhoc_data);
> + 	}
> +     }

I am wondering if there isn't an indentation issue here.

> +   return ((*slot) - location_adhoc_data) | 0x80000000;
> + }
> +

Other than that, I don't really have anything worthwhile to say.  I am
deferring to the maintainers now :-)

Thank you for bearing with me.

-- 
		Dodji