On 13 October 2014 10:52, Dodji Seketeli <do...@redhat.com> wrote: > Manuel López-Ibáñez <lopeziba...@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.
