On Tue, 2016-07-26 at 19:05 +0100, Manuel López-Ibáñez wrote: > On 26/07/16 18:11, David Malcolm wrote: > > > gcc/ChangeLog: > > * gcc.c (cpp_options): Rename string to... > > (cpp_options_): ...this, to avoid clashing with struct in > > cpplib.h. > > It seems to me that you need this because now gcc.c includes > cpplib.h via > input.h, which seems wrong. > > input.h was FE-independent (it depends on line-map.h but it is an > accident of > history that line-map.h is in libcpp since it doesn't depend on > anything from > libcpp [*]). Note that input.h is included in coretypes.h, so this > means that > now cpplib.h is included almost everywhere! [**] > > There is the following in coretypes.h: > > /* Provide forward struct declaration so that we don't have to > include > all of cpplib.h whenever a random prototype includes a pointer. > Note that the cpp_reader and cpp_token typedefs remain part of > cpplib.h. */ > > struct cpp_reader; > struct cpp_token; > > precisely to avoid including cpplib.h. > > > If I understand correctly, cpplib.h is needed in input.h because of > this > declaration: > > +extern const char *get_source_range_for_substring (cpp_reader > *pfile, > + string_concat_db > *concats, > + location_t > strloc, > + enum cpp_ttype > type, > + int start_idx, > int end_idx, > + source_range > *out_range); > > > Does this really need to be in input.h ? It seems something that > only C-family > languages will be able to use. Note that you need a reader to use > this > function, and for that, you need to already include cpplib.h.
Fair point; the attached modification to patch 1 compiles cleanly, and moves it to a new header. > Perhaps it could live for now in c-format.c, since it is the only > place using it? Martin Sebor [CC-ed] wants to use it from the middle-end: https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01088.html so it's unclear to me that c-format.c would be a better location. There are various places it could live; but getting it working took a lot of effort to achieve - the currently proposed mixture of libcpp, input.c and c-format.c for the locations of the various pieces works (for example, auto_vec isn't available in libcpp). Given that both Martin and I have candidate patches that are touching the same area, I'd prefer to focus on getting this code in to trunk, rather than rewrite it out-of-tree, so that we can at least have the improvement to location-handling for Wformat. Once the code is in the tree, it should be easier to figure out how to access it from the middle-end. > Cheers, > > Manuel. > > [*] In an ideal world, we would have a language-agnostic diagnostics > library > that would include line-map and that would be used by libcpp and the > rest of > GCC, so that we can remove all the error-routines in libcpp and the > awkward > glue code that ties it into diagnostics.c., Agreed, though that may have to wait until gcc 8 at this point. (Given that the proposed diagnostics library would use line maps, and would be used by libcpp, would it make sense to move the diagnostics into libcpp itself? Diagnostics would seem to be intimately related to location-tracking) > [**] And it seems that we are slowly undoing all the work that was > done by > Andrew MacLeod to clean up the .h web and remove dependencies > (https://gcc.gnu.org/wiki/rearch). > >
From 09824cb27c0e817b29de1c7eb9b53c603116f13e Mon Sep 17 00:00:00 2001 From: David Malcolm <dmalc...@redhat.com> Date: Wed, 27 Jul 2016 10:33:52 -0400 Subject: [PATCH] Avoid including cpplib.h from input.h gcc/c-family/ChangeLog: * c-common.c: Include "substring-locations.h". gcc/ChangeLog: * input.h: Don't include cpplib.h. (get_source_range_for_substring): Move to... * substring-locations.h: New header. --- gcc/c-family/c-common.c | 1 + gcc/input.h | 8 -------- gcc/substring-locations.h | 30 ++++++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 8 deletions(-) create mode 100644 gcc/substring-locations.h diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index f4ffc0e..c4843db 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3. If not see #include "tree-iterator.h" #include "opts.h" #include "gimplify.h" +#include "substring-locations.h" cpp_reader *parse_in; /* Declared in c-pragma.h. */ diff --git a/gcc/input.h b/gcc/input.h index 24d9115..c17e440 100644 --- a/gcc/input.h +++ b/gcc/input.h @@ -22,7 +22,6 @@ along with GCC; see the file COPYING3. If not see #define GCC_INPUT_H #include "line-map.h" -#include <cpplib.h> extern GTY(()) struct line_maps *line_table; @@ -131,11 +130,4 @@ class GTY(()) string_concat_db hash_map <location_hash, string_concat *> *m_table; }; -extern const char *get_source_range_for_substring (cpp_reader *pfile, - string_concat_db *concats, - location_t strloc, - enum cpp_ttype type, - int start_idx, int end_idx, - source_range *out_range); - #endif diff --git a/gcc/substring-locations.h b/gcc/substring-locations.h new file mode 100644 index 0000000..274ebbe --- /dev/null +++ b/gcc/substring-locations.h @@ -0,0 +1,30 @@ +/* Source locations within string literals. + Copyright (C) 2016 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify it under +the terms of the GNU General Public License as published by the Free +Software Foundation; either version 3, or (at your option) any later +version. + +GCC is distributed in the hope that it will be useful, but WITHOUT ANY +WARRANTY; without even the implied warranty of MERCHANTABILITY or +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. */ + +#ifndef GCC_SUBSTRING_LOCATIONS_H +#define GCC_SUBSTRING_LOCATIONS_H + +extern const char *get_source_range_for_substring (cpp_reader *pfile, + string_concat_db *concats, + location_t strloc, + enum cpp_ttype type, + int start_idx, int end_idx, + source_range *out_range); + +#endif /* ! GCC_SUBSTRING_LOCATIONS_H */ -- 1.8.5.3
