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] cleanups in line-map


On 13 October 2014 10:52, Dodji Seketeli <dodji@redhat.com> wrote:
> Manuel López-Ibáñez <lopezibanez@gmail.com> writes:
>
>> Index: libcpp/line-map.c
>> ===================================================================
>> --- libcpp/line-map.c (revision 216098)
>> +++ libcpp/line-map.c (working copy)
>> @@ -29,12 +29,10 @@ along with this program; see the file CO
>>  static void trace_include (const struct line_maps *, const struct line_map *);
>>  static const struct line_map * linemap_ordinary_map_lookup (struct line_maps *,
>>                                                           source_location);
>>  static const struct line_map* linemap_macro_map_lookup (struct line_maps *,
>>                                                       source_location);
>> -static source_location linemap_macro_map_loc_to_def_point
>> -(const struct line_map*, source_location);
>
> This is not redundant per se, is it?  It's just a forward declaration of
> the function that is defined later.  Just like for
> linemap_macro_map_loc_unwind_toward_spelling() below.  Or what am I
> missing?  I'd prefer to see this forward declaration stay, FWIW.

Oh, well, I guess it is a matter of taste. I was annoyed by having to
update two different places (I added "const" to the first argument of
this definition function, so I will have to also add it here).
Moreover, as the patch shows, the two declarations might be different
(one was static, the other not), and then which one is the "correct
one" requires some expert knowledge of C++. I understand using forward
declarations when otherwise it would be a mess to re-order the
functions, but in this case, it is not really necessary. But I can
leave it and just update the argument type.

> Otherwise, this cleanup patch looks good to me.  If it was my call, I'd
> say "OK with that change".
>
> Thank you for tackling this.

Thanks for the review.

Cheers,

Manuel.


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