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 v3 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output


On 11/2/18 5:04 PM, David Malcolm wrote:
> On Thu, 2018-11-01 at 11:56 -0400, Mike Gulick wrote:
>> 2017-10-31  Mike Gulick  <mgulick@mathworks.com>
>>
>> 	PR preprocessor/83173
>> 	* gcc/input.c (dump_location_info): Dump reason and
>> 	included_from fields from line_map_ordinary struct.  Fix
>> 	indentation when location > 5 digits.
>>
>> 	* libcpp/location-example.txt: Update example
>> 	-fdump-internal-locations output.
>> ---
>>  gcc/input.c                 |  49 +++++-
>>  libcpp/location-example.txt | 333 +++++++++++++++++++++-------------
>> --
>>  2 files changed, 241 insertions(+), 141 deletions(-)
> 
> Sorry about the belated response.  This is a nice enhancement; some
> nits below.
> 
>> diff --git a/gcc/input.c b/gcc/input.c
>> index a94a010f353..f938a37f20e 100644
>> --- a/gcc/input.c
>> +++ b/gcc/input.c
>> @@ -1075,6 +1075,17 @@ dump_labelled_location_range (FILE *stream,
>>    fprintf (stream, "\n");
>>  }
>>  
>> +#define NUM_DIGITS(x) ((x) >= 1000000000 ? 10 : \
>> +		       (x) >= 100000000 ? 9 : \
>> +		       (x) >= 10000000 ? 8 : \
>> +		       (x) >= 1000000 ? 7 : \
>> +		       (x) >= 100000 ? 6 : \
>> +		       (x) >= 10000 ? 5 : \
>> +		       (x) >= 1000 ? 4 : \
>> +		       (x) >= 100 ? 3 : \
>> +		       (x) >= 10 ? 2 : \
>> +		       1)
> 
> diagnostic-show-locus.c has a function "num_digits" (currently static)
> and, fwiw, a unit test.  It would be good to share the implementation.
> 

I initially tried to use this function by just adding "extern int
num_digits(int);" into diagnostic-core.h, but that failed to link, so it seems
like diagnostic-show-locus.c is not included in whatever library input.c gets
linked with (I forget which library it was trying to link).  Instead I moved
num_digits and its unit test to diagnostic.c, and added the extern definition to
diagnostic-core.h.  That builds and tests successfully.  Does that seem like a
reasonable way to do this?

>>  /* Write a visualization of the locations in the line_table to
>> STREAM.  */
>>  
>>  void
>> @@ -1104,6 +1115,35 @@ dump_location_info (FILE *stream)
>>  	       map->m_column_and_range_bits - map->m_range_bits);
>>        fprintf (stream, "  range bits: %i\n",
>>  	       map->m_range_bits);
>> +      const char * reason;
>> +      switch (map->reason) {
>> +      case LC_ENTER:
>> +	reason = "LC_ENTER";
>> +	break;
>> +      case LC_LEAVE:
>> +	reason = "LC_LEAVE";
>> +	break;
>> +      case LC_RENAME:
>> +	reason = "LC_RENAME";
>> +	break;
>> +      case LC_RENAME_VERBATIM:
>> +	reason = "LC_RENAME_VERBATIM";
>> +	break;
>> +      case LC_ENTER_MACRO:
>> +	reason = "LC_RENAME_MACRO";
>> +	break;
>> +      default:
>> +	reason = "Unknown";
>> +      }
>> +      fprintf (stream, "  reason: %d (%s)\n", map->reason, reason);
>> +
>> +      const line_map_ordinary *includer_map
>> +	= linemap_included_from_linemap (line_table, map);
>> +      fprintf (stream, "  included from map: %d\n",
>> +	       includer_map ? int (includer_map - line_table-
>>> info_ordinary.maps)
>> +	       : -1);
> 
> I'm not a fan of "-1" here; it's a NULL pointer in the original data.
> How about "n/a" for that case?
> 

That's a good suggestion.  Thanks.

>> +      fprintf (stream, "  included from location: %d\n",
>> +	       linemap_included_from (map));
> 
> ...or merging it with this line, for something like:
> 
>   included from location: 127 (in ordinary map 2)
> 
> vs:
> 
>   included from location: 0
> 
> [...snip...]
> 
> Other than that, this is OK for trunk, assuming your contributor
> paperwork is in place.
> 
> Dave
> 

What is the preferred way to re-send this patch?  Should I re-send the entire
patch series as v4, or just an updated version of this single patch?

Also, I'm waiting on FSF for assignment paperwork.  I've re-pinged them after
waiting a week.

Thanks for the feedback and help.

-Mike

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