[PATCH 0/4] libcpp patches

[PATCH 0/4] libcpp patches

[David Malcolm]

The following patches for libcpp (and gcc) are an attempt at making
the libcpp API and internal representation easier to understand
and to hack on, simplifying it (IMHO) with a modest amount of C++,
without changing its behavior.

See the individual patches for their descriptions.

How do these look? (for trunk)

I've successfully bootstrapped&regrtested the cumulative effect of the
patches on x86_64-unknown-linux-gnu (Fedora 20).  I would verify the
bootstrap&regrtest of each patch before applying them.

I didn't do any performance testing; what would be appropriate?

David Malcolm (4):
  libcpp: Improvements to comments in line-map.h/c
  libcpp: Replace macro usage with C++ constructs
  libcpp/input.c: Add a way to visualize the linemaps
  Replace line_map union with C++ class hierarchy

 gcc/ada/gcc-interface/trans.c |   2 +-
 gcc/c-family/c-common.h       |   4 +-
 gcc/c-family/c-lex.c          |   6 +-
 gcc/c-family/c-opts.c         |  14 +-
 gcc/c-family/c-ppoutput.c     |   6 +-
 gcc/common.opt                |   4 +
 gcc/diagnostic.c              |   2 +-
 gcc/fortran/cpp.c             |  12 +-
 gcc/genmatch.c                |   4 +-
 gcc/input.c                   | 244 +++++++++++++-
 gcc/input.h                   |   2 +
 gcc/toplev.c                  |   3 +
 gcc/tree-diagnostic.c         |   9 +-
 libcpp/directives.c           |  17 +-
 libcpp/files.c                |   2 +-
 libcpp/include/cpplib.h       |   2 +-
 libcpp/include/line-map.h     | 735 +++++++++++++++++++++++++++++++-----------
 libcpp/internal.h             |  12 +-
 libcpp/line-map.c             | 243 ++++++++------
 libcpp/location-example.txt   | 216 +++++++++++++
 libcpp/macro.c                |  18 +-
 21 files changed, 1204 insertions(+), 353 deletions(-)
 create mode 100644 libcpp/location-example.txt

-- 
1.8.5.3

[PATCH 2/4] libcpp: Replace macro usage with C++ constructs

[David Malcolm]

libcpp makes extensive use of the C preprocessor.  Whilst this has a
pleasingly self-referential quality, I find the code hard-to-read;
implementing source location support in my JIT branch was much harder than
I felt it should have been.

In an attempt at making the code easier to follow, and to build towards
a followup patch, this patch converts most of these macros to C++
equivalents: using "const" for compile-time constants, and inline
functions where macros aren't used as lvalues.

This effectively documents the expected types of the params, and makes
them available from the debugger e.g.:

  (gdb) p LINEMAP_FILE ($3)
  $1 = 0x13b8b37 "<command-line>"

and indeed the constants also:

  (gdb) p IS_ADHOC_LOC(MAX_SOURCE_LOCATION)
  $2 = false
  (gdb) p IS_ADHOC_LOC(MAX_SOURCE_LOCATION + 1)
  $3 = true

[I didn't mark the inline functions as "static"; should they be?]

[FWIW, I posted a reduced version of this patch about a year ago as:
  https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01092.html
which covered a smaller subset of the macros].

libcpp/ChangeLog:
	* include/line-map.h (MAX_SOURCE_LOCATION): Convert from a macro
	to a const source_location.
	(RESERVED_LOCATION_COUNT): Likewise.
	(linemap_check_ordinary): Convert from a macro to a pair of inline
	functions, for const/non-const arguments.
	(MAP_START_LOCATION): Likewise.
	(ORDINARY_MAP_STARTING_LINE_NUMBER): Likewise.
	(ORDINARY_MAP_INCLUDER_FILE_INDEX): Likewise.
	(ORDINARY_MAP_IN_SYSTEM_HEADER_P): Likewise.
	(ORDINARY_MAP_NUMBER_OF_COLUMN_BITS): Convert from a macro to a
	pair of inline functions, for const/non-const arguments, where the
	latter is named...
	(SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS): New function.
	(ORDINARY_MAP_FILE_NAME): Convert from a macro to a pair of inline
	functions, for const/non-const arguments.
	(MACRO_MAP_MACRO): Likewise.
	(MACRO_MAP_NUM_MACRO_TOKENS): Likewise.
	(MACRO_MAP_LOCATIONS): Likewise.
	(MACRO_MAP_EXPANSION_POINT_LOCATION): Likewise.
	(LINEMAPS_MAP_INFO): Likewise.
	(LINEMAPS_MAPS): Likewise.
	(LINEMAPS_ALLOCATED): Likewise.
	(LINEMAPS_USED): Likewise.
	(LINEMAPS_CACHE): Likewise.
	(LINEMAPS_ORDINARY_CACHE): Likewise.
	(LINEMAPS_MACRO_CACHE): Likewise.
	(LINEMAPS_MAP_AT): Convert from a macro to an inline function.
	(LINEMAPS_LAST_MAP): Likewise.
	(LINEMAPS_LAST_ALLOCATED_MAP): Likewise.
	(LINEMAPS_ORDINARY_MAPS): Likewise.
	(LINEMAPS_ORDINARY_MAP_AT): Likewise.
	(LINEMAPS_ORDINARY_ALLOCATED): Likewise.
	(LINEMAPS_ORDINARY_USED): Likewise.
	(LINEMAPS_LAST_ORDINARY_MAP): Likewise.
	(LINEMAPS_LAST_ALLOCATED_ORDINARY_MAP): Likewise.
	(LINEMAPS_MACRO_MAPS): Likewise.
	(LINEMAPS_MACRO_MAP_AT): Likewise.
	(LINEMAPS_MACRO_ALLOCATED): Likewise.
	(LINEMAPS_MACRO_USED): Likewise.
	(LINEMAPS_MACRO_LOWEST_LOCATION): Likewise.
	(LINEMAPS_LAST_MACRO_MAP): Likewise.
	(LINEMAPS_LAST_ALLOCATED_MACRO_MAP): Likewise.
	(IS_ADHOC_LOC): Likewise.
	(COMBINE_LOCATION_DATA): Likewise.
	(SOURCE_LINE): Likewise.
	(SOURCE_COLUMN): Likewise.
	(LAST_SOURCE_LINE_LOCATION): Likewise.
	(LAST_SOURCE_LINE): Likewise.
	(LAST_SOURCE_COLUMN): Likewise.
	(LAST_SOURCE_LINE_LOCATION)
	(INCLUDED_FROM): Likewise.
	(MAIN_FILE_P): Likewise.
	(LINEMAP_FILE): Likewise.
	(LINEMAP_LINE): Likewise.
	(LINEMAP_SYSP): Likewise.
	(linemap_location_before_p): Likewise.
	* line-map.c (linemap_check_files_exited): Make local "map" const.
	(linemap_add): Use SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS.
	(linemap_line_start): Likewise.
---
 libcpp/include/line-map.h | 533 ++++++++++++++++++++++++++++++++++------------
 libcpp/line-map.c         |   6 +-
 2 files changed, 397 insertions(+), 142 deletions(-)

diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index e1681ea..021ca6e 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -155,7 +155,7 @@ struct GTY(()) line_map_ordinary {
 
 /* This is the highest possible source location encoded within an
    ordinary or macro map.  */
-#define MAX_SOURCE_LOCATION 0x7FFFFFFF
+const source_location MAX_SOURCE_LOCATION = 0x7FFFFFFF;
 
 struct cpp_hashnode;
 
@@ -279,30 +279,168 @@ struct GTY(()) line_map {
   } GTY((desc ("%1.reason == LC_ENTER_MACRO"))) d;
 };
 
-#define MAP_START_LOCATION(MAP) (MAP)->start_location
+#if defined ENABLE_CHECKING && (GCC_VERSION >= 2007)
+
+/* Assertion macro to be used in line-map code.  */
+#define linemap_assert(EXPR)                  \
+  do {                                                \
+    if (! (EXPR))                             \
+      abort ();                                       \
+  } while (0)
+
+/* Assert that becomes a conditional expression when checking is disabled at
+   compilation time.  Use this for conditions that should not happen but if
+   they happen, it is better to handle them gracefully rather than crash
+   randomly later.
+   Usage:
+
+   if (linemap_assert_fails(EXPR)) handle_error(); */
+#define linemap_assert_fails(EXPR) __extension__ \
+  ({linemap_assert (EXPR); false;})
+
+#else
+/* Include EXPR, so that unused variable warnings do not occur.  */
+#define linemap_assert(EXPR) ((void)(0 && (EXPR)))
+#define linemap_assert_fails(EXPR) (! (EXPR))
+#endif
 
-#define ORDINARY_MAP_FILE_NAME(MAP) \
-  linemap_check_ordinary (MAP)->d.ordinary.to_file
+/* Return TRUE if MAP encodes locations coming from a macro
+   replacement-list at macro expansion point.  */
+bool linemap_macro_expansion_map_p (const struct line_map *);
+
+/* Assert that MAP encodes locations of tokens that are not part of
+   the replacement-list of a macro expansion.  */
+inline struct line_map *linemap_check_ordinary (struct line_map *map)
+{
+  linemap_assert (!linemap_macro_expansion_map_p (map));
+  return map;
+}
+
+inline const struct line_map *
+linemap_check_ordinary (const struct line_map *map)
+{
+  linemap_assert (!linemap_macro_expansion_map_p (map));
+  return map;
+}
 
-#define ORDINARY_MAP_STARTING_LINE_NUMBER(MAP) \
-  linemap_check_ordinary (MAP)->d.ordinary.to_line
+inline source_location
+MAP_START_LOCATION (const line_map *map)
+{
+  return map->start_location;
+}
 
-#define ORDINARY_MAP_INCLUDER_FILE_INDEX(MAP) \
-  linemap_check_ordinary (MAP)->d.ordinary.included_from
+inline source_location&
+MAP_START_LOCATION (line_map *map)
+{
+  return map->start_location;
+}
 
-#define ORDINARY_MAP_IN_SYSTEM_HEADER_P(MAP) \
-  linemap_check_ordinary (MAP)->d.ordinary.sysp
+inline linenum_type
+ORDINARY_MAP_STARTING_LINE_NUMBER (const line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.to_line;
+}
 
-#define ORDINARY_MAP_NUMBER_OF_COLUMN_BITS(MAP) \
-  linemap_check_ordinary (MAP)->d.ordinary.column_bits
+inline linenum_type&
+ORDINARY_MAP_STARTING_LINE_NUMBER (line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.to_line;
+}
 
-#define MACRO_MAP_MACRO(MAP) (MAP)->d.macro.macro
+inline int
+ORDINARY_MAP_INCLUDER_FILE_INDEX (const line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.included_from;
+}
 
-#define MACRO_MAP_NUM_MACRO_TOKENS(MAP) (MAP)->d.macro.n_tokens
+inline int&
+ORDINARY_MAP_INCLUDER_FILE_INDEX (line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.included_from;
+}
 
-#define MACRO_MAP_LOCATIONS(MAP) (MAP)->d.macro.macro_locations
+inline unsigned char
+ORDINARY_MAP_IN_SYSTEM_HEADER_P (const line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.sysp;
+}
 
-#define MACRO_MAP_EXPANSION_POINT_LOCATION(MAP) (MAP)->d.macro.expansion
+inline unsigned char &
+ORDINARY_MAP_IN_SYSTEM_HEADER_P (line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.sysp;
+}
+
+inline unsigned char
+ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (const line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.column_bits;
+}
+inline void
+SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (line_map *map, int col_bits)
+{
+  linemap_check_ordinary (map)->d.ordinary.column_bits = col_bits;
+}
+
+inline const char *
+ORDINARY_MAP_FILE_NAME (const line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.to_file;
+}
+
+inline const char * &
+ORDINARY_MAP_FILE_NAME (line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.to_file;
+}
+
+inline cpp_hashnode *
+MACRO_MAP_MACRO (const line_map *map)
+{
+  return map->d.macro.macro;
+}
+
+inline cpp_hashnode * &
+MACRO_MAP_MACRO (line_map *map)
+{
+  return map->d.macro.macro;
+}
+
+inline unsigned int
+MACRO_MAP_NUM_MACRO_TOKENS (const line_map *map)
+{
+  return map->d.macro.n_tokens;
+}
+
+inline unsigned int &
+MACRO_MAP_NUM_MACRO_TOKENS (line_map *map)
+{
+  return map->d.macro.n_tokens;
+}
+
+inline source_location *
+MACRO_MAP_LOCATIONS (const line_map *map)
+{
+  return map->d.macro.macro_locations;
+}
+
+inline source_location * &
+MACRO_MAP_LOCATIONS (line_map *map)
+{
+  return map->d.macro.macro_locations;
+}
+
+inline source_location
+MACRO_MAP_EXPANSION_POINT_LOCATION (const line_map *map)
+{
+  return map->d.macro.expansion;
+}
+
+inline source_location &
+MACRO_MAP_EXPANSION_POINT_LOCATION (line_map *map)
+{
+  return map->d.macro.expansion;
+}
 
 /* The abstraction of a set of location maps. There can be several
    types of location maps. This abstraction contains the attributes
@@ -391,119 +529,233 @@ struct GTY(()) line_maps {
 /* Returns the pointer to the memory region where information about
    maps are stored in the line table SET. MACRO_MAP_P is a flag
    telling if we want macro or ordinary maps.  */
-#define LINEMAPS_MAP_INFO(SET, MACRO_MAP_P)				\
-  ((MACRO_MAP_P)							\
-   ? &((SET)->info_macro)						\
-   : &((SET)->info_ordinary))
+inline struct maps_info *
+LINEMAPS_MAP_INFO (line_maps *set, bool macro_map_p)
+{
+  return (macro_map_p
+	  ? &(set->info_macro)
+	  : &(set->info_ordinary));
+}
+
+inline const struct maps_info *
+LINEMAPS_MAP_INFO (const line_maps *set, bool macro_map_p)
+{
+  return (macro_map_p
+	  ? &(set->info_macro)
+	  : &(set->info_ordinary));
+}
 
 /* Returns the pointer to the memory region where maps are stored in
    the line table SET. MAP_KIND shall be TRUE if we are interested in
    macro maps false otherwise.  */
-#define LINEMAPS_MAPS(SET, MAP_KIND) \
-  (LINEMAPS_MAP_INFO (SET, MAP_KIND))->maps
+inline line_map *
+LINEMAPS_MAPS (const line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->maps;
+}
+
+inline line_map * &
+LINEMAPS_MAPS (line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->maps;
+}
 
 /* Returns the number of allocated maps so far. MAP_KIND shall be TRUE
    if we are interested in macro maps, FALSE otherwise.  */
-#define LINEMAPS_ALLOCATED(SET, MAP_KIND) \
-  (LINEMAPS_MAP_INFO (SET, MAP_KIND))->allocated
+inline unsigned int
+LINEMAPS_ALLOCATED (const line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->allocated;
+}
+
+inline unsigned int &
+LINEMAPS_ALLOCATED (line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->allocated;
+}
 
 /* Returns the number of used maps so far. MAP_KIND shall be TRUE if
    we are interested in macro maps, FALSE otherwise.*/
-#define LINEMAPS_USED(SET, MAP_KIND) \
-  (LINEMAPS_MAP_INFO (SET, MAP_KIND))->used
+inline unsigned int
+LINEMAPS_USED (const line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->used;
+}
+
+inline unsigned int &
+LINEMAPS_USED (line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->used;
+}
 
 /* Returns the index of the last map that was looked up with
    linemap_lookup. MAP_KIND shall be TRUE if we are interested in
    macro maps, FALSE otherwise.  */
-#define LINEMAPS_CACHE(SET, MAP_KIND) \
-  (LINEMAPS_MAP_INFO (SET, MAP_KIND))->cache
+inline unsigned int
+LINEMAPS_CACHE (const line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->cache;
+}
+
+inline unsigned int &
+LINEMAPS_CACHE (line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->cache;
+}
 
 /* Return the map at a given index.  */
-#define LINEMAPS_MAP_AT(SET, MAP_KIND, INDEX)	\
-  (&((LINEMAPS_MAPS (SET, MAP_KIND))[(INDEX)]))
+inline line_map *
+LINEMAPS_MAP_AT (const line_maps *set, bool map_kind, int index)
+{
+  return &(LINEMAPS_MAPS (set, map_kind)[index]);
+}
 
 /* Returns the last map used in the line table SET. MAP_KIND
    shall be TRUE if we are interested in macro maps, FALSE
    otherwise.*/
-#define LINEMAPS_LAST_MAP(SET, MAP_KIND) \
-  LINEMAPS_MAP_AT (SET, MAP_KIND, (LINEMAPS_USED (SET, MAP_KIND) - 1))
+inline line_map *
+LINEMAPS_LAST_MAP (const line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_AT (set, map_kind,
+			  LINEMAPS_USED (set, map_kind) - 1);
+}
 
 /* Returns the last map that was allocated in the line table SET.
    MAP_KIND shall be TRUE if we are interested in macro maps, FALSE
    otherwise.*/
-#define LINEMAPS_LAST_ALLOCATED_MAP(SET, MAP_KIND) \
-  LINEMAPS_MAP_AT (SET, MAP_KIND, LINEMAPS_ALLOCATED (SET, MAP_KIND) - 1)
+inline line_map *
+LINEMAPS_LAST_ALLOCATED_MAP (const line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_AT (set, map_kind,
+			  LINEMAPS_ALLOCATED (set, map_kind) - 1);
+}
 
 /* Returns a pointer to the memory region where ordinary maps are
    allocated in the line table SET.  */
-#define LINEMAPS_ORDINARY_MAPS(SET) \
-  LINEMAPS_MAPS (SET, false)
+inline line_map *
+LINEMAPS_ORDINARY_MAPS (const line_maps *set)
+{
+  return LINEMAPS_MAPS (set, false);
+}
 
 /* Returns the INDEXth ordinary map.  */
-#define LINEMAPS_ORDINARY_MAP_AT(SET, INDEX)	\
-  LINEMAPS_MAP_AT (SET, false, INDEX)
+inline line_map *
+LINEMAPS_ORDINARY_MAP_AT (const line_maps *set, int index)
+{
+  return LINEMAPS_MAP_AT (set, false, index);
+}
 
 /* Return the number of ordinary maps allocated in the line table
    SET.  */
-#define LINEMAPS_ORDINARY_ALLOCATED(SET) \
-  LINEMAPS_ALLOCATED(SET, false)
+inline unsigned int
+LINEMAPS_ORDINARY_ALLOCATED (const line_maps *set)
+{
+  return LINEMAPS_ALLOCATED (set, false);
+}
 
 /* Return the number of ordinary maps used in the line table SET.  */
-#define LINEMAPS_ORDINARY_USED(SET) \
-  LINEMAPS_USED(SET, false)
+inline unsigned int
+LINEMAPS_ORDINARY_USED (const line_maps *set)
+{
+  return LINEMAPS_USED (set, false);
+}
 
 /* Return the index of the last ordinary map that was looked up with
    linemap_lookup.  */
-#define LINEMAPS_ORDINARY_CACHE(SET) \
-  LINEMAPS_CACHE(SET, false)
+inline unsigned int
+LINEMAPS_ORDINARY_CACHE (const line_maps *set)
+{
+  return LINEMAPS_CACHE (set, false);
+}
+
+inline unsigned int &
+LINEMAPS_ORDINARY_CACHE (line_maps *set)
+{
+  return LINEMAPS_CACHE (set, false);
+}
 
 /* Returns a pointer to the last ordinary map used in the line table
    SET.  */
-#define LINEMAPS_LAST_ORDINARY_MAP(SET) \
-  LINEMAPS_LAST_MAP(SET, false)
+inline line_map *
+LINEMAPS_LAST_ORDINARY_MAP (const line_maps *set)
+{
+  return LINEMAPS_LAST_MAP (set, false);
+}
 
 /* Returns a pointer to the last ordinary map allocated the line table
    SET.  */
-#define LINEMAPS_LAST_ALLOCATED_ORDINARY_MAP(SET) \
-  LINEMAPS_LAST_ALLOCATED_MAP(SET, false)
+inline line_map *
+LINEMAPS_LAST_ALLOCATED_ORDINARY_MAP (const line_maps *set)
+{
+  return LINEMAPS_LAST_ALLOCATED_MAP (set, false);
+}
 
 /* Returns a pointer to the beginning of the region where macro maps
    are allcoated.  */
-#define LINEMAPS_MACRO_MAPS(SET) \
-  LINEMAPS_MAPS(SET, true)
+inline line_map *
+LINEMAPS_MACRO_MAPS (const line_maps *set)
+{
+  return LINEMAPS_MAPS (set, true);
+}
 
 /* Returns the INDEXth macro map.  */
-#define LINEMAPS_MACRO_MAP_AT(SET, INDEX)	\
-  LINEMAPS_MAP_AT (SET, true, INDEX)
+inline line_map *LINEMAPS_MACRO_MAP_AT (const line_maps *set, int index)
+{
+  return LINEMAPS_MAP_AT (set, true, index);
+}
 
 /* Returns the number of macro maps that were allocated in the line
    table SET.  */
-#define LINEMAPS_MACRO_ALLOCATED(SET) \
-  LINEMAPS_ALLOCATED(SET, true)
+inline unsigned int
+LINEMAPS_MACRO_ALLOCATED (const line_maps *set)
+{
+  return LINEMAPS_ALLOCATED (set, true);
+}
 
 /* Returns the number of macro maps used in the line table SET.  */
-#define LINEMAPS_MACRO_USED(SET) \
-  LINEMAPS_USED(SET, true)
+inline unsigned int
+LINEMAPS_MACRO_USED (const line_maps *set)
+{
+  return LINEMAPS_USED (set, true);
+}
 
 /* Returns the index of the last macro map looked up with
    linemap_lookup.  */
-#define LINEMAPS_MACRO_CACHE(SET) \
-  LINEMAPS_CACHE(SET, true)
+inline unsigned int
+LINEMAPS_MACRO_CACHE (const line_maps *set)
+{
+  return LINEMAPS_CACHE (set, true);
+}
 
-/* Returns the lowest location [of a token resulting from macro
-   expansion] encoded in this line table.  */
-#define LINEMAPS_MACRO_LOWEST_LOCATION(SET)			\
-  (LINEMAPS_MACRO_USED (set)					\
-   ? MAP_START_LOCATION (LINEMAPS_LAST_MACRO_MAP (set))		\
-   : MAX_SOURCE_LOCATION)
+inline unsigned int &
+LINEMAPS_MACRO_CACHE (line_maps *set)
+{
+  return LINEMAPS_CACHE (set, true);
+}
 
 /* Returns the last macro map used in the line table SET.  */
-#define LINEMAPS_LAST_MACRO_MAP(SET) \
-  LINEMAPS_LAST_MAP (SET, true)
+inline line_map *
+LINEMAPS_LAST_MACRO_MAP (const line_maps *set)
+{
+  return LINEMAPS_LAST_MAP (set, true);
+}
+
+/* Returns the lowest location [of a token resulting from macro
+   expansion] encoded in this line table.  */
+inline source_location
+LINEMAPS_MACRO_LOWEST_LOCATION (const line_maps *set)
+{
+  return LINEMAPS_MACRO_USED (set)
+         ? MAP_START_LOCATION (LINEMAPS_LAST_MACRO_MAP (set))
+         : MAX_SOURCE_LOCATION;
+}
 
 /* Returns the last macro map allocated in the line table SET.  */
-#define LINEMAPS_LAST_ALLOCATED_MACRO_MAP(SET) \
-  LINEMAPS_LAST_ALLOCATED_MAP (SET, true)
+inline line_map *
+LINEMAPS_LAST_ALLOCATED_MACRO_MAP (const line_maps *set)
+{
+  return LINEMAPS_LAST_ALLOCATED_MAP (set, true);
+}
 
 extern void location_adhoc_data_fini (struct line_maps *);
 extern source_location get_combined_adhoc_loc (struct line_maps *,
@@ -512,9 +764,18 @@ extern void *get_data_from_adhoc_loc (struct line_maps *, source_location);
 extern source_location get_location_from_adhoc_loc (struct line_maps *,
 						    source_location);
 
-#define IS_ADHOC_LOC(LOC) (((LOC) & MAX_SOURCE_LOCATION) != (LOC))
-#define COMBINE_LOCATION_DATA(SET, LOC, BLOCK) \
-  get_combined_adhoc_loc ((SET), (LOC), (BLOCK))
+inline bool IS_ADHOC_LOC (source_location loc)
+{
+  return (loc & MAX_SOURCE_LOCATION) != loc;
+}
+
+inline source_location
+COMBINE_LOCATION_DATA (struct line_maps *set,
+		       source_location loc,
+		       void *block)
+{
+  return get_combined_adhoc_loc (set, loc, block);
+}
 
 extern void rebuild_location_adhoc_htab (struct line_maps *);
 
@@ -568,10 +829,6 @@ extern const struct line_map *linemap_lookup
    macro expansion, FALSE otherwise.  */
 bool linemap_tracks_macro_expansion_locs_p (struct line_maps *);
 
-/* Return TRUE if MAP encodes locations coming from a macro
-   replacement-list at macro expansion point.  */
-bool linemap_macro_expansion_map_p (const struct line_map *);
-
 /* Return the name of the macro associated to MACRO_MAP.  */
 const char* linemap_map_get_macro_name (const struct line_map*);
 
@@ -596,78 +853,66 @@ bool linemap_location_from_macro_expansion_p (const struct line_maps *,
 /* source_location values from 0 to RESERVED_LOCATION_COUNT-1 will
    be reserved for libcpp user as special values, no token from libcpp
    will contain any of those locations.  */
-#define RESERVED_LOCATION_COUNT	2
+const int RESERVED_LOCATION_COUNT = 2;
 
 /* Converts a map and a source_location to source line.  */
-#define SOURCE_LINE(MAP, LOC)						\
-  (((((LOC) - linemap_check_ordinary (MAP)->start_location)		\
-     >> (MAP)->d.ordinary.column_bits) + (MAP)->d.ordinary.to_line))
+inline linenum_type
+SOURCE_LINE (const struct line_map *map, source_location loc)
+{
+  return ((loc - linemap_check_ordinary (map)->start_location)
+	  >> map->d.ordinary.column_bits) + map->d.ordinary.to_line;
+}
 
 /* Convert a map and source_location to source column number.  */
-#define SOURCE_COLUMN(MAP, LOC)						\
-  ((((LOC) - linemap_check_ordinary (MAP)->start_location)		\
-    & ((1 << (MAP)->d.ordinary.column_bits) - 1)))
+inline linenum_type
+SOURCE_COLUMN (const struct line_map *map, source_location loc)
+{
+  return ((loc - linemap_check_ordinary (map)->start_location)
+	  & ((1 << map->d.ordinary.column_bits) - 1));
+}
+
+/* Return the location of the last source line within an ordinary
+   map.  */
+inline source_location
+LAST_SOURCE_LINE_LOCATION (const struct line_map *map)
+{
+  return (((linemap_check_ordinary (map)[1].start_location - 1
+	    - map->start_location)
+	   & ~((1 << map->d.ordinary.column_bits) - 1))
+	  + map->start_location);
+}
 
 /* Returns the last source line number within an ordinary map.  This
    is the (last) line of the #include, or other directive, that caused
    a map change.  */
-#define LAST_SOURCE_LINE(MAP) \
-  SOURCE_LINE (MAP, LAST_SOURCE_LINE_LOCATION (MAP))
+inline linenum_type LAST_SOURCE_LINE (const struct line_map *map)
+{
+  return SOURCE_LINE (map, LAST_SOURCE_LINE_LOCATION (map));
+}
 
 /* Return the last column number within an ordinary map.  */
-#define LAST_SOURCE_COLUMN(MAP) \
-  SOURCE_COLUMN (MAP, LAST_SOURCE_LINE_LOCATION (MAP))
 
-/* Return the location of the last source line within an ordinary
-   map.  */
-#define LAST_SOURCE_LINE_LOCATION(MAP)					\
-  ((((linemap_check_ordinary (MAP)[1].start_location - 1		\
-      - (MAP)->start_location)						\
-     & ~((1 << (MAP)->d.ordinary.column_bits) - 1))			\
-    + (MAP)->start_location))
+inline linenum_type LAST_SOURCE_COLUMN (const struct line_map *map)
+{
+  return SOURCE_COLUMN (map, LAST_SOURCE_LINE_LOCATION (map));
+}
 
 /* Returns the map a given map was included from, or NULL if the map
    belongs to the main file, i.e, a file that wasn't included by
    another one.  */
-#define INCLUDED_FROM(SET, MAP)						\
-  ((linemap_check_ordinary (MAP)->d.ordinary.included_from == -1)	\
-   ? NULL								\
-   : (&LINEMAPS_ORDINARY_MAPS (SET)[(MAP)->d.ordinary.included_from]))
-
-/* Nonzero if the map is at the bottom of the include stack.  */
-#define MAIN_FILE_P(MAP)						\
-  ((linemap_check_ordinary (MAP)->d.ordinary.included_from < 0))
-
-#if defined ENABLE_CHECKING && (GCC_VERSION >= 2007)
-
-/* Assertion macro to be used in line-map code.  */
-#define linemap_assert(EXPR)			\
-  do {						\
-    if (! (EXPR))				\
-      abort ();					\
-  } while (0)
- 
-/* Assert that becomes a conditional expression when checking is disabled at
-   compilation time.  Use this for conditions that should not happen but if
-   they happen, it is better to handle them gracefully rather than crash
-   randomly later. 
-   Usage:
-
-   if (linemap_assert_fails(EXPR)) handle_error(); */
-#define linemap_assert_fails(EXPR) __extension__ \
-  ({linemap_assert (EXPR); false;}) 
+inline struct line_map *
+INCLUDED_FROM (struct line_maps *set, const struct line_map *map)
+{
+  return ((linemap_check_ordinary (map)->d.ordinary.included_from == -1)
+	  ? NULL
+	  : (&LINEMAPS_ORDINARY_MAPS (set)[(map)->d.ordinary.included_from]));
+}
 
-/* Assert that MAP encodes locations of tokens that are not part of
-   the replacement-list of a macro expansion.  */
-#define linemap_check_ordinary(LINE_MAP) __extension__		\
-  ({linemap_assert (!linemap_macro_expansion_map_p (LINE_MAP)); \
-    (LINE_MAP);})
-#else
-/* Include EXPR, so that unused variable warnings do not occur.  */
-#define linemap_assert(EXPR) ((void)(0 && (EXPR)))
-#define linemap_assert_fails(EXPR) (! (EXPR))
-#define linemap_check_ordinary(LINE_MAP) (LINE_MAP)
-#endif
+/* True if the map is at the bottom of the include stack.  */
+inline bool MAIN_FILE_P (const struct line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.included_from < 0;
+}
 
 /* Encode and return a source_location from a column number. The
    source line considered is the last source line used to call
@@ -691,19 +936,25 @@ linemap_position_for_loc_and_offset (struct line_maps *set,
 				     unsigned int offset);
 
 /* Return the file this map is for.  */
-#define LINEMAP_FILE(MAP)					\
-  (linemap_check_ordinary (MAP)->d.ordinary.to_file)
+inline const char * LINEMAP_FILE (const struct line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.to_file;
+}
 
 /* Return the line number this map started encoding location from.  */
-#define LINEMAP_LINE(MAP)					\
-  (linemap_check_ordinary (MAP)->d.ordinary.to_line)
+inline linenum_type LINEMAP_LINE (const struct line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.to_line;
+}
 
 /* Return a positive value if map encodes locations from a system
    header, 0 otherwise. Returns 1 if MAP encodes locations in a
    system header and 2 if it encodes locations in a C system header
    that therefore needs to be extern "C" protected in C++.  */
-#define LINEMAP_SYSP(MAP)					\
-  (linemap_check_ordinary (MAP)->d.ordinary.sysp)
+inline unsigned char LINEMAP_SYSP (const struct line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.sysp;
+}
 
 /* Return a positive value if PRE denotes the location of a token that
    comes before the token of POST, 0 if PRE denotes the location of
@@ -716,8 +967,12 @@ int linemap_compare_locations (struct line_maps *set,
 /* Return TRUE if LOC_A denotes the location a token that comes
    topogically before the token denoted by location LOC_B, or if they
    are equal.  */
-#define linemap_location_before_p(SET, LOC_A, LOC_B)	\
-  (linemap_compare_locations ((SET), (LOC_A), (LOC_B)) >= 0)
+inline bool linemap_location_before_p (struct line_maps *set,
+				       source_location loc_a,
+				       source_location loc_b)
+{
+  return linemap_compare_locations (set, loc_a, loc_b) >= 0;
+}
 
 typedef struct
 {
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 1db59a7..fd16c24 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -191,7 +191,7 @@ linemap_init (struct line_maps *set,
 void
 linemap_check_files_exited (struct line_maps *set)
 {
-  struct line_map *map;
+  const struct line_map *map;
   /* Depending upon whether we are handling preprocessed input or
      not, this can be a user error or an ICE.  */
   for (map = LINEMAPS_LAST_ORDINARY_MAP (set);
@@ -371,7 +371,7 @@ linemap_add (struct line_maps *set, enum lc_reason reason,
   ORDINARY_MAP_FILE_NAME (map) = to_file;
   ORDINARY_MAP_STARTING_LINE_NUMBER (map) = to_line;
   LINEMAPS_ORDINARY_CACHE (set) = LINEMAPS_ORDINARY_USED (set) - 1;
-  ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map) = 0;
+  SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map, 0);
   set->highest_location = start_location;
   set->highest_line = start_location;
   set->max_column_hint = 0;
@@ -564,7 +564,7 @@ linemap_line_start (struct line_maps *set, linenum_type to_line,
 					       (map),
 					       ORDINARY_MAP_FILE_NAME (map),
 					       to_line);
-      ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map) = column_bits;
+      SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map, column_bits);
       r = (MAP_START_LOCATION (map)
 	   + ((to_line - ORDINARY_MAP_STARTING_LINE_NUMBER (map))
 	      << column_bits));
-- 
1.8.5.3

Re: [PATCH 2/4] libcpp: Replace macro usage with C++ constructs

[Jeff Law]

On 05/01/2015 06:56 PM, David Malcolm wrote:
> libcpp makes extensive use of the C preprocessor.  Whilst this has a
> pleasingly self-referential quality, I find the code hard-to-read;
> implementing source location support in my JIT branch was much harder than
> I felt it should have been.
>
> In an attempt at making the code easier to follow, and to build towards
> a followup patch, this patch converts most of these macros to C++
> equivalents: using "const" for compile-time constants, and inline
> functions where macros aren't used as lvalues.
>
> This effectively documents the expected types of the params, and makes
> them available from the debugger e.g.:
>
>    (gdb) p LINEMAP_FILE ($3)
>    $1 = 0x13b8b37 "<command-line>"
>
> and indeed the constants also:
>
>    (gdb) p IS_ADHOC_LOC(MAX_SOURCE_LOCATION)
>    $2 = false
>    (gdb) p IS_ADHOC_LOC(MAX_SOURCE_LOCATION + 1)
>    $3 = true
>
> [I didn't mark the inline functions as "static"; should they be?]
>
> [FWIW, I posted a reduced version of this patch about a year ago as:
>    https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01092.html
> which covered a smaller subset of the macros].
>
> libcpp/ChangeLog:
> 	* include/line-map.h (MAX_SOURCE_LOCATION): Convert from a macro
> 	to a const source_location.
> 	(RESERVED_LOCATION_COUNT): Likewise.
> 	(linemap_check_ordinary): Convert from a macro to a pair of inline
> 	functions, for const/non-const arguments.
> 	(MAP_START_LOCATION): Likewise.
> 	(ORDINARY_MAP_STARTING_LINE_NUMBER): Likewise.
> 	(ORDINARY_MAP_INCLUDER_FILE_INDEX): Likewise.
> 	(ORDINARY_MAP_IN_SYSTEM_HEADER_P): Likewise.
> 	(ORDINARY_MAP_NUMBER_OF_COLUMN_BITS): Convert from a macro to a
> 	pair of inline functions, for const/non-const arguments, where the
> 	latter is named...
> 	(SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS): New function.
> 	(ORDINARY_MAP_FILE_NAME): Convert from a macro to a pair of inline
> 	functions, for const/non-const arguments.
> 	(MACRO_MAP_MACRO): Likewise.
> 	(MACRO_MAP_NUM_MACRO_TOKENS): Likewise.
> 	(MACRO_MAP_LOCATIONS): Likewise.
> 	(MACRO_MAP_EXPANSION_POINT_LOCATION): Likewise.
> 	(LINEMAPS_MAP_INFO): Likewise.
> 	(LINEMAPS_MAPS): Likewise.
> 	(LINEMAPS_ALLOCATED): Likewise.
> 	(LINEMAPS_USED): Likewise.
> 	(LINEMAPS_CACHE): Likewise.
> 	(LINEMAPS_ORDINARY_CACHE): Likewise.
> 	(LINEMAPS_MACRO_CACHE): Likewise.
> 	(LINEMAPS_MAP_AT): Convert from a macro to an inline function.
> 	(LINEMAPS_LAST_MAP): Likewise.
> 	(LINEMAPS_LAST_ALLOCATED_MAP): Likewise.
> 	(LINEMAPS_ORDINARY_MAPS): Likewise.
> 	(LINEMAPS_ORDINARY_MAP_AT): Likewise.
> 	(LINEMAPS_ORDINARY_ALLOCATED): Likewise.
> 	(LINEMAPS_ORDINARY_USED): Likewise.
> 	(LINEMAPS_LAST_ORDINARY_MAP): Likewise.
> 	(LINEMAPS_LAST_ALLOCATED_ORDINARY_MAP): Likewise.
> 	(LINEMAPS_MACRO_MAPS): Likewise.
> 	(LINEMAPS_MACRO_MAP_AT): Likewise.
> 	(LINEMAPS_MACRO_ALLOCATED): Likewise.
> 	(LINEMAPS_MACRO_USED): Likewise.
> 	(LINEMAPS_MACRO_LOWEST_LOCATION): Likewise.
> 	(LINEMAPS_LAST_MACRO_MAP): Likewise.
> 	(LINEMAPS_LAST_ALLOCATED_MACRO_MAP): Likewise.
> 	(IS_ADHOC_LOC): Likewise.
> 	(COMBINE_LOCATION_DATA): Likewise.
> 	(SOURCE_LINE): Likewise.
> 	(SOURCE_COLUMN): Likewise.
> 	(LAST_SOURCE_LINE_LOCATION): Likewise.
> 	(LAST_SOURCE_LINE): Likewise.
> 	(LAST_SOURCE_COLUMN): Likewise.
> 	(LAST_SOURCE_LINE_LOCATION)
> 	(INCLUDED_FROM): Likewise.
> 	(MAIN_FILE_P): Likewise.
> 	(LINEMAP_FILE): Likewise.
> 	(LINEMAP_LINE): Likewise.
> 	(LINEMAP_SYSP): Likewise.
> 	(linemap_location_before_p): Likewise.
> 	* line-map.c (linemap_check_files_exited): Make local "map" const.
> 	(linemap_add): Use SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS.
> 	(linemap_line_start): Likewise.
> ---
> -#define MAP_START_LOCATION(MAP) (MAP)->start_location
> +#if defined ENABLE_CHECKING && (GCC_VERSION >= 2007)
> +
> +/* Assertion macro to be used in line-map code.  */
> +#define linemap_assert(EXPR)                  \
> +  do {                                                \
> +    if (! (EXPR))                             \
> +      abort ();                                       \
> +  } while (0)
> +
> +/* Assert that becomes a conditional expression when checking is disabled at
> +   compilation time.  Use this for conditions that should not happen but if
> +   they happen, it is better to handle them gracefully rather than crash
> +   randomly later.
> +   Usage:
> +
> +   if (linemap_assert_fails(EXPR)) handle_error(); */
> +#define linemap_assert_fails(EXPR) __extension__ \
> +  ({linemap_assert (EXPR); false;})
> +
> +#else
> +/* Include EXPR, so that unused variable warnings do not occur.  */
> +#define linemap_assert(EXPR) ((void)(0 && (EXPR)))
> +#define linemap_assert_fails(EXPR) (! (EXPR))
> +#endif
So if we're generally trying to get away from #define programming, then 
this part seems like a bit of a step backwards.

The definition of linemap_assert in the #else clause seems odd.  Aren't 
you worried that the compiler will short-circuit the test?    If EXPR 
has side effects, then that's clearly not good.

I note you've got an abort () in here, that's generally frowned upon in 
libraries.


>
> -#define ORDINARY_MAP_FILE_NAME(MAP) \
> -  linemap_check_ordinary (MAP)->d.ordinary.to_file
> +/* Return TRUE if MAP encodes locations coming from a macro
> +   replacement-list at macro expansion point.  */
> +bool linemap_macro_expansion_map_p (const struct line_map *);
> +
> +/* Assert that MAP encodes locations of tokens that are not part of
> +   the replacement-list of a macro expansion.  */
> +inline struct line_map *linemap_check_ordinary (struct line_map *map)
Return type and inline declarations on a different line than the 
function name/arguments.

> -#define LINEMAPS_MAPS(SET, MAP_KIND) \
> -  (LINEMAPS_MAP_INFO (SET, MAP_KIND))->maps
> +inline line_map *
> +LINEMAPS_MAPS (const line_maps *set, bool map_kind)
> +{
> +  return LINEMAPS_MAP_INFO (set, map_kind)->maps;
> +}
> +
> +inline line_map * &
> +LINEMAPS_MAPS (line_maps *set, bool map_kind)
> +{
> +  return LINEMAPS_MAP_INFO (set, map_kind)->maps;
> +}
Do we really need two implementation of those various functions, one for 
const line_maps * the other for line_maps *?

While I suspect ICF ultimately should merge these functions, I'd really 
prefer to avoid the duplication.

Note that many of the macros didn't have comments (they should have, but 
it's often missed), correcting that as you turn them into functions 
would be greatly appreciated.

Generally, I like what I see.  There's some minor questions/issues to 
address, so let's go with another iteration.

jeff

[PATCH 2/4 v2] libcpp: Replace macro usage with C++ constructs)

[David Malcolm]

On Mon, 2015-05-04 at 13:15 -0600, Jeff Law wrote:
On 05/01/2015 06:56 PM, David Malcolm wrote:
> > libcpp makes extensive use of the C preprocessor.  Whilst this has a
> > pleasingly self-referential quality, I find the code hard-to-read;
> > implementing source location support in my JIT branch was much harder than
> > I felt it should have been.
> >
> > In an attempt at making the code easier to follow, and to build towards
> > a followup patch, this patch converts most of these macros to C++
> > equivalents: using "const" for compile-time constants, and inline
> > functions where macros aren't used as lvalues.
> >
> > This effectively documents the expected types of the params, and makes
> > them available from the debugger e.g.:
> >
> >    (gdb) p LINEMAP_FILE ($3)
> >    $1 = 0x13b8b37 "<command-line>"
> >
> > and indeed the constants also:
> >
> >    (gdb) p IS_ADHOC_LOC(MAX_SOURCE_LOCATION)
> >    $2 = false
> >    (gdb) p IS_ADHOC_LOC(MAX_SOURCE_LOCATION + 1)
> >    $3 = true
> >
> > [I didn't mark the inline functions as "static"; should they be?]
> >
> > [FWIW, I posted a reduced version of this patch about a year ago as:
> >    https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01092.html
> > which covered a smaller subset of the macros].
> >
> > libcpp/ChangeLog:
> > 	* include/line-map.h (MAX_SOURCE_LOCATION): Convert from a macro
> > 	to a const source_location.
> > 	(RESERVED_LOCATION_COUNT): Likewise.
> > 	(linemap_check_ordinary): Convert from a macro to a pair of inline
> > 	functions, for const/non-const arguments.
> > 	(MAP_START_LOCATION): Likewise.
> > 	(ORDINARY_MAP_STARTING_LINE_NUMBER): Likewise.
> > 	(ORDINARY_MAP_INCLUDER_FILE_INDEX): Likewise.
> > 	(ORDINARY_MAP_IN_SYSTEM_HEADER_P): Likewise.
> > 	(ORDINARY_MAP_NUMBER_OF_COLUMN_BITS): Convert from a macro to a
> > 	pair of inline functions, for const/non-const arguments, where the
> > 	latter is named...
> > 	(SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS): New function.
> > 	(ORDINARY_MAP_FILE_NAME): Convert from a macro to a pair of inline
> > 	functions, for const/non-const arguments.
> > 	(MACRO_MAP_MACRO): Likewise.
> > 	(MACRO_MAP_NUM_MACRO_TOKENS): Likewise.
> > 	(MACRO_MAP_LOCATIONS): Likewise.
> > 	(MACRO_MAP_EXPANSION_POINT_LOCATION): Likewise.
> > 	(LINEMAPS_MAP_INFO): Likewise.
> > 	(LINEMAPS_MAPS): Likewise.
> > 	(LINEMAPS_ALLOCATED): Likewise.
> > 	(LINEMAPS_USED): Likewise.
> > 	(LINEMAPS_CACHE): Likewise.
> > 	(LINEMAPS_ORDINARY_CACHE): Likewise.
> > 	(LINEMAPS_MACRO_CACHE): Likewise.
> > 	(LINEMAPS_MAP_AT): Convert from a macro to an inline function.
> > 	(LINEMAPS_LAST_MAP): Likewise.
> > 	(LINEMAPS_LAST_ALLOCATED_MAP): Likewise.
> > 	(LINEMAPS_ORDINARY_MAPS): Likewise.
> > 	(LINEMAPS_ORDINARY_MAP_AT): Likewise.
> > 	(LINEMAPS_ORDINARY_ALLOCATED): Likewise.
> > 	(LINEMAPS_ORDINARY_USED): Likewise.
> > 	(LINEMAPS_LAST_ORDINARY_MAP): Likewise.
> > 	(LINEMAPS_LAST_ALLOCATED_ORDINARY_MAP): Likewise.
> > 	(LINEMAPS_MACRO_MAPS): Likewise.
> > 	(LINEMAPS_MACRO_MAP_AT): Likewise.
> > 	(LINEMAPS_MACRO_ALLOCATED): Likewise.
> > 	(LINEMAPS_MACRO_USED): Likewise.
> > 	(LINEMAPS_MACRO_LOWEST_LOCATION): Likewise.
> > 	(LINEMAPS_LAST_MACRO_MAP): Likewise.
> > 	(LINEMAPS_LAST_ALLOCATED_MACRO_MAP): Likewise.
> > 	(IS_ADHOC_LOC): Likewise.
> > 	(COMBINE_LOCATION_DATA): Likewise.
> > 	(SOURCE_LINE): Likewise.
> > 	(SOURCE_COLUMN): Likewise.
> > 	(LAST_SOURCE_LINE_LOCATION): Likewise.
> > 	(LAST_SOURCE_LINE): Likewise.
> > 	(LAST_SOURCE_COLUMN): Likewise.
> > 	(LAST_SOURCE_LINE_LOCATION)
> > 	(INCLUDED_FROM): Likewise.
> > 	(MAIN_FILE_P): Likewise.
> > 	(LINEMAP_FILE): Likewise.
> > 	(LINEMAP_LINE): Likewise.
> > 	(LINEMAP_SYSP): Likewise.
> > 	(linemap_location_before_p): Likewise.
> > 	* line-map.c (linemap_check_files_exited): Make local "map" const.
> > 	(linemap_add): Use SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS.
> > 	(linemap_line_start): Likewise.
> > ---
> > -#define MAP_START_LOCATION(MAP) (MAP)->start_location
> > +#if defined ENABLE_CHECKING && (GCC_VERSION >= 2007)
> > +
> > +/* Assertion macro to be used in line-map code.  */
> > +#define linemap_assert(EXPR)                  \
> > +  do {                                                \
> > +    if (! (EXPR))                             \
> > +      abort ();                                       \
> > +  } while (0)
> > +
> > +/* Assert that becomes a conditional expression when checking is disabled at
> > +   compilation time.  Use this for conditions that should not happen but if
> > +   they happen, it is better to handle them gracefully rather than crash
> > +   randomly later.
> > +   Usage:
> > +
> > +   if (linemap_assert_fails(EXPR)) handle_error(); */
> > +#define linemap_assert_fails(EXPR) __extension__ \
> > +  ({linemap_assert (EXPR); false;})
> > +
> > +#else
> > +/* Include EXPR, so that unused variable warnings do not occur.  */
> > +#define linemap_assert(EXPR) ((void)(0 && (EXPR)))
> > +#define linemap_assert_fails(EXPR) (! (EXPR))
> > +#endif
> 
I moved linemap_assert and linemap_assert_fails higher up within the
file so that they can appear before the bodies of the inline functions.
I didn't change their implementation; it's a simple cut-and-paste, so
these hunks are just an artifact of git's diff-ing algorithm getting
confused by that.

Should I have called that out in the ChangeLog entries?

I've split patch 2 of the original kit into two parts, one containing
the move of these macros (and the decl of linemap_macro_expansion_map_p)
to higher up within the file, then a followup patch containing the
"real" changes.  (assuming they're OK, should I commit them separately,
for ease of seeing this in our SVN history?)

> So if we're generally trying to get away from #define programming, then 
> this part seems like a bit of a step backwards.
> 
> The definition of linemap_assert in the #else clause seems odd.  Aren't 
> you worried that the compiler will short-circuit the test?    If EXPR 
> has side effects, then that's clearly not good.
> 
The (0 && ....) part of this was added in r217473
(aka 36a4e8f0351cdac4befb259ad9ef1738498cdf4e):

2014-11-13  Manuel López-Ibáñez  <manu@gcc.gnu.org>

       * include/line-map.h: Include EXPR, so that unused variable warnings
       do not occur.

which was due to:
https://gcc.gnu.org/ml/gcc-patches/2014-11/msg01240.html

> I note you've got an abort () in here, that's generally frowned upon in 
> libraries.
> 
Agreed, though it's an assertion; it's not clear to me what else we
could do.  As noted above, I'm not changing this code, simply moving it
higher up in the header.

> -#define ORDINARY_MAP_FILE_NAME(MAP) \
> > -  linemap_check_ordinary (MAP)->d.ordinary.to_file
> > +/* Return TRUE if MAP encodes locations coming from a macro
> > +   replacement-list at macro expansion point.  */
> > +bool linemap_macro_expansion_map_p (const struct line_map *);
> > +
> > +/* Assert that MAP encodes locations of tokens that are not part of
> > +   the replacement-list of a macro expansion.  */
> > +inline struct line_map *linemap_check_ordinary (struct line_map *map)
> Return type and inline declarations on a different line than the 
> function name/arguments.
> 
(nods; will fix)

> -#define LINEMAPS_MAPS(SET, MAP_KIND) \
> > -  (LINEMAPS_MAP_INFO (SET, MAP_KIND))->maps
> > +inline line_map *
> > +LINEMAPS_MAPS (const line_maps *set, bool map_kind)
> > +{
> > +  return LINEMAPS_MAP_INFO (set, map_kind)->maps;
> > +}
> > +
> > +inline line_map * &
> > +LINEMAPS_MAPS (line_maps *set, bool map_kind)
> > +{
> > +  return LINEMAPS_MAP_INFO (set, map_kind)->maps;
> > +}
> Do we really need two implementation of those various functions, one for 
> const line_maps * the other for line_maps *?
> 

The issue here is when they are accessed as lvalues, rather than just as
rvalues.

I've had a go at eliminating some of the lvalue ones as a followup
patch: given that after patch 4 these become just field accesses, it's
trivial to turn the usage sites into plain accesses of fields.

Is that desirable, or are the uses of the macros regarded as useful
(e.g. for grepping)?  (my preference is for the former: to turn them
into plain field accesses at those sites)

Ironically, one of the ones that isn't so easy to eliminate is the
one you called out above.  Eliminating those ones made the code messier,
so I kept those ones.

> While I suspect ICF ultimately should merge these functions, I'd really 
> prefer to avoid the duplication.
> 
> Note that many of the macros didn't have comments (they should have, but 
> it's often missed), correcting that as you turn them into functions 
> would be greatly appreciated.
> 
> Generally, I like what I see.  There's some minor questions/issues to 
> address, so let's go with another iteration.
> 
> jeff

Thanks.

I'm sending updated patches; the old patch 2 split into 2 parts, and
a followup ("patch 5 of 4", as it were):

  [PATCH 2/4 v2: part 1] Move linemap_assert higher up within the file
  [PATCH 2/4 v2: part 2] libcpp: Replace macro usage with C++ constructs
  [PATCH 5/4] libcpp: Eliminate most of the non-const/reference-returning inline fns

Do I need to do any performance testing of this kit?  Given that the
resulting inline functions become trivial field lookups in patch 4,
I'm assuming that our inliner is good enough to optimize it as well
as the status quo implementation.

Also, for comments before functions, do we prefer a blank line between
the comment and decl, or for them to abutt?

/* Foo.  */

foo ();

vs

/* Bar.  */
bar ();

How do the following look?

Thanks
Dave

 gcc/ada/gcc-interface/trans.c |   2 +-
 gcc/c-family/c-common.h       |   4 +-
 gcc/c-family/c-lex.c          |   6 +-
 gcc/c-family/c-opts.c         |  14 +-
 gcc/c-family/c-ppoutput.c     |   6 +-
 gcc/common.opt                |   4 +
 gcc/diagnostic.c              |   2 +-
 gcc/fortran/cpp.c             |  12 +-
 gcc/genmatch.c                |   4 +-
 gcc/input.c                   | 244 ++++++++++++++-
 gcc/input.h                   |   2 +
 gcc/java/jcf-parse.c          |   2 +-
 gcc/toplev.c                  |   3 +
 gcc/tree-diagnostic.c         |   9 +-
 libcpp/directives.c           |  17 +-
 libcpp/files.c                |   2 +-
 libcpp/include/cpplib.h       |   2 +-
 libcpp/include/line-map.h     | 667 ++++++++++++++++++++++++++++++------------
 libcpp/internal.h             |  12 +-
 libcpp/line-map.c             | 266 +++++++++--------
 libcpp/location-example.txt   | 216 ++++++++++++++
 libcpp/macro.c                |  18 +-
 22 files changed, 1150 insertions(+), 364 deletions(-)
 create mode 100644 libcpp/location-example.txt

-- 
1.8.5.3

[PATCH 2/4 v2: part 2] libcpp: Replace macro usage with C++ constructs

[David Malcolm]

libcpp/ChangeLog:
	* include/line-map.h (MAX_SOURCE_LOCATION): Convert from a macro
	to a const source_location.
	(RESERVED_LOCATION_COUNT): Likewise.
	(linemap_check_ordinary): Convert from a macro to a pair of inline
	functions, for const/non-const arguments.
	(MAP_START_LOCATION): Likewise.
	(ORDINARY_MAP_STARTING_LINE_NUMBER): Likewise.
	(ORDINARY_MAP_INCLUDER_FILE_INDEX): Likewise.
	(ORDINARY_MAP_IN_SYSTEM_HEADER_P): Likewise.
	(ORDINARY_MAP_NUMBER_OF_COLUMN_BITS): Convert from a macro to a
	pair of inline functions, for const/non-const arguments, where the
	latter is named...
	(SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS): New function.
	(ORDINARY_MAP_FILE_NAME): Convert from a macro to a pair of inline
	functions, for const/non-const arguments.
	(MACRO_MAP_MACRO): Likewise.
	(MACRO_MAP_NUM_MACRO_TOKENS): Likewise.
	(MACRO_MAP_LOCATIONS): Likewise.
	(MACRO_MAP_EXPANSION_POINT_LOCATION): Likewise.
	(LINEMAPS_MAP_INFO): Likewise.
	(LINEMAPS_MAPS): Likewise.
	(LINEMAPS_ALLOCATED): Likewise.
	(LINEMAPS_USED): Likewise.
	(LINEMAPS_CACHE): Likewise.
	(LINEMAPS_ORDINARY_CACHE): Likewise.
	(LINEMAPS_MACRO_CACHE): Likewise.
	(LINEMAPS_MAP_AT): Convert from a macro to an inline function.
	(LINEMAPS_LAST_MAP): Likewise.
	(LINEMAPS_LAST_ALLOCATED_MAP): Likewise.
	(LINEMAPS_ORDINARY_MAPS): Likewise.
	(LINEMAPS_ORDINARY_MAP_AT): Likewise.
	(LINEMAPS_ORDINARY_ALLOCATED): Likewise.
	(LINEMAPS_ORDINARY_USED): Likewise.
	(LINEMAPS_LAST_ORDINARY_MAP): Likewise.
	(LINEMAPS_LAST_ALLOCATED_ORDINARY_MAP): Likewise.
	(LINEMAPS_MACRO_MAPS): Likewise.
	(LINEMAPS_MACRO_MAP_AT): Likewise.
	(LINEMAPS_MACRO_ALLOCATED): Likewise.
	(LINEMAPS_MACRO_USED): Likewise.
	(LINEMAPS_MACRO_LOWEST_LOCATION): Likewise.
	(LINEMAPS_LAST_MACRO_MAP): Likewise.
	(LINEMAPS_LAST_ALLOCATED_MACRO_MAP): Likewise.
	(IS_ADHOC_LOC): Likewise.
	(COMBINE_LOCATION_DATA): Likewise.
	(SOURCE_LINE): Likewise.
	(SOURCE_COLUMN): Likewise.
	(LAST_SOURCE_LINE_LOCATION): Likewise.
	(LAST_SOURCE_LINE): Likewise.
	(LAST_SOURCE_COLUMN): Likewise.
	(LAST_SOURCE_LINE_LOCATION)
	(INCLUDED_FROM): Likewise.
	(MAIN_FILE_P): Likewise.
	(LINEMAP_FILE): Likewise.
	(LINEMAP_LINE): Likewise.
	(LINEMAP_SYSP): Likewise.
	(linemap_location_before_p): Likewise.
	* line-map.c (linemap_check_files_exited): Make local "map" const.
	(linemap_add): Use SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS.
	(linemap_line_start): Likewise.
---
 libcpp/include/line-map.h | 562 +++++++++++++++++++++++++++++++++++++---------
 libcpp/line-map.c         |   6 +-
 2 files changed, 455 insertions(+), 113 deletions(-)

diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 7001552..67a64e9 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -155,7 +155,7 @@ struct GTY(()) line_map_ordinary {
 
 /* This is the highest possible source location encoded within an
    ordinary or macro map.  */
-#define MAX_SOURCE_LOCATION 0x7FFFFFFF
+const source_location MAX_SOURCE_LOCATION = 0x7FFFFFFF;
 
 struct cpp_hashnode;
 
@@ -298,16 +298,10 @@ struct GTY(()) line_map {
 #define linemap_assert_fails(EXPR) __extension__ \
   ({linemap_assert (EXPR); false;})
 
-/* Assert that MAP encodes locations of tokens that are not part of
-   the replacement-list of a macro expansion.  */
-#define linemap_check_ordinary(LINE_MAP) __extension__		\
-  ({linemap_assert (!linemap_macro_expansion_map_p (LINE_MAP)); \
-    (LINE_MAP);})
 #else
 /* Include EXPR, so that unused variable warnings do not occur.  */
 #define linemap_assert(EXPR) ((void)(0 && (EXPR)))
 #define linemap_assert_fails(EXPR) (! (EXPR))
-#define linemap_check_ordinary(LINE_MAP) (LINE_MAP)
 #endif
 
 /* Return TRUE if MAP encodes locations coming from a macro
@@ -315,30 +309,196 @@ struct GTY(()) line_map {
 bool
 linemap_macro_expansion_map_p (const struct line_map *);
 
-#define MAP_START_LOCATION(MAP) (MAP)->start_location
+/* Assert that MAP encodes locations of tokens that are not part of
+   the replacement-list of a macro expansion.  */
+inline struct line_map *
+linemap_check_ordinary (struct line_map *map)
+{
+  linemap_assert (!linemap_macro_expansion_map_p (map));
+  return map;
+}
+
+/* Assert that MAP encodes locations of tokens that are not part of
+   the replacement-list of a macro expansion.  */
+
+inline const struct line_map *
+linemap_check_ordinary (const struct line_map *map)
+{
+  linemap_assert (!linemap_macro_expansion_map_p (map));
+  return map;
+}
+
+/* Read the start location of MAP, as an rvalue.  */
+
+inline source_location
+MAP_START_LOCATION (const line_map *map)
+{
+  return map->start_location;
+}
+
+/* Access the start location of MAP as a reference
+   (e.g. as an lvalue).  */
+
+inline source_location&
+MAP_START_LOCATION (line_map *map)
+{
+  return map->start_location;
+}
+
+/* Get the starting line number of ordinary map MAP.  */
+
+inline linenum_type
+ORDINARY_MAP_STARTING_LINE_NUMBER (const line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.to_line;
+}
+
+/* Access the starting line number of ordinary map MAP by
+   reference (e.g. as an lvalue).  */
+
+inline linenum_type&
+ORDINARY_MAP_STARTING_LINE_NUMBER (line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.to_line;
+}
+
+/* Get the index of the ordinary map at whose end
+   ordinary map MAP was included.
+
+   File(s) at the bottom of the include stack have this set.  */
+
+inline int
+ORDINARY_MAP_INCLUDER_FILE_INDEX (const line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.included_from;
+}
+
+/* As above, but by reference (e.g. as an lvalue).  */
+
+inline int&
+ORDINARY_MAP_INCLUDER_FILE_INDEX (line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.included_from;
+}
+
+/* Return a positive value if map encodes locations from a system
+   header, 0 otherwise. Returns 1 if ordinary map MAP encodes locations
+   in a system header and 2 if it encodes locations in a C system header
+   that therefore needs to be extern "C" protected in C++.  */
+
+inline unsigned char
+ORDINARY_MAP_IN_SYSTEM_HEADER_P (const line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.sysp;
+}
+
+/* As above, but by reference (e.g. as an lvalue).  */
+
+inline unsigned char &
+ORDINARY_MAP_IN_SYSTEM_HEADER_P (line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.sysp;
+}
+
+/* Get the number of the low-order source_location bits used for a
+   column number within ordinary map MAP.  */
+
+inline unsigned char
+ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (const line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.column_bits;
+}
+
+/* Set the number of the low-order source_location bits used for a
+   column number within ordinary map MAP.  */
+
+inline void
+SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (line_map *map, int col_bits)
+{
+  linemap_check_ordinary (map)->d.ordinary.column_bits = col_bits;
+}
+
+/* Get the filename of ordinary map MAP.  */
+
+inline const char *
+ORDINARY_MAP_FILE_NAME (const line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.to_file;
+}
+
+/* As above, but by reference (e.g. as an lvalue).  */
+
+inline const char * &
+ORDINARY_MAP_FILE_NAME (line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.to_file;
+}
+
+/* Get the cpp macro whose expansion gave birth to macro map MAP.  */
+
+inline cpp_hashnode *
+MACRO_MAP_MACRO (const line_map *map)
+{
+  return map->d.macro.macro;
+}
 
-#define ORDINARY_MAP_FILE_NAME(MAP) \
-  linemap_check_ordinary (MAP)->d.ordinary.to_file
+/* As above, but by reference (e.g. as an lvalue).  */
 
-#define ORDINARY_MAP_STARTING_LINE_NUMBER(MAP) \
-  linemap_check_ordinary (MAP)->d.ordinary.to_line
+inline cpp_hashnode * &
+MACRO_MAP_MACRO (line_map *map)
+{
+  return map->d.macro.macro;
+}
 
-#define ORDINARY_MAP_INCLUDER_FILE_INDEX(MAP) \
-  linemap_check_ordinary (MAP)->d.ordinary.included_from
+/* Get the number of tokens inside the replacement-list of the macro
+   that led to macro map MAP.  */
 
-#define ORDINARY_MAP_IN_SYSTEM_HEADER_P(MAP) \
-  linemap_check_ordinary (MAP)->d.ordinary.sysp
+inline unsigned int
+MACRO_MAP_NUM_MACRO_TOKENS (const line_map *map)
+{
+  return map->d.macro.n_tokens;
+}
 
-#define ORDINARY_MAP_NUMBER_OF_COLUMN_BITS(MAP) \
-  linemap_check_ordinary (MAP)->d.ordinary.column_bits
+/* As above, but by reference (e.g. as an lvalue).  */
 
-#define MACRO_MAP_MACRO(MAP) (MAP)->d.macro.macro
+inline unsigned int &
+MACRO_MAP_NUM_MACRO_TOKENS (line_map *map)
+{
+  return map->d.macro.n_tokens;
+}
 
-#define MACRO_MAP_NUM_MACRO_TOKENS(MAP) (MAP)->d.macro.n_tokens
+/* Get the array of pairs of locations within macro map MAP.
+   See the declaration of line_map_macro for more information.  */
 
-#define MACRO_MAP_LOCATIONS(MAP) (MAP)->d.macro.macro_locations
+inline source_location *
+MACRO_MAP_LOCATIONS (const line_map *map)
+{
+  return map->d.macro.macro_locations;
+}
 
-#define MACRO_MAP_EXPANSION_POINT_LOCATION(MAP) (MAP)->d.macro.expansion
+/* As above, but by reference (e.g. as an lvalue).  */
+
+inline source_location * &
+MACRO_MAP_LOCATIONS (line_map *map)
+{
+  return map->d.macro.macro_locations;
+}
+
+/* Get the location of the expansion point of the macro map MAP.  */
+
+inline source_location
+MACRO_MAP_EXPANSION_POINT_LOCATION (const line_map *map)
+{
+  return map->d.macro.expansion;
+}
+
+/* As above, but by reference (e.g. as an lvalue).  */
+
+inline source_location &
+MACRO_MAP_EXPANSION_POINT_LOCATION (line_map *map)
+{
+  return map->d.macro.expansion;
+}
 
 /* The abstraction of a set of location maps. There can be several
    types of location maps. This abstraction contains the attributes
@@ -427,119 +587,248 @@ struct GTY(()) line_maps {
 /* Returns the pointer to the memory region where information about
    maps are stored in the line table SET. MACRO_MAP_P is a flag
    telling if we want macro or ordinary maps.  */
-#define LINEMAPS_MAP_INFO(SET, MACRO_MAP_P)				\
-  ((MACRO_MAP_P)							\
-   ? &((SET)->info_macro)						\
-   : &((SET)->info_ordinary))
+inline struct maps_info *
+LINEMAPS_MAP_INFO (line_maps *set, bool macro_map_p)
+{
+  return (macro_map_p
+	  ? &(set->info_macro)
+	  : &(set->info_ordinary));
+}
+
+/* As above, but preserving constness.  */
+
+inline const struct maps_info *
+LINEMAPS_MAP_INFO (const line_maps *set, bool macro_map_p)
+{
+  return (macro_map_p
+	  ? &(set->info_macro)
+	  : &(set->info_ordinary));
+}
 
 /* Returns the pointer to the memory region where maps are stored in
    the line table SET. MAP_KIND shall be TRUE if we are interested in
    macro maps false otherwise.  */
-#define LINEMAPS_MAPS(SET, MAP_KIND) \
-  (LINEMAPS_MAP_INFO (SET, MAP_KIND))->maps
+inline line_map *
+LINEMAPS_MAPS (const line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->maps;
+}
+
+/* As above, but by reference (e.g. as an lvalue).  */
+
+inline line_map * &
+LINEMAPS_MAPS (line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->maps;
+}
 
 /* Returns the number of allocated maps so far. MAP_KIND shall be TRUE
    if we are interested in macro maps, FALSE otherwise.  */
-#define LINEMAPS_ALLOCATED(SET, MAP_KIND) \
-  (LINEMAPS_MAP_INFO (SET, MAP_KIND))->allocated
+inline unsigned int
+LINEMAPS_ALLOCATED (const line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->allocated;
+}
+
+/* As above, but by reference (e.g. as an lvalue).  */
+
+inline unsigned int &
+LINEMAPS_ALLOCATED (line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->allocated;
+}
 
 /* Returns the number of used maps so far. MAP_KIND shall be TRUE if
    we are interested in macro maps, FALSE otherwise.*/
-#define LINEMAPS_USED(SET, MAP_KIND) \
-  (LINEMAPS_MAP_INFO (SET, MAP_KIND))->used
+inline unsigned int
+LINEMAPS_USED (const line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->used;
+}
+
+/* As above, but by reference (e.g. as an lvalue).  */
+
+inline unsigned int &
+LINEMAPS_USED (line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->used;
+}
 
 /* Returns the index of the last map that was looked up with
    linemap_lookup. MAP_KIND shall be TRUE if we are interested in
    macro maps, FALSE otherwise.  */
-#define LINEMAPS_CACHE(SET, MAP_KIND) \
-  (LINEMAPS_MAP_INFO (SET, MAP_KIND))->cache
+inline unsigned int
+LINEMAPS_CACHE (const line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->cache;
+}
+
+/* As above, but by reference (e.g. as an lvalue).  */
+
+inline unsigned int &
+LINEMAPS_CACHE (line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_INFO (set, map_kind)->cache;
+}
 
 /* Return the map at a given index.  */
-#define LINEMAPS_MAP_AT(SET, MAP_KIND, INDEX)	\
-  (&((LINEMAPS_MAPS (SET, MAP_KIND))[(INDEX)]))
+inline line_map *
+LINEMAPS_MAP_AT (const line_maps *set, bool map_kind, int index)
+{
+  return &(LINEMAPS_MAPS (set, map_kind)[index]);
+}
 
 /* Returns the last map used in the line table SET. MAP_KIND
    shall be TRUE if we are interested in macro maps, FALSE
    otherwise.*/
-#define LINEMAPS_LAST_MAP(SET, MAP_KIND) \
-  LINEMAPS_MAP_AT (SET, MAP_KIND, (LINEMAPS_USED (SET, MAP_KIND) - 1))
+inline line_map *
+LINEMAPS_LAST_MAP (const line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_AT (set, map_kind,
+			  LINEMAPS_USED (set, map_kind) - 1);
+}
 
 /* Returns the last map that was allocated in the line table SET.
    MAP_KIND shall be TRUE if we are interested in macro maps, FALSE
    otherwise.*/
-#define LINEMAPS_LAST_ALLOCATED_MAP(SET, MAP_KIND) \
-  LINEMAPS_MAP_AT (SET, MAP_KIND, LINEMAPS_ALLOCATED (SET, MAP_KIND) - 1)
+inline line_map *
+LINEMAPS_LAST_ALLOCATED_MAP (const line_maps *set, bool map_kind)
+{
+  return LINEMAPS_MAP_AT (set, map_kind,
+			  LINEMAPS_ALLOCATED (set, map_kind) - 1);
+}
 
 /* Returns a pointer to the memory region where ordinary maps are
    allocated in the line table SET.  */
-#define LINEMAPS_ORDINARY_MAPS(SET) \
-  LINEMAPS_MAPS (SET, false)
+inline line_map *
+LINEMAPS_ORDINARY_MAPS (const line_maps *set)
+{
+  return LINEMAPS_MAPS (set, false);
+}
 
 /* Returns the INDEXth ordinary map.  */
-#define LINEMAPS_ORDINARY_MAP_AT(SET, INDEX)	\
-  LINEMAPS_MAP_AT (SET, false, INDEX)
+inline line_map *
+LINEMAPS_ORDINARY_MAP_AT (const line_maps *set, int index)
+{
+  return LINEMAPS_MAP_AT (set, false, index);
+}
 
 /* Return the number of ordinary maps allocated in the line table
    SET.  */
-#define LINEMAPS_ORDINARY_ALLOCATED(SET) \
-  LINEMAPS_ALLOCATED(SET, false)
+inline unsigned int
+LINEMAPS_ORDINARY_ALLOCATED (const line_maps *set)
+{
+  return LINEMAPS_ALLOCATED (set, false);
+}
 
 /* Return the number of ordinary maps used in the line table SET.  */
-#define LINEMAPS_ORDINARY_USED(SET) \
-  LINEMAPS_USED(SET, false)
+inline unsigned int
+LINEMAPS_ORDINARY_USED (const line_maps *set)
+{
+  return LINEMAPS_USED (set, false);
+}
 
 /* Return the index of the last ordinary map that was looked up with
    linemap_lookup.  */
-#define LINEMAPS_ORDINARY_CACHE(SET) \
-  LINEMAPS_CACHE(SET, false)
+inline unsigned int
+LINEMAPS_ORDINARY_CACHE (const line_maps *set)
+{
+  return LINEMAPS_CACHE (set, false);
+}
+
+/* As above, but by reference (e.g. as an lvalue).  */
+
+inline unsigned int &
+LINEMAPS_ORDINARY_CACHE (line_maps *set)
+{
+  return LINEMAPS_CACHE (set, false);
+}
 
 /* Returns a pointer to the last ordinary map used in the line table
    SET.  */
-#define LINEMAPS_LAST_ORDINARY_MAP(SET) \
-  LINEMAPS_LAST_MAP(SET, false)
+inline line_map *
+LINEMAPS_LAST_ORDINARY_MAP (const line_maps *set)
+{
+  return LINEMAPS_LAST_MAP (set, false);
+}
 
 /* Returns a pointer to the last ordinary map allocated the line table
    SET.  */
-#define LINEMAPS_LAST_ALLOCATED_ORDINARY_MAP(SET) \
-  LINEMAPS_LAST_ALLOCATED_MAP(SET, false)
+inline line_map *
+LINEMAPS_LAST_ALLOCATED_ORDINARY_MAP (const line_maps *set)
+{
+  return LINEMAPS_LAST_ALLOCATED_MAP (set, false);
+}
 
 /* Returns a pointer to the beginning of the region where macro maps
    are allcoated.  */
-#define LINEMAPS_MACRO_MAPS(SET) \
-  LINEMAPS_MAPS(SET, true)
+inline line_map *
+LINEMAPS_MACRO_MAPS (const line_maps *set)
+{
+  return LINEMAPS_MAPS (set, true);
+}
 
 /* Returns the INDEXth macro map.  */
-#define LINEMAPS_MACRO_MAP_AT(SET, INDEX)	\
-  LINEMAPS_MAP_AT (SET, true, INDEX)
+inline line_map *
+LINEMAPS_MACRO_MAP_AT (const line_maps *set, int index)
+{
+  return LINEMAPS_MAP_AT (set, true, index);
+}
 
 /* Returns the number of macro maps that were allocated in the line
    table SET.  */
-#define LINEMAPS_MACRO_ALLOCATED(SET) \
-  LINEMAPS_ALLOCATED(SET, true)
+inline unsigned int
+LINEMAPS_MACRO_ALLOCATED (const line_maps *set)
+{
+  return LINEMAPS_ALLOCATED (set, true);
+}
 
 /* Returns the number of macro maps used in the line table SET.  */
-#define LINEMAPS_MACRO_USED(SET) \
-  LINEMAPS_USED(SET, true)
+inline unsigned int
+LINEMAPS_MACRO_USED (const line_maps *set)
+{
+  return LINEMAPS_USED (set, true);
+}
 
 /* Returns the index of the last macro map looked up with
    linemap_lookup.  */
-#define LINEMAPS_MACRO_CACHE(SET) \
-  LINEMAPS_CACHE(SET, true)
+inline unsigned int
+LINEMAPS_MACRO_CACHE (const line_maps *set)
+{
+  return LINEMAPS_CACHE (set, true);
+}
 
-/* Returns the lowest location [of a token resulting from macro
-   expansion] encoded in this line table.  */
-#define LINEMAPS_MACRO_LOWEST_LOCATION(SET)			\
-  (LINEMAPS_MACRO_USED (set)					\
-   ? MAP_START_LOCATION (LINEMAPS_LAST_MACRO_MAP (set))		\
-   : MAX_SOURCE_LOCATION)
+/* As above, but by reference (e.g. as an lvalue).  */
+
+inline unsigned int &
+LINEMAPS_MACRO_CACHE (line_maps *set)
+{
+  return LINEMAPS_CACHE (set, true);
+}
 
 /* Returns the last macro map used in the line table SET.  */
-#define LINEMAPS_LAST_MACRO_MAP(SET) \
-  LINEMAPS_LAST_MAP (SET, true)
+inline line_map *
+LINEMAPS_LAST_MACRO_MAP (const line_maps *set)
+{
+  return LINEMAPS_LAST_MAP (set, true);
+}
+
+/* Returns the lowest location [of a token resulting from macro
+   expansion] encoded in this line table.  */
+inline source_location
+LINEMAPS_MACRO_LOWEST_LOCATION (const line_maps *set)
+{
+  return LINEMAPS_MACRO_USED (set)
+         ? MAP_START_LOCATION (LINEMAPS_LAST_MACRO_MAP (set))
+         : MAX_SOURCE_LOCATION;
+}
 
 /* Returns the last macro map allocated in the line table SET.  */
-#define LINEMAPS_LAST_ALLOCATED_MACRO_MAP(SET) \
-  LINEMAPS_LAST_ALLOCATED_MAP (SET, true)
+inline line_map *
+LINEMAPS_LAST_ALLOCATED_MACRO_MAP (const line_maps *set)
+{
+  return LINEMAPS_LAST_ALLOCATED_MAP (set, true);
+}
 
 extern void location_adhoc_data_fini (struct line_maps *);
 extern source_location get_combined_adhoc_loc (struct line_maps *,
@@ -548,9 +837,23 @@ extern void *get_data_from_adhoc_loc (struct line_maps *, source_location);
 extern source_location get_location_from_adhoc_loc (struct line_maps *,
 						    source_location);
 
-#define IS_ADHOC_LOC(LOC) (((LOC) & MAX_SOURCE_LOCATION) != (LOC))
-#define COMBINE_LOCATION_DATA(SET, LOC, BLOCK) \
-  get_combined_adhoc_loc ((SET), (LOC), (BLOCK))
+/* Get whether location LOC is an ad-hoc location.  */
+
+inline bool
+IS_ADHOC_LOC (source_location loc)
+{
+  return (loc & MAX_SOURCE_LOCATION) != loc;
+}
+
+/* Combine LOC and BLOCK, giving a combined adhoc location.  */
+
+inline source_location
+COMBINE_LOCATION_DATA (struct line_maps *set,
+		       source_location loc,
+		       void *block)
+{
+  return get_combined_adhoc_loc (set, loc, block);
+}
 
 extern void rebuild_location_adhoc_htab (struct line_maps *);
 
@@ -628,47 +931,72 @@ bool linemap_location_from_macro_expansion_p (const struct line_maps *,
 /* source_location values from 0 to RESERVED_LOCATION_COUNT-1 will
    be reserved for libcpp user as special values, no token from libcpp
    will contain any of those locations.  */
-#define RESERVED_LOCATION_COUNT	2
+const int RESERVED_LOCATION_COUNT = 2;
 
 /* Converts a map and a source_location to source line.  */
-#define SOURCE_LINE(MAP, LOC)						\
-  (((((LOC) - linemap_check_ordinary (MAP)->start_location)		\
-     >> (MAP)->d.ordinary.column_bits) + (MAP)->d.ordinary.to_line))
+inline linenum_type
+SOURCE_LINE (const struct line_map *map, source_location loc)
+{
+  return ((loc - linemap_check_ordinary (map)->start_location)
+	  >> map->d.ordinary.column_bits) + map->d.ordinary.to_line;
+}
 
 /* Convert a map and source_location to source column number.  */
-#define SOURCE_COLUMN(MAP, LOC)						\
-  ((((LOC) - linemap_check_ordinary (MAP)->start_location)		\
-    & ((1 << (MAP)->d.ordinary.column_bits) - 1)))
+inline linenum_type
+SOURCE_COLUMN (const struct line_map *map, source_location loc)
+{
+  return ((loc - linemap_check_ordinary (map)->start_location)
+	  & ((1 << map->d.ordinary.column_bits) - 1));
+}
+
+/* Return the location of the last source line within an ordinary
+   map.  */
+inline source_location
+LAST_SOURCE_LINE_LOCATION (const struct line_map *map)
+{
+  return (((linemap_check_ordinary (map)[1].start_location - 1
+	    - map->start_location)
+	   & ~((1 << map->d.ordinary.column_bits) - 1))
+	  + map->start_location);
+}
 
 /* Returns the last source line number within an ordinary map.  This
    is the (last) line of the #include, or other directive, that caused
    a map change.  */
-#define LAST_SOURCE_LINE(MAP) \
-  SOURCE_LINE (MAP, LAST_SOURCE_LINE_LOCATION (MAP))
+
+inline linenum_type
+LAST_SOURCE_LINE (const struct line_map *map)
+{
+  return SOURCE_LINE (map, LAST_SOURCE_LINE_LOCATION (map));
+}
 
 /* Return the last column number within an ordinary map.  */
-#define LAST_SOURCE_COLUMN(MAP) \
-  SOURCE_COLUMN (MAP, LAST_SOURCE_LINE_LOCATION (MAP))
 
-/* Return the location of the last source line within an ordinary
-   map.  */
-#define LAST_SOURCE_LINE_LOCATION(MAP)					\
-  ((((linemap_check_ordinary (MAP)[1].start_location - 1		\
-      - (MAP)->start_location)						\
-     & ~((1 << (MAP)->d.ordinary.column_bits) - 1))			\
-    + (MAP)->start_location))
+inline linenum_type
+LAST_SOURCE_COLUMN (const struct line_map *map)
+{
+  return SOURCE_COLUMN (map, LAST_SOURCE_LINE_LOCATION (map));
+}
 
 /* Returns the map a given map was included from, or NULL if the map
    belongs to the main file, i.e, a file that wasn't included by
    another one.  */
-#define INCLUDED_FROM(SET, MAP)						\
-  ((linemap_check_ordinary (MAP)->d.ordinary.included_from == -1)	\
-   ? NULL								\
-   : (&LINEMAPS_ORDINARY_MAPS (SET)[(MAP)->d.ordinary.included_from]))
 
-/* Nonzero if the map is at the bottom of the include stack.  */
-#define MAIN_FILE_P(MAP)						\
-  ((linemap_check_ordinary (MAP)->d.ordinary.included_from < 0))
+inline struct line_map *
+INCLUDED_FROM (struct line_maps *set, const struct line_map *map)
+{
+  return ((linemap_check_ordinary (map)->d.ordinary.included_from == -1)
+	  ? NULL
+	  : (&LINEMAPS_ORDINARY_MAPS (set)[(map)->d.ordinary.included_from]));
+}
+
+/* True if the map is at the bottom of the include stack.  */
+
+inline bool
+MAIN_FILE_P (const struct line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.included_from < 0;
+}
 
 /* Encode and return a source_location from a column number. The
    source line considered is the last source line used to call
@@ -692,19 +1020,28 @@ linemap_position_for_loc_and_offset (struct line_maps *set,
 				     unsigned int offset);
 
 /* Return the file this map is for.  */
-#define LINEMAP_FILE(MAP)					\
-  (linemap_check_ordinary (MAP)->d.ordinary.to_file)
+inline const char *
+LINEMAP_FILE (const struct line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.to_file;
+}
 
 /* Return the line number this map started encoding location from.  */
-#define LINEMAP_LINE(MAP)					\
-  (linemap_check_ordinary (MAP)->d.ordinary.to_line)
+inline linenum_type
+LINEMAP_LINE (const struct line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.to_line;
+}
 
 /* Return a positive value if map encodes locations from a system
    header, 0 otherwise. Returns 1 if MAP encodes locations in a
    system header and 2 if it encodes locations in a C system header
    that therefore needs to be extern "C" protected in C++.  */
-#define LINEMAP_SYSP(MAP)					\
-  (linemap_check_ordinary (MAP)->d.ordinary.sysp)
+inline unsigned char
+LINEMAP_SYSP (const struct line_map *map)
+{
+  return linemap_check_ordinary (map)->d.ordinary.sysp;
+}
 
 /* Return a positive value if PRE denotes the location of a token that
    comes before the token of POST, 0 if PRE denotes the location of
@@ -717,8 +1054,13 @@ int linemap_compare_locations (struct line_maps *set,
 /* Return TRUE if LOC_A denotes the location a token that comes
    topogically before the token denoted by location LOC_B, or if they
    are equal.  */
-#define linemap_location_before_p(SET, LOC_A, LOC_B)	\
-  (linemap_compare_locations ((SET), (LOC_A), (LOC_B)) >= 0)
+inline bool
+linemap_location_before_p (struct line_maps *set,
+			   source_location loc_a,
+			   source_location loc_b)
+{
+  return linemap_compare_locations (set, loc_a, loc_b) >= 0;
+}
 
 typedef struct
 {
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 1db59a7..fd16c24 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -191,7 +191,7 @@ linemap_init (struct line_maps *set,
 void
 linemap_check_files_exited (struct line_maps *set)
 {
-  struct line_map *map;
+  const struct line_map *map;
   /* Depending upon whether we are handling preprocessed input or
      not, this can be a user error or an ICE.  */
   for (map = LINEMAPS_LAST_ORDINARY_MAP (set);
@@ -371,7 +371,7 @@ linemap_add (struct line_maps *set, enum lc_reason reason,
   ORDINARY_MAP_FILE_NAME (map) = to_file;
   ORDINARY_MAP_STARTING_LINE_NUMBER (map) = to_line;
   LINEMAPS_ORDINARY_CACHE (set) = LINEMAPS_ORDINARY_USED (set) - 1;
-  ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map) = 0;
+  SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map, 0);
   set->highest_location = start_location;
   set->highest_line = start_location;
   set->max_column_hint = 0;
@@ -564,7 +564,7 @@ linemap_line_start (struct line_maps *set, linenum_type to_line,
 					       (map),
 					       ORDINARY_MAP_FILE_NAME (map),
 					       to_line);
-      ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map) = column_bits;
+      SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map, column_bits);
       r = (MAP_START_LOCATION (map)
 	   + ((to_line - ORDINARY_MAP_STARTING_LINE_NUMBER (map))
 	      << column_bits));
-- 
1.8.5.3

Re: [PATCH 2/4 v2: part 2] libcpp: Replace macro usage with C++ constructs

[Jeff Law]

On 05/05/2015 12:21 PM, David Malcolm wrote:
> libcpp/ChangeLog:
> 	* include/line-map.h (MAX_SOURCE_LOCATION): Convert from a macro
> 	to a const source_location.
> 	(RESERVED_LOCATION_COUNT): Likewise.
> 	(linemap_check_ordinary): Convert from a macro to a pair of inline
> 	functions, for const/non-const arguments.
> 	(MAP_START_LOCATION): Likewise.
> 	(ORDINARY_MAP_STARTING_LINE_NUMBER): Likewise.
> 	(ORDINARY_MAP_INCLUDER_FILE_INDEX): Likewise.
> 	(ORDINARY_MAP_IN_SYSTEM_HEADER_P): Likewise.
> 	(ORDINARY_MAP_NUMBER_OF_COLUMN_BITS): Convert from a macro to a
> 	pair of inline functions, for const/non-const arguments, where the
> 	latter is named...
> 	(SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS): New function.
> 	(ORDINARY_MAP_FILE_NAME): Convert from a macro to a pair of inline
> 	functions, for const/non-const arguments.
> 	(MACRO_MAP_MACRO): Likewise.
> 	(MACRO_MAP_NUM_MACRO_TOKENS): Likewise.
> 	(MACRO_MAP_LOCATIONS): Likewise.
> 	(MACRO_MAP_EXPANSION_POINT_LOCATION): Likewise.
> 	(LINEMAPS_MAP_INFO): Likewise.
> 	(LINEMAPS_MAPS): Likewise.
> 	(LINEMAPS_ALLOCATED): Likewise.
> 	(LINEMAPS_USED): Likewise.
> 	(LINEMAPS_CACHE): Likewise.
> 	(LINEMAPS_ORDINARY_CACHE): Likewise.
> 	(LINEMAPS_MACRO_CACHE): Likewise.
> 	(LINEMAPS_MAP_AT): Convert from a macro to an inline function.
> 	(LINEMAPS_LAST_MAP): Likewise.
> 	(LINEMAPS_LAST_ALLOCATED_MAP): Likewise.
> 	(LINEMAPS_ORDINARY_MAPS): Likewise.
> 	(LINEMAPS_ORDINARY_MAP_AT): Likewise.
> 	(LINEMAPS_ORDINARY_ALLOCATED): Likewise.
> 	(LINEMAPS_ORDINARY_USED): Likewise.
> 	(LINEMAPS_LAST_ORDINARY_MAP): Likewise.
> 	(LINEMAPS_LAST_ALLOCATED_ORDINARY_MAP): Likewise.
> 	(LINEMAPS_MACRO_MAPS): Likewise.
> 	(LINEMAPS_MACRO_MAP_AT): Likewise.
> 	(LINEMAPS_MACRO_ALLOCATED): Likewise.
> 	(LINEMAPS_MACRO_USED): Likewise.
> 	(LINEMAPS_MACRO_LOWEST_LOCATION): Likewise.
> 	(LINEMAPS_LAST_MACRO_MAP): Likewise.
> 	(LINEMAPS_LAST_ALLOCATED_MACRO_MAP): Likewise.
> 	(IS_ADHOC_LOC): Likewise.
> 	(COMBINE_LOCATION_DATA): Likewise.
> 	(SOURCE_LINE): Likewise.
> 	(SOURCE_COLUMN): Likewise.
> 	(LAST_SOURCE_LINE_LOCATION): Likewise.
> 	(LAST_SOURCE_LINE): Likewise.
> 	(LAST_SOURCE_COLUMN): Likewise.
> 	(LAST_SOURCE_LINE_LOCATION)
> 	(INCLUDED_FROM): Likewise.
> 	(MAIN_FILE_P): Likewise.
> 	(LINEMAP_FILE): Likewise.
> 	(LINEMAP_LINE): Likewise.
> 	(LINEMAP_SYSP): Likewise.
> 	(linemap_location_before_p): Likewise.
> 	* line-map.c (linemap_check_files_exited): Make local "map" const.
> 	(linemap_add): Use SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS.
> 	(linemap_line_start): Likewise.
OK.

Jeff

Re: [PATCH 2/4 v2: part 2] libcpp: Replace macro usage with C++ constructs

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 2676 bytes --]

Hi!

On Tue,  5 May 2015 14:21:13 -0400, David Malcolm <dmalcolm@redhat.com> wrote:
> libcpp/ChangeLog:
> 	* include/line-map.h (MAX_SOURCE_LOCATION): Convert from a macro
> 	to a const source_location.
> 	(RESERVED_LOCATION_COUNT): Likewise.
> 	[...]

> --- a/libcpp/include/line-map.h
> +++ b/libcpp/include/line-map.h

>  /* This is the highest possible source location encoded within an
>     ordinary or macro map.  */
> -#define MAX_SOURCE_LOCATION 0x7FFFFFFF
> +const source_location MAX_SOURCE_LOCATION = 0x7FFFFFFF;

>  /* source_location values from 0 to RESERVED_LOCATION_COUNT-1 will
>     be reserved for libcpp user as special values, no token from libcpp
>     will contain any of those locations.  */
> -#define RESERVED_LOCATION_COUNT	2
> +const int RESERVED_LOCATION_COUNT = 2;

That's not quite what the ChangeLog says ;-) -- now committed in r225558,
as obvious:

commit a5ea4da319fa2c7e9dd28e86bb69637a82e8e551
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Wed Jul 8 14:48:11 2015 +0000

    Address -Wsign-compare diagnostics
    
    ... which have been introduced in r223152, for example hundreds of:
    
        [...]/gcc/input.h:37:12: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
    
    	libcpp/
    	* include/line-map.h (RESERVED_LOCATION_COUNT): Change type to
    	source_location.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@225558 138bc75d-0d04-0410-961f-82ee72b054a4
---
 libcpp/ChangeLog          |    5 +++++
 libcpp/include/line-map.h |    2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git libcpp/ChangeLog libcpp/ChangeLog
index 2d2893e..e72b535 100644
--- libcpp/ChangeLog
+++ libcpp/ChangeLog
@@ -1,3 +1,8 @@
+2015-07-08  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* include/line-map.h (RESERVED_LOCATION_COUNT): Change type to
+	source_location.
+
 2015-07-02  Paolo Carlini  <paolo.carlini@oracle.com>
 
 	PR preprocessor/53690
diff --git libcpp/include/line-map.h libcpp/include/line-map.h
index 4155156..bc747c1 100644
--- libcpp/include/line-map.h
+++ libcpp/include/line-map.h
@@ -870,7 +870,7 @@ bool linemap_location_from_macro_expansion_p (const struct line_maps *,
 /* source_location values from 0 to RESERVED_LOCATION_COUNT-1 will
    be reserved for libcpp user as special values, no token from libcpp
    will contain any of those locations.  */
-const int RESERVED_LOCATION_COUNT = 2;
+const source_location RESERVED_LOCATION_COUNT = 2;
 
 /* Converts a map and a source_location to source line.  */
 inline linenum_type


Grüße,
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH 2/4 v2: part 2] libcpp: Replace macro usage with C++ constructs

[David Malcolm]

On Wed, 2015-07-08 at 16:50 +0200, Thomas Schwinge wrote:
> Hi!
> 
> On Tue,  5 May 2015 14:21:13 -0400, David Malcolm <dmalcolm@redhat.com> wrote:
> > libcpp/ChangeLog:
> > 	* include/line-map.h (MAX_SOURCE_LOCATION): Convert from a macro
> > 	to a const source_location.
> > 	(RESERVED_LOCATION_COUNT): Likewise.
> > 	[...]
> 
> > --- a/libcpp/include/line-map.h
> > +++ b/libcpp/include/line-map.h
> 
> >  /* This is the highest possible source location encoded within an
> >     ordinary or macro map.  */
> > -#define MAX_SOURCE_LOCATION 0x7FFFFFFF
> > +const source_location MAX_SOURCE_LOCATION = 0x7FFFFFFF;
> 
> >  /* source_location values from 0 to RESERVED_LOCATION_COUNT-1 will
> >     be reserved for libcpp user as special values, no token from libcpp
> >     will contain any of those locations.  */
> > -#define RESERVED_LOCATION_COUNT	2
> > +const int RESERVED_LOCATION_COUNT = 2;
> 
> That's not quite what the ChangeLog says ;-) -- now committed in r225558,
> as obvious

Oops; thanks!

[PATCH 2/4 v2: part 1] Move linemap_assert higher up within the file

[David Malcolm]

---
 libcpp/include/line-map.h | 71 ++++++++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 35 deletions(-)

diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index e1681ea..7001552 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -279,6 +279,42 @@ struct GTY(()) line_map {
   } GTY((desc ("%1.reason == LC_ENTER_MACRO"))) d;
 };
 
+#if defined ENABLE_CHECKING && (GCC_VERSION >= 2007)
+
+/* Assertion macro to be used in line-map code.  */
+#define linemap_assert(EXPR)                  \
+  do {                                                \
+    if (! (EXPR))                             \
+      abort ();                                       \
+  } while (0)
+
+/* Assert that becomes a conditional expression when checking is disabled at
+   compilation time.  Use this for conditions that should not happen but if
+   they happen, it is better to handle them gracefully rather than crash
+   randomly later.
+   Usage:
+
+   if (linemap_assert_fails(EXPR)) handle_error(); */
+#define linemap_assert_fails(EXPR) __extension__ \
+  ({linemap_assert (EXPR); false;})
+
+/* Assert that MAP encodes locations of tokens that are not part of
+   the replacement-list of a macro expansion.  */
+#define linemap_check_ordinary(LINE_MAP) __extension__		\
+  ({linemap_assert (!linemap_macro_expansion_map_p (LINE_MAP)); \
+    (LINE_MAP);})
+#else
+/* Include EXPR, so that unused variable warnings do not occur.  */
+#define linemap_assert(EXPR) ((void)(0 && (EXPR)))
+#define linemap_assert_fails(EXPR) (! (EXPR))
+#define linemap_check_ordinary(LINE_MAP) (LINE_MAP)
+#endif
+
+/* Return TRUE if MAP encodes locations coming from a macro
+   replacement-list at macro expansion point.  */
+bool
+linemap_macro_expansion_map_p (const struct line_map *);
+
 #define MAP_START_LOCATION(MAP) (MAP)->start_location
 
 #define ORDINARY_MAP_FILE_NAME(MAP) \
@@ -568,10 +604,6 @@ extern const struct line_map *linemap_lookup
    macro expansion, FALSE otherwise.  */
 bool linemap_tracks_macro_expansion_locs_p (struct line_maps *);
 
-/* Return TRUE if MAP encodes locations coming from a macro
-   replacement-list at macro expansion point.  */
-bool linemap_macro_expansion_map_p (const struct line_map *);
-
 /* Return the name of the macro associated to MACRO_MAP.  */
 const char* linemap_map_get_macro_name (const struct line_map*);
 
@@ -638,37 +670,6 @@ bool linemap_location_from_macro_expansion_p (const struct line_maps *,
 #define MAIN_FILE_P(MAP)						\
   ((linemap_check_ordinary (MAP)->d.ordinary.included_from < 0))
 
-#if defined ENABLE_CHECKING && (GCC_VERSION >= 2007)
-
-/* Assertion macro to be used in line-map code.  */
-#define linemap_assert(EXPR)			\
-  do {						\
-    if (! (EXPR))				\
-      abort ();					\
-  } while (0)
- 
-/* Assert that becomes a conditional expression when checking is disabled at
-   compilation time.  Use this for conditions that should not happen but if
-   they happen, it is better to handle them gracefully rather than crash
-   randomly later. 
-   Usage:
-
-   if (linemap_assert_fails(EXPR)) handle_error(); */
-#define linemap_assert_fails(EXPR) __extension__ \
-  ({linemap_assert (EXPR); false;}) 
-
-/* Assert that MAP encodes locations of tokens that are not part of
-   the replacement-list of a macro expansion.  */
-#define linemap_check_ordinary(LINE_MAP) __extension__		\
-  ({linemap_assert (!linemap_macro_expansion_map_p (LINE_MAP)); \
-    (LINE_MAP);})
-#else
-/* Include EXPR, so that unused variable warnings do not occur.  */
-#define linemap_assert(EXPR) ((void)(0 && (EXPR)))
-#define linemap_assert_fails(EXPR) (! (EXPR))
-#define linemap_check_ordinary(LINE_MAP) (LINE_MAP)
-#endif
-
 /* Encode and return a source_location from a column number. The
    source line considered is the last source line used to call
    linemap_line_start, i.e, the last source line which a location was
-- 
1.8.5.3

Re: [PATCH 2/4 v2: part 1] Move linemap_assert higher up within the file

[Jeff Law]

On 05/05/2015 12:21 PM, David Malcolm wrote:
> ---
>   libcpp/include/line-map.h | 71 ++++++++++++++++++++++++-----------------------
>   1 file changed, 36 insertions(+), 35 deletions(-)
Cobble together a quick ChangeLog and test and this is fine.

jeff

[PATCH 5/4] libcpp: Eliminate most of the non-const/reference-returning inline fns

[David Malcolm]

gcc/java/ChangeLog:
	* jcf-parse.c (set_source_filename): Replace write through
	ORDINARY_MAP_FILE_NAME with direct access to "to_file".

libcpp/ChangeLog:
	* include/line-map.h (MAP_START_LOCATION): Eliminate the non-const
	variant, and tweak comment for the const variant.
	(ORDINARY_MAP_STARTING_LINE_NUMBER): Drop the non-const variant.
	(ORDINARY_MAP_INCLUDER_FILE_INDEX): Likewise.
	(ORDINARY_MAP_IN_SYSTEM_HEADER_P): Likewise.
	(SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS): Delete.
	(ORDINARY_MAP_FILE_NAME): Drop the non-const variant.
	(MACRO_MAP_MACRO): Likewise.
	(MACRO_MAP_NUM_MACRO_TOKENS): Likewise.
	(MACRO_MAP_LOCATIONS): Likewise.
	(MACRO_MAP_EXPANSION_POINT_LOCATION): Likewise.
	* line-map.c (linemap_add): Replace writes through macros with
	direct field accesses.
	(linemap_enter_macro): Likewise.
	(linemap_line_start): Likewise.
---
 gcc/java/jcf-parse.c      |  2 +-
 libcpp/include/line-map.h | 86 +----------------------------------------------
 libcpp/line-map.c         | 29 ++++++++--------
 3 files changed, 16 insertions(+), 101 deletions(-)

diff --git a/gcc/java/jcf-parse.c b/gcc/java/jcf-parse.c
index e609331..d1c9fc4 100644
--- a/gcc/java/jcf-parse.c
+++ b/gcc/java/jcf-parse.c
@@ -374,7 +374,7 @@ set_source_filename (JCF *jcf, int index)
     }
       
   sfname = find_sourcefile (sfname);
-  ORDINARY_MAP_FILE_NAME (LINEMAPS_LAST_ORDINARY_MAP (line_table)) = sfname;
+  LINEMAPS_LAST_ORDINARY_MAP (line_table)->to_file = sfname;
   if (current_class == main_class) main_input_filename = sfname;
 }
 
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 95a1d85..0530072 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -343,7 +343,7 @@ linemap_check_macro (const line_map *map)
   return (const line_map_macro *)map;
 }
 
-/* Read the start location of MAP, as an rvalue.  */
+/* Read the start location of MAP.  */
 
 inline source_location
 MAP_START_LOCATION (const line_map *map)
@@ -351,15 +351,6 @@ MAP_START_LOCATION (const line_map *map)
   return map->start_location;
 }
 
-/* Access the start location of MAP as a reference
-   (e.g. as an lvalue).  */
-
-inline source_location&
-MAP_START_LOCATION (line_map *map)
-{
-  return map->start_location;
-}
-
 /* Get the starting line number of ordinary map MAP.  */
 
 inline linenum_type
@@ -368,15 +359,6 @@ ORDINARY_MAP_STARTING_LINE_NUMBER (const line_map_ordinary *ord_map)
   return ord_map->to_line;
 }
 
-/* Access the starting line number of ordinary map MAP by
-   reference (e.g. as an lvalue).  */
-
-inline linenum_type&
-ORDINARY_MAP_STARTING_LINE_NUMBER (line_map_ordinary *ord_map)
-{
-  return ord_map->to_line;
-}
-
 /* Get the index of the ordinary map at whose end
    ordinary map MAP was included.
 
@@ -388,14 +370,6 @@ ORDINARY_MAP_INCLUDER_FILE_INDEX (const line_map_ordinary *ord_map)
   return ord_map->included_from;
 }
 
-/* As above, but by reference (e.g. as an lvalue).  */
-
-inline int&
-ORDINARY_MAP_INCLUDER_FILE_INDEX (line_map_ordinary *ord_map)
-{
-  return ord_map->included_from;
-}
-
 /* Return a positive value if map encodes locations from a system
    header, 0 otherwise. Returns 1 if ordinary map MAP encodes locations
    in a system header and 2 if it encodes locations in a C system header
@@ -407,14 +381,6 @@ ORDINARY_MAP_IN_SYSTEM_HEADER_P (const line_map_ordinary *ord_map)
   return ord_map->sysp;
 }
 
-/* As above, but by reference (e.g. as an lvalue).  */
-
-inline unsigned char &
-ORDINARY_MAP_IN_SYSTEM_HEADER_P (line_map_ordinary *ord_map)
-{
-  return ord_map->sysp;
-}
-
 /* Get the number of the low-order source_location bits used for a
    column number within ordinary map MAP.  */
 
@@ -424,16 +390,6 @@ ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (const line_map_ordinary *ord_map)
   return ord_map->column_bits;
 }
 
-/* Set the number of the low-order source_location bits used for a
-   column number within ordinary map MAP.  */
-
-inline void
-SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (line_map_ordinary *ord_map,
-					int col_bits)
-{
-  ord_map->column_bits = col_bits;
-}
-
 /* Get the filename of ordinary map MAP.  */
 
 inline const char *
@@ -442,14 +398,6 @@ ORDINARY_MAP_FILE_NAME (const line_map_ordinary *ord_map)
   return ord_map->to_file;
 }
 
-/* As above, but by reference (e.g. as an lvalue).  */
-
-inline const char * &
-ORDINARY_MAP_FILE_NAME (line_map_ordinary *ord_map)
-{
-  return ord_map->to_file;
-}
-
 /* Get the cpp macro whose expansion gave birth to macro map MAP.  */
 
 inline cpp_hashnode *
@@ -458,14 +406,6 @@ MACRO_MAP_MACRO (const line_map_macro *macro_map)
   return macro_map->macro;
 }
 
-/* As above, but by reference (e.g. as an lvalue).  */
-
-inline cpp_hashnode * &
-MACRO_MAP_MACRO (line_map_macro *macro_map)
-{
-  return macro_map->macro;
-}
-
 /* Get the number of tokens inside the replacement-list of the macro
    that led to macro map MAP.  */
 
@@ -475,14 +415,6 @@ MACRO_MAP_NUM_MACRO_TOKENS (const line_map_macro *macro_map)
   return macro_map->n_tokens;
 }
 
-/* As above, but by reference (e.g. as an lvalue).  */
-
-inline unsigned int &
-MACRO_MAP_NUM_MACRO_TOKENS (line_map_macro *macro_map)
-{
-  return macro_map->n_tokens;
-}
-
 /* Get the array of pairs of locations within macro map MAP.
    See the declaration of line_map_macro for more information.  */
 
@@ -492,14 +424,6 @@ MACRO_MAP_LOCATIONS (const line_map_macro *macro_map)
   return macro_map->macro_locations;
 }
 
-/* As above, but by reference (e.g. as an lvalue).  */
-
-inline source_location * &
-MACRO_MAP_LOCATIONS (line_map_macro *macro_map)
-{
-  return macro_map->macro_locations;
-}
-
 /* Get the location of the expansion point of the macro map MAP.  */
 
 inline source_location
@@ -508,14 +432,6 @@ MACRO_MAP_EXPANSION_POINT_LOCATION (const line_map_macro *macro_map)
   return macro_map->expansion;
 }
 
-/* As above, but by reference (e.g. as an lvalue).  */
-
-inline source_location &
-MACRO_MAP_EXPANSION_POINT_LOCATION (line_map_macro *macro_map)
-{
-  return macro_map->expansion;
-}
-
 /* The abstraction of a set of location maps. There can be several
    types of location maps. This abstraction contains the attributes
    that are independent from the type of the map.
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index e262df4..a5e45e4 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -381,31 +381,30 @@ linemap_add (struct line_maps *set, enum lc_reason reason,
 	}
     }
 
-  ORDINARY_MAP_IN_SYSTEM_HEADER_P (map) = sysp;
-  MAP_START_LOCATION (map) = start_location;
-  ORDINARY_MAP_FILE_NAME (map) = to_file;
-  ORDINARY_MAP_STARTING_LINE_NUMBER (map) = to_line;
+  map->sysp = sysp;
+  map->start_location = start_location;
+  map->to_file = to_file;
+  map->to_line = to_line;
   LINEMAPS_ORDINARY_CACHE (set) = LINEMAPS_ORDINARY_USED (set) - 1;
-  SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map, 0);
+  map->column_bits = 0;
   set->highest_location = start_location;
   set->highest_line = start_location;
   set->max_column_hint = 0;
 
   if (reason == LC_ENTER)
     {
-      ORDINARY_MAP_INCLUDER_FILE_INDEX (map) = 
+      map->included_from =
 	set->depth == 0 ? -1 : (int) (LINEMAPS_ORDINARY_USED (set) - 2);
       set->depth++;
       if (set->trace_includes)
 	trace_include (set, map);
     }
   else if (reason == LC_RENAME)
-    ORDINARY_MAP_INCLUDER_FILE_INDEX (map) =
-      ORDINARY_MAP_INCLUDER_FILE_INDEX (&map[-1]);
+    map->included_from = ORDINARY_MAP_INCLUDER_FILE_INDEX (&map[-1]);
   else if (reason == LC_LEAVE)
     {
       set->depth--;
-      ORDINARY_MAP_INCLUDER_FILE_INDEX (map) =
+      map->included_from =
 	ORDINARY_MAP_INCLUDER_FILE_INDEX (INCLUDED_FROM (set, map - 1));
     }
 
@@ -464,14 +463,14 @@ linemap_enter_macro (struct line_maps *set, struct cpp_hashnode *macro_node,
 
   map = linemap_check_macro (new_linemap (set, LC_ENTER_MACRO));
 
-  MAP_START_LOCATION (map) = start_location;
-  MACRO_MAP_MACRO (map) = macro_node;
-  MACRO_MAP_NUM_MACRO_TOKENS (map) = num_tokens;
-  MACRO_MAP_LOCATIONS (map)
+  map->start_location = start_location;
+  map->macro = macro_node;
+  map->n_tokens = num_tokens;
+  map->macro_locations
     = (source_location*) reallocator (NULL,
 				      2 * num_tokens
 				      * sizeof (source_location));
-  MACRO_MAP_EXPANSION_POINT_LOCATION (map) = expansion;
+  map->expansion = expansion;
   memset (MACRO_MAP_LOCATIONS (map), 0,
 	  num_tokens * sizeof (source_location));
 
@@ -580,7 +579,7 @@ linemap_line_start (struct line_maps *set, linenum_type to_line,
 				ORDINARY_MAP_IN_SYSTEM_HEADER_P (map),
 				ORDINARY_MAP_FILE_NAME (map),
 				to_line)));
-      SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map, column_bits);
+      map->column_bits = column_bits;
       r = (MAP_START_LOCATION (map)
 	   + ((to_line - ORDINARY_MAP_STARTING_LINE_NUMBER (map))
 	      << column_bits));
-- 
1.8.5.3

Re: [PATCH 5/4] libcpp: Eliminate most of the non-const/reference-returning inline fns

[Jeff Law]

On 05/05/2015 12:21 PM, David Malcolm wrote:
> gcc/java/ChangeLog:
> 	* jcf-parse.c (set_source_filename): Replace write through
> 	ORDINARY_MAP_FILE_NAME with direct access to "to_file".
>
> libcpp/ChangeLog:
> 	* include/line-map.h (MAP_START_LOCATION): Eliminate the non-const
> 	variant, and tweak comment for the const variant.
> 	(ORDINARY_MAP_STARTING_LINE_NUMBER): Drop the non-const variant.
> 	(ORDINARY_MAP_INCLUDER_FILE_INDEX): Likewise.
> 	(ORDINARY_MAP_IN_SYSTEM_HEADER_P): Likewise.
> 	(SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS): Delete.
> 	(ORDINARY_MAP_FILE_NAME): Drop the non-const variant.
> 	(MACRO_MAP_MACRO): Likewise.
> 	(MACRO_MAP_NUM_MACRO_TOKENS): Likewise.
> 	(MACRO_MAP_LOCATIONS): Likewise.
> 	(MACRO_MAP_EXPANSION_POINT_LOCATION): Likewise.
> 	* line-map.c (linemap_add): Replace writes through macros with
> 	direct field accesses.
> 	(linemap_enter_macro): Likewise.
> 	(linemap_line_start): Likewise.
OK.
jeff

Re: [PATCH 2/4 v2] libcpp: Replace macro usage with C++ constructs)

[Jeff Law]

On 05/05/2015 12:21 PM, David Malcolm wrote:
> On Mon, 2015-05-04 at 13:15 -0600, Jeff Law wrote:
> On 05/01/2015 06:56 PM, David Malcolm wrote:
>>>
>>> [I didn't mark the inline functions as "static"; should they be?]
Just a follow-up on this.  I got burned by the ODR issues around 
implicit extern inlines earlier this week.  Basically I had two distinct 
implementations for an inline function with the same name.  They 
obviously appeared in different "header files".

When optimizng, this was fine as they'd actually get inlined and all was 
good.  Things blew up when the optimizer was off.  We got two functions 
with the same name, but different implementations.  The linker blindly 
chose one for me, and in one context it was the wrong one.

This led to bootstrap comparison failures.

So, just be careful :-)

>>
> I moved linemap_assert and linemap_assert_fails higher up within the
> file so that they can appear before the bodies of the inline functions.
> I didn't change their implementation; it's a simple cut-and-paste, so
> these hunks are just an artifact of git's diff-ing algorithm getting
> confused by that.
>
> Should I have called that out in the ChangeLog entries?
It might have helped.  Or you could have pulled it out like you did in 
the follow-up.  No worries.  So consider the issues raised in that code 
as a non-issues for your patch.  They'd be nice things to clean up 
someday though.

>> -#define LINEMAPS_MAPS(SET, MAP_KIND) \
>>> -  (LINEMAPS_MAP_INFO (SET, MAP_KIND))->maps
>>> +inline line_map *
>>> +LINEMAPS_MAPS (const line_maps *set, bool map_kind)
>>> +{
>>> +  return LINEMAPS_MAP_INFO (set, map_kind)->maps;
>>> +}
>>> +
>>> +inline line_map * &
>>> +LINEMAPS_MAPS (line_maps *set, bool map_kind)
>>> +{
>>> +  return LINEMAPS_MAP_INFO (set, map_kind)->maps;
>>> +}
>> Do we really need two implementation of those various functions, one for
>> const line_maps * the other for line_maps *?
>>
>
> The issue here is when they are accessed as lvalues, rather than just as
> rvalues.
>
> I've had a go at eliminating some of the lvalue ones as a followup
> patch: given that after patch 4 these become just field accesses, it's
> trivial to turn the usage sites into plain accesses of fields.
>
> Is that desirable, or are the uses of the macros regarded as useful
> (e.g. for grepping)?  (my preference is for the former: to turn them
> into plain field accesses at those sites)
I think macros would only be useful for those already familiar with the 
code.  If I'm looking for how a particular field gets accessed, I'm 
generally going to be grepping for the field.

Of course, if the field have names that are common ("next" anyone?), 
then, well, grepping for field names is pointless.


>
> Ironically, one of the ones that isn't so easy to eliminate is the
> one you called out above.  Eliminating those ones made the code messier,
> so I kept those ones.
lol.  I just picked one after seeing the pattern repeat a bit.
>
> I'm sending updated patches; the old patch 2 split into 2 parts, and
> a followup ("patch 5 of 4", as it were):
>
>    [PATCH 2/4 v2: part 1] Move linemap_assert higher up within the file
>    [PATCH 2/4 v2: part 2] libcpp: Replace macro usage with C++ constructs
>    [PATCH 5/4] libcpp: Eliminate most of the non-const/reference-returning inline fns
>
> Do I need to do any performance testing of this kit?  Given that the
> resulting inline functions become trivial field lookups in patch 4,
> I'm assuming that our inliner is good enough to optimize it as well
> as the status quo implementation.
Yea, I wouldn't worry about performance of an inline field access vs a 
macro.  We should be good to go.

>
> Also, for comments before functions, do we prefer a blank line between
> the comment and decl, or for them to abutt?
>
> /* Foo.  */
>
> foo ();
>
> vs
>
> /* Bar.  */
> bar ();
>
> How do the following look?
I've done both in my days. I try to follow whatever is in the nearby code.

Let me have a look at the followup...

jeff

Re: [PATCH 2/4 v2] libcpp: Replace macro usage with C++ constructs)

[Jan Hubicka]

> On 05/05/2015 12:21 PM, David Malcolm wrote:
> >On Mon, 2015-05-04 at 13:15 -0600, Jeff Law wrote:
> >On 05/01/2015 06:56 PM, David Malcolm wrote:
> >>>
> >>>[I didn't mark the inline functions as "static"; should they be?]
> Just a follow-up on this.  I got burned by the ODR issues around
> implicit extern inlines earlier this week.  Basically I had two
> distinct implementations for an inline function with the same name.
> They obviously appeared in different "header files".
> 
> When optimizng, this was fine as they'd actually get inlined and all
> was good.  Things blew up when the optimizer was off.  We got two
> functions with the same name, but different implementations.  The
> linker blindly chose one for me, and in one context it was the wrong
> one.
> 
> This led to bootstrap comparison failures.
> 
> So, just be careful :-)

Martin actually tried using ICF to warn about such cases at LTO time (if the
two COMDAT function bodies does not seem to have same implementation, complain
about it). One showstopper here is that early inliner may divere easily (if you
provide inline implementation of called function in one unit), but perhaps we could
output warnings at least in cases we know that both optimization flags and
inline decisions match. (this can be checked during the compare by looking
at DECL_INITIAL block structure)

In GCC I would really love us to switch to non-static inline and just chase
out ODR violations if they exists (hopefully it is not that common). This
makes code reuse more explicit to IPA and hopefully will lead to smaller
binaries.

Honza

[PATCH 1/4] libcpp: Improvements to comments in line-map.h/c

[David Malcolm]

This patch updates and expands some comments in libcpp, adding
a big table to try to clarify what an individual source_location
value can mean.

libcpp/ChangeLog:
	* include/line-map.h: Fix comment at the top of the file.
	(source_location): Rewrite and expand the comment for this
	typedef, adding an ascii-art table to clarify how source_location
	values are allocated.
	* line-map.c: Fix comment at the top of the file.
---
 libcpp/include/line-map.h | 71 +++++++++++++++++++++++++++++++++++++++++++++--
 libcpp/line-map.c         |  2 +-
 2 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 403d798..e1681ea 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -1,4 +1,4 @@
-/* Map logical line numbers to (source file, line number) pairs.
+/* Map (unsigned int) keys to (source file, line, column) triples.
    Copyright (C) 2001-2015 Free Software Foundation, Inc.
 
 This program is free software; you can redistribute it and/or modify it
@@ -46,7 +46,74 @@ enum lc_reason
 /* The type of line numbers.  */
 typedef unsigned int linenum_type;
 
-/* A logical line/column number, i.e. an "index" into a line_map.  */
+/* The typedef "source_location" is a key within the location database,
+   identifying a source location or macro expansion.
+
+   This key only has meaning in relation to a line_maps instance.  Within
+   gcc there is a single line_maps instance: "line_table", declared in
+   gcc/input.h and defined in gcc/input.c.
+
+   The values of the keys are intended to be internal to libcpp,
+   but for ease-of-understanding the implementation, they are currently
+   assigned as follows:
+
+  Actual     | Value                         | Meaning
+  -----------+-------------------------------+-------------------------------
+  0x00000000 |                               | Reserved for use by libcpp
+  0x00000001 | RESERVED_LOCATION_COUNT - 1   | Reserved for use by libcpp
+  -----------+-------------------------------+-------------------------------
+  0x00000002 | RESERVED_LOCATION_COUNT       | The first location to be
+             | (also                         | handed out, and the
+             |  ordmap[0]->start_location)   | first line in ordmap 0
+  -----------+-------------------------------+-------------------------------
+             | ordmap[1]->start_location     | First line in ordmap 1
+             | ordmap[1]->start_location+1   | First column in that line
+             | ordmap[1]->start_location+2   | 2nd column in that line
+             |                               | Subsequent lines are offset by
+             |                               | (1 << column_bits),
+             |                               | e.g. 128 for 7 bits, with a
+             |                               | column value of 0 representing
+             |                               | "the whole line".
+             | ordmap[2]->start_location-1   | Final location in ordmap 1
+  -----------+-------------------------------+-------------------------------
+             | ordmap[2]->start_location     | First line in ordmap 2
+             | ordmap[3]->start_location-1   | Final location in ordmap 2
+  -----------+-------------------------------+-------------------------------
+             |                               | (etc)
+  -----------+-------------------------------+-------------------------------
+             | ordmap[n-1]->start_location   | First line in final ord map
+             |                               | (etc)
+             | set->highest_location - 1     | Final location in that ordmap
+  -----------+-------------------------------+-------------------------------
+             | set->highest_location         | Location of the where the next
+             |                               | ordinary linemap would start
+  -----------+-------------------------------+-------------------------------
+             |                               |
+             |                  VVVVVVVVVVVVVVVVVVVVVVVVVVV
+             |                  Ordinary maps grow this way
+             |
+             |                    (unallocated integers)
+             |
+             |                   Macro maps grow this way
+             |                   ^^^^^^^^^^^^^^^^^^^^^^^^
+             |                               |
+  -----------+-------------------------------+-------------------------------
+             | LINEMAPS_MACRO_LOWEST_LOCATION| Locations within macro maps
+             | macromap[m-1]->start_location | Start of last macro map
+             |                               |
+  -----------+-------------------------------+-------------------------------
+             | macromap[m-2]->start_location | Start of penultimate macro map
+  -----------+-------------------------------+-------------------------------
+             | macromap[1]->start_location   | Start of macro map 1
+  -----------+-------------------------------+-------------------------------
+             | macromap[0]->start_location   | Start of macro map 0
+  0x7fffffff | MAX_SOURCE_LOCATION           |
+  -----------+-------------------------------+-------------------------------
+  0x80000000 | Start of ad-hoc values        |
+  ...        |                               |
+  0xffffffff | UINT_MAX                      |
+  -----------+-------------------------------+-------------------------------
+  .  */
 typedef unsigned int source_location;
 
 /* Memory allocation function typedef.  Works like xrealloc.  */
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index f9260d0..1db59a7 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -1,4 +1,4 @@
-/* Map logical line numbers to (source file, line number) pairs.
+/* Map (unsigned int) keys to (source file, line, column) triples.
    Copyright (C) 2001-2015 Free Software Foundation, Inc.
 
 This program is free software; you can redistribute it and/or modify it
-- 
1.8.5.3

Re: [PATCH 1/4] libcpp: Improvements to comments in line-map.h/c

[Jeff Law]

On 05/01/2015 06:56 PM, David Malcolm wrote:
> This patch updates and expands some comments in libcpp, adding
> a big table to try to clarify what an individual source_location
> value can mean.
>
> libcpp/ChangeLog:
> 	* include/line-map.h: Fix comment at the top of the file.
> 	(source_location): Rewrite and expand the comment for this
> 	typedef, adding an ascii-art table to clarify how source_location
> 	values are allocated.
> 	* line-map.c: Fix comment at the top of the file.
OK.
jeff

[PATCH 3/4] libcpp/input.c: Add a way to visualize the linemaps

[David Malcolm]

As a relative newcomer to GCC, one of the issues I had was
becoming comfortable with the linemap API and its internal
representation.

To familiarize myself with it, I wrote a dumping routine
to try to visualize how the source_location space is carved
up between line maps, and what each number can mean.

It struck me that this would benefit others, so this patch
adds this visualization, via an undocumented option
-fdump-locations, and adds a text file to libcpp's sources
documenting a simple example of compiling a small C file,
with a header and macro expansions (built using the
-fdump-locations option and a little hand-editing).

gcc/ChangeLog:
	* common.opt (fdump-locations): New option.
	* input.c: Include diagnostic-core.h.
	(get_end_location): New function.
	(write_digit): New function.
	(write_digit_row): New function.
	(dump_location_range): New function.
	(dump_labelled_location_range): New function.
	(dump_location_info): New function.
	* input.h (dump_location_info): New prototype.
	* toplev.c (compile_file): Handle flag_dump_locations.

libcpp/ChangeLog:
	* include/line-map.h (source_location): Add a reference to
	location-example.txt to the descriptive comment.
	* location-example.txt: New file.
---
 gcc/common.opt              |   4 +
 gcc/input.c                 | 237 ++++++++++++++++++++++++++++++++++++++++++++
 gcc/input.h                 |   2 +
 gcc/toplev.c                |   3 +
 libcpp/include/line-map.h   |   4 +-
 libcpp/location-example.txt | 216 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 465 insertions(+), 1 deletion(-)
 create mode 100644 libcpp/location-example.txt

diff --git a/gcc/common.opt b/gcc/common.opt
index 380848c..444a73a 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1166,6 +1166,10 @@ Common Driver Var(flag_report_bug)
 Collect and dump debug information into temporary file if ICE in C/C++
 compiler occured.
 
+fdump-locations
+Common Var(flag_dump_locations) Init(0)
+Dump detailed information on GCC's internal representation of source code locations
+
 fdump-passes
 Common Var(flag_dump_passes) Init(0)
 Dump optimization passes
diff --git a/gcc/input.c b/gcc/input.c
index 18c1e50..922c2f1 100644
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "intl.h"
 #include "input.h"
 #include "vec.h"
+#include "diagnostic-core.h"
 
 /* This is a cache used by get_next_line to store the content of a
    file to be searched for file lines.  */
@@ -869,3 +870,239 @@ dump_line_table_statistics (void)
            STAT_LABEL (total_used_map_size));
   fprintf (stderr, "\n");
 }
+
+/* Get location one beyond the final location in ordinary map IDX.  */
+
+static source_location
+get_end_location (struct line_maps *set, unsigned int idx)
+{
+  if (idx == LINEMAPS_ORDINARY_USED (set) - 1)
+    return set->highest_location;
+
+  struct line_map *next_map = LINEMAPS_ORDINARY_MAP_AT (set, idx + 1);
+  return MAP_START_LOCATION (next_map);
+}
+
+/* Helper function for write_digit_row.  */
+
+static void
+write_digit (FILE *stream, int digit)
+{
+  fputc ('0' + (digit % 10), stream);
+}
+
+/* Helper function for dump_location_info.
+   Write a row of numbers to STREAM, numbering a source line,
+   giving the units, tens, hundreds etc of the column number.  */
+
+static void
+write_digit_row (FILE *stream, int indent,
+		 source_location loc, int max_col, int divisor)
+{
+  fprintf (stream, "%*c", indent, ' ');
+  fprintf (stream, "|");
+  for (int column = 1; column < max_col; column++)
+    {
+      source_location column_loc = loc + column;
+      write_digit (stream, column_loc / divisor);
+    }
+  fprintf (stream, "\n");
+}
+
+/* Write a half-closed (START) / half-open (END) interval of
+   source_location to STREAM.  */
+
+static void
+dump_location_range (FILE *stream,
+		     source_location start, source_location end)
+{
+  fprintf (stream,
+	   "  source_location interval: %u <= loc < %u\n",
+	   start, end);
+}
+
+/* Write a labelled description of a half-closed (START) / half-open (END)
+   interval of source_location to STREAM.  */
+
+static void
+dump_labelled_location_range (FILE *stream,
+			      const char *name,
+			      source_location start, source_location end)
+{
+  fprintf (stream, "%s\n", name);
+  dump_location_range (stream, start, end);
+  fprintf (stream, "\n");
+}
+
+/* Write a visualization of the locations in the line_table to STREAM.  */
+
+void
+dump_location_info (FILE *stream)
+{
+  if (0)
+    line_table_dump (stream,
+		     line_table,
+		     LINEMAPS_ORDINARY_USED (line_table),
+		     LINEMAPS_MACRO_USED (line_table));
+
+  /* Visualize the reserved locations.  */
+  dump_labelled_location_range (stream, "RESERVED LOCATIONS",
+				0, RESERVED_LOCATION_COUNT);
+
+  /* Visualize the ordinary line_map instances, rendering the sources. */
+  for (unsigned int idx = 0; idx < LINEMAPS_ORDINARY_USED (line_table); idx++)
+    {
+      source_location end_location = get_end_location (line_table, idx);
+      /* half-closed: doesn't include this one. */
+
+      struct line_map *map = LINEMAPS_ORDINARY_MAP_AT (line_table, idx);
+      fprintf (stream, "ORDINARY MAP: %i\n", idx);
+      dump_location_range (stream,
+			   MAP_START_LOCATION (map), end_location);
+      fprintf (stream, "  file: %s\n", ORDINARY_MAP_FILE_NAME (map));
+      fprintf (stream, "  starting at line: %i\n",
+	       ORDINARY_MAP_STARTING_LINE_NUMBER (map));
+      fprintf (stream, "  column bits: %i\n",
+	       ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map));
+
+      /* Render the span of source lines that this "map" covers.  */
+      for (source_location loc = MAP_START_LOCATION (map);
+	   loc < end_location;
+	   loc++)
+	{
+	  expanded_location exploc
+	    = linemap_expand_location (line_table, map, loc);
+
+	  if (0 == exploc.column)
+	    {
+	      /* Beginning of a new source line: draw the line.  */
+
+	      int line_size;
+	      const char *line_text = location_get_source_line (exploc, &line_size);
+	      if (!line_text)
+		break;
+	      fprintf (stream,
+		       "%s:%3i|loc:%5i|%.*s\n",
+		       exploc.file, exploc.line,
+		       loc,
+		       line_size, line_text);
+
+	      /* "loc" is at column 0, which means "the whole line".
+		 Render the locations *within* the line, by underlining
+		 it, showing the source_location numeric values
+		 at each column.  */
+	      int max_col
+		= (1 << ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map)) - 1;
+	      if (max_col > line_size)
+		max_col = line_size + 1;
+
+	      int indent = 14 + strlen (exploc.file);
+
+	      /* Thousands.  */
+	      if (end_location > 999)
+		write_digit_row (stream, indent, loc, max_col, 1000);
+
+	      /* Hundreds.  */
+	      if (end_location > 99)
+		write_digit_row (stream, indent, loc, max_col, 100);
+
+	      /* Tens.  */
+	      write_digit_row (stream, indent, loc, max_col, 10);
+
+	      /* Units.  */
+	      write_digit_row (stream, indent, loc, max_col, 1);
+	    }
+	}
+      fprintf (stream, "\n");
+    }
+
+  /* Visualize unallocated values.  */
+  dump_labelled_location_range (stream, "UNALLOCATED LOCATIONS",
+				line_table->highest_location,
+				LINEMAPS_MACRO_LOWEST_LOCATION (line_table));
+
+  /* Visualize the macro line_map instances, rendering the sources. */
+  for (unsigned int i = 0; i < LINEMAPS_MACRO_USED (line_table); i++)
+    {
+      /* Each macro map that is allocated owns source_location values
+	 that are *lower* that the one before them.
+	 Hence it's meaningful to view them either in order of ascending
+	 source locations, or in order of ascending macro map index.  */
+      const bool ascending_source_locations = true;
+      unsigned int idx = (ascending_source_locations
+			  ? (LINEMAPS_MACRO_USED (line_table) - (i + 1))
+			  : i);
+      struct line_map *map = LINEMAPS_MACRO_MAP_AT (line_table, idx);
+      fprintf (stream, "MACRO %i: %s (%u tokens)\n",
+	       idx,
+	       linemap_map_get_macro_name (map),
+	       MACRO_MAP_NUM_MACRO_TOKENS (map));
+      dump_location_range (stream,
+			   map->start_location,
+			   (map->start_location
+			    + MACRO_MAP_NUM_MACRO_TOKENS (map)));
+      inform (MACRO_MAP_EXPANSION_POINT_LOCATION (map),
+	      "expansion point is location %i",
+	      MACRO_MAP_EXPANSION_POINT_LOCATION (map));
+      fprintf (stream, "  map->start_location: %u\n",
+	       map->start_location);
+
+      fprintf (stream, "  macro_locations:\n");
+      for (unsigned int i = 0; i < MACRO_MAP_NUM_MACRO_TOKENS (map); i++)
+	{
+	  source_location x = MACRO_MAP_LOCATIONS (map)[2 * i];
+	  source_location y = MACRO_MAP_LOCATIONS (map)[(2 * i) + 1];
+
+	  /* linemap_add_macro_token encodes token numbers in an expansion
+	     by putting them after MAP_START_LOCATION. */
+
+	  /* I'm typically seeing 4 uninitialized entries at the end of
+	     0xafafafaf.
+	     This appears to be due to macro.c:replace_args
+	     adding 2 extra args for padding tokens; presumably there may
+	     be a leading and/or trailing padding token injected,
+	     each for 2 more location slots.
+	     This would explain there being up to 4 source_locations slots
+	     that may be uninitialized.  */
+
+	  fprintf (stream, "    %u: %u, %u\n",
+		   i,
+		   x,
+		   y);
+	  if (x == y)
+	    {
+	      if (x < MAP_START_LOCATION (map))
+		inform (x, "token %u has x-location == y-location == %u", i, x);
+	      else
+		fprintf (stream,
+			 "x-location == y-location == %u encodes token # %u\n",
+			 x, x - MAP_START_LOCATION (map));
+		}
+	  else
+	    {
+	      inform (x, "token %u has x-location == %u", i, x);
+	      inform (x, "token %u has y-location == %u", i, y);
+	    }
+	}
+      fprintf (stream, "\n");
+    }
+
+  /* It appears that MAX_SOURCE_LOCATION itself is never assigned to a
+     macro map, presumably due to an off-by-one error somewhere
+     between the logic in linemap_enter_macro and
+     LINEMAPS_MACRO_LOWEST_LOCATION.  */
+  dump_labelled_location_range (stream, "MAX_SOURCE_LOCATION",
+				MAX_SOURCE_LOCATION,
+				MAX_SOURCE_LOCATION + 1);
+
+  /* Visualize ad-hoc values.  */
+  dump_labelled_location_range (stream, "AD-HOC LOCATIONS",
+				MAX_SOURCE_LOCATION + 1, UINT_MAX);
+
+  /* A brute-force visualization: emit a warning at every location.  */
+  if (0)
+    for (source_location loc = 0; loc < line_table->highest_location; loc++)
+      warning_at (loc, 0, "this is location %i", loc);
+      /* Alternatively, we could use inform (), though this
+	 also shows lots of locations in stdc-predef.h */
+}
diff --git a/gcc/input.h b/gcc/input.h
index 93eb6ed..5ba4d3b 100644
--- a/gcc/input.h
+++ b/gcc/input.h
@@ -77,6 +77,8 @@ extern location_t input_location;
 
 void dump_line_table_statistics (void);
 
+void dump_location_info (FILE *stream);
+
 void diagnostics_file_cache_fini (void);
 
 #endif
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 38de36b..fe86c7e 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -593,6 +593,9 @@ compile_file (void)
   timevar_pop (TV_PARSE_GLOBAL);
   timevar_stop (TV_PHASE_PARSING);
 
+  if (flag_dump_locations)
+    dump_location_info (stderr);
+
   /* Compilation is now finished except for writing
      what's left of the symbol table output.  */
 
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 021ca6e..9dfc396 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -113,7 +113,9 @@ typedef unsigned int linenum_type;
   ...        |                               |
   0xffffffff | UINT_MAX                      |
   -----------+-------------------------------+-------------------------------
-  .  */
+
+  To see how this works in practice, see the worked example in
+  libcpp/location-example.txt.  */
 typedef unsigned int source_location;
 
 /* Memory allocation function typedef.  Works like xrealloc.  */
diff --git a/libcpp/location-example.txt b/libcpp/location-example.txt
new file mode 100644
index 0000000..be53175
--- /dev/null
+++ b/libcpp/location-example.txt
@@ -0,0 +1,216 @@
+Consider compiling test.c, with this content:
+VVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVV
+#include "test.h"
+
+int
+main (int argc, char **argv)
+{
+  int a = PLUS (1,2);
+  int b = PLUS (3,4);
+  return 0;
+}
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+...where test.h has this content:
+VVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVV
+extern int foo ();
+
+#define PLUS(A, B) A + B
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+
+The undocumented -fdump-locations option outputs this information to stderr,
+showing what each source_location value means.  Source code lines are quoted,
+showing both per-line source_location values and per-line&column
+source_location values (written vertically under the corresponding character
+of source code).
+
+VVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVV
+RESERVED LOCATIONS
+  source_location interval: 0 <= loc < 2
+
+ORDINARY MAP: 0
+  source_location interval: 2 <= loc < 3
+  file: test.c
+  starting at line: 1
+  column bits: 7
+test.c:  1|loc:    2|#include "test.h"
+                    |00000001111111111
+                    |34567890123456789
+
+ORDINARY MAP: 1
+  source_location interval: 3 <= loc < 4
+  file: <built-in>
+  starting at line: 0
+  column bits: 0
+
+ORDINARY MAP: 2
+  source_location interval: 4 <= loc < 5
+  file: <command-line>
+  starting at line: 0
+  column bits: 0
+
+ORDINARY MAP: 3
+  source_location interval: 5 <= loc < 5005
+  file: /usr/include/stdc-predef.h
+  starting at line: 1
+  column bits: 7
+(contents of /usr/include/stdc-predef.h snipped for brevity)
+
+ORDINARY MAP: 4
+  source_location interval: 5005 <= loc < 5006
+  file: <command-line>
+  starting at line: 1
+  column bits: 7
+
+ORDINARY MAP: 5
+  source_location interval: 5006 <= loc < 5134
+  file: test.c
+  starting at line: 1
+  column bits: 7
+test.c:  1|loc: 5006|#include "test.h"
+                    |55555555555555555
+                    |00000000000000000
+                    |00011111111112222
+                    |78901234567890123
+
+ORDINARY MAP: 6
+  source_location interval: 5134 <= loc < 5416
+  file: test.h
+  starting at line: 1
+  column bits: 7
+test.h:  1|loc: 5134|extern int foo ();
+                    |555555555555555555
+                    |111111111111111111
+                    |333334444444444555
+                    |567890123456789012
+test.h:  2|loc: 5262|
+                    |
+                    |
+                    |
+                    |
+test.h:  3|loc: 5390|#define PLUS(A, B) A + B
+                    |555555555555555555555555
+                    |333333333444444444444444
+                    |999999999000000000011111
+                    |123456789012345678901234
+
+ORDINARY MAP: 7
+  source_location interval: 5416 <= loc < 6314
+  file: test.c
+  starting at line: 2
+  column bits: 7
+test.c:  2|loc: 5416|
+                    |
+                    |
+                    |
+                    |
+test.c:  3|loc: 5544|int
+                    |555
+                    |555
+                    |444
+                    |567
+test.c:  4|loc: 5672|main (int argc, char **argv)
+                    |5555555555555555555555555555
+                    |6666666666666666666666666667
+                    |7777777888888888899999999990
+                    |3456789012345678901234567890
+test.c:  5|loc: 5800|{
+                    |5
+                    |8
+                    |0
+                    |1
+test.c:  6|loc: 5928|  int a = PLUS (1,2);
+                    |555555555555555555555
+                    |999999999999999999999
+                    |233333333334444444444
+                    |901234567890123456789
+test.c:  7|loc: 6056|  int b = PLUS (3,4);
+                    |666666666666666666666
+                    |000000000000000000000
+                    |555666666666677777777
+                    |789012345678901234567
+test.c:  8|loc: 6184|  return 0;
+                    |66666666666
+                    |11111111111
+                    |88888999999
+                    |56789012345
+test.c:  9|loc: 6312|}
+                    |6
+                    |3
+                    |1
+                    |3
+
+UNALLOCATED LOCATIONS
+  source_location interval: 6314 <= loc < 2147483633
+
+MACRO 1: PLUS (7 tokens)
+  source_location interval: 2147483633 <= loc < 2147483640
+test.c:7:11: note: expansion point is location 6067
+   int b = PLUS (3,4);
+           ^
+  map->start_location: 2147483633
+  macro_locations:
+    0: 6073, 5410
+test.c:7:17: note: token 0 has x-location == 6073
+   int b = PLUS (3,4);
+                 ^
+test.c:7:17: note: token 0 has y-location == 5410
+    1: 5412, 5412
+In file included from test.c:1:0:
+test.h:3:22: note: token 1 has x-location == y-location == 5412
+ #define PLUS(A, B) A + B
+                      ^
+    2: 6075, 5414
+test.c:7:19: note: token 2 has x-location == 6075
+   int b = PLUS (3,4);
+                   ^
+test.c:7:19: note: token 2 has y-location == 5414
+    3: 0, 2947526575
+cc1: note: token 3 has x-location == 0
+cc1: note: token 3 has y-location == 2947526575
+    4: 2947526575, 2947526575
+x-location == y-location == 2947526575 encodes token # 800042942
+    5: 2947526575, 2947526575
+x-location == y-location == 2947526575 encodes token # 800042942
+    6: 2947526575, 2947526575
+x-location == y-location == 2947526575 encodes token # 800042942
+
+MACRO 0: PLUS (7 tokens)
+  source_location interval: 2147483640 <= loc < 2147483647
+test.c:6:11: note: expansion point is location 5939
+   int a = PLUS (1,2);
+           ^
+  map->start_location: 2147483640
+  macro_locations:
+    0: 5945, 5410
+test.c:6:17: note: token 0 has x-location == 5945
+   int a = PLUS (1,2);
+                 ^
+test.c:6:17: note: token 0 has y-location == 5410
+    1: 5412, 5412
+In file included from test.c:1:0:
+test.h:3:22: note: token 1 has x-location == y-location == 5412
+ #define PLUS(A, B) A + B
+                      ^
+    2: 5947, 5414
+test.c:6:19: note: token 2 has x-location == 5947
+   int a = PLUS (1,2);
+                   ^
+test.c:6:19: note: token 2 has y-location == 5414
+    3: 0, 2947526575
+cc1: note: token 3 has x-location == 0
+cc1: note: token 3 has y-location == 2947526575
+    4: 2947526575, 2947526575
+x-location == y-location == 2947526575 encodes token # 800042935
+    5: 2947526575, 2947526575
+x-location == y-location == 2947526575 encodes token # 800042935
+    6: 2947526575, 2947526575
+x-location == y-location == 2947526575 encodes token # 800042935
+
+MAX_SOURCE_LOCATION
+  source_location interval: 2147483647 <= loc < 2147483648
+
+AD-HOC LOCATIONS
+  source_location interval: 2147483648 <= loc < 4294967295
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-- 
1.8.5.3

Re: [PATCH 3/4] libcpp/input.c: Add a way to visualize the linemaps

[Jeff Law]

On 05/01/2015 06:56 PM, David Malcolm wrote:
> As a relative newcomer to GCC, one of the issues I had was
> becoming comfortable with the linemap API and its internal
> representation.
>
> To familiarize myself with it, I wrote a dumping routine
> to try to visualize how the source_location space is carved
> up between line maps, and what each number can mean.
>
> It struck me that this would benefit others, so this patch
> adds this visualization, via an undocumented option
> -fdump-locations, and adds a text file to libcpp's sources
> documenting a simple example of compiling a small C file,
> with a header and macro expansions (built using the
> -fdump-locations option and a little hand-editing).
>
> gcc/ChangeLog:
> 	* common.opt (fdump-locations): New option.
> 	* input.c: Include diagnostic-core.h.
> 	(get_end_location): New function.
> 	(write_digit): New function.
> 	(write_digit_row): New function.
> 	(dump_location_range): New function.
> 	(dump_labelled_location_range): New function.
> 	(dump_location_info): New function.
> 	* input.h (dump_location_info): New prototype.
> 	* toplev.c (compile_file): Handle flag_dump_locations.
>
> libcpp/ChangeLog:
> 	* include/line-map.h (source_location): Add a reference to
> 	location-example.txt to the descriptive comment.
> 	* location-example.txt: New file.
Maybe "dump-internal-locations"?  Not sure I want to bikeshed on the 
name any more than that.   If you feel strongly about the option name, 
then I won't stress about it.



> +void
> +dump_location_info (FILE *stream)
> +{
> +  if (0)
> +    line_table_dump (stream,
> +		     line_table,
> +		     LINEMAPS_ORDINARY_USED (line_table),
> +		     LINEMAPS_MACRO_USED (line_table));
Should the if (0) code go away?

> +
> +  /* A brute-force visualization: emit a warning at every location.  */
> +  if (0)
> +    for (source_location loc = 0; loc < line_table->highest_location; loc++)
> +      warning_at (loc, 0, "this is location %i", loc);
> +      /* Alternatively, we could use inform (), though this
> +	 also shows lots of locations in stdc-predef.h */
And again.


So I think with removing the if (0) code and the possible option name 
change this is good to go.

Jeff

Re: [PATCH 3/4] libcpp/input.c: Add a way to visualize the linemaps

[David Malcolm]

[-- Attachment #1: Type: text/plain, Size: 2565 bytes --]

On Mon, 2015-05-04 at 13:19 -0600, Jeff Law wrote:
> On 05/01/2015 06:56 PM, David Malcolm wrote:
> > As a relative newcomer to GCC, one of the issues I had was
> > becoming comfortable with the linemap API and its internal
> > representation.
> >
> > To familiarize myself with it, I wrote a dumping routine
> > to try to visualize how the source_location space is carved
> > up between line maps, and what each number can mean.
> >
> > It struck me that this would benefit others, so this patch
> > adds this visualization, via an undocumented option
> > -fdump-locations, and adds a text file to libcpp's sources
> > documenting a simple example of compiling a small C file,
> > with a header and macro expansions (built using the
> > -fdump-locations option and a little hand-editing).
> >
> > gcc/ChangeLog:
> > 	* common.opt (fdump-locations): New option.
> > 	* input.c: Include diagnostic-core.h.
> > 	(get_end_location): New function.
> > 	(write_digit): New function.
> > 	(write_digit_row): New function.
> > 	(dump_location_range): New function.
> > 	(dump_labelled_location_range): New function.
> > 	(dump_location_info): New function.
> > 	* input.h (dump_location_info): New prototype.
> > 	* toplev.c (compile_file): Handle flag_dump_locations.
> >
> > libcpp/ChangeLog:
> > 	* include/line-map.h (source_location): Add a reference to
> > 	location-example.txt to the descriptive comment.
> > 	* location-example.txt: New file.
> Maybe "dump-internal-locations"?  Not sure I want to bikeshed on the 
> name any more than that.   If you feel strongly about the option name, 
> then I won't stress about it.
> 
> 
> 
> > +void
> > +dump_location_info (FILE *stream)
> > +{
> > +  if (0)
> > +    line_table_dump (stream,
> > +		     line_table,
> > +		     LINEMAPS_ORDINARY_USED (line_table),
> > +		     LINEMAPS_MACRO_USED (line_table));
> Should the if (0) code go away?
> 
> > +
> > +  /* A brute-force visualization: emit a warning at every location.  */
> > +  if (0)
> > +    for (source_location loc = 0; loc < line_table->highest_location; loc++)
> > +      warning_at (loc, 0, "this is location %i", loc);
> > +      /* Alternatively, we could use inform (), though this
> > +	 also shows lots of locations in stdc-predef.h */
> And again.
> 
> 
> So I think with removing the if (0) code and the possible option name 
> change this is good to go.

Thanks.  I removed the if (0) code and renamed it to
-fdump-internal-locations.

Bootstrapped&regrtested on x86_64-unknown-linux-gnu (Fedora 20).

Committed to trunk as r223163 (attached).


[-- Attachment #2: r223163.patch --]
[-- Type: text/x-patch, Size: 18256 bytes --]

Index: gcc/input.c
===================================================================
--- gcc/input.c	(revision 223162)
+++ gcc/input.c	(revision 223163)
@@ -23,6 +23,7 @@
 #include "intl.h"
 #include "input.h"
 #include "vec.h"
+#include "diagnostic-core.h"
 
 /* This is a cache used by get_next_line to store the content of a
    file to be searched for file lines.  */
@@ -869,3 +870,226 @@
            STAT_LABEL (total_used_map_size));
   fprintf (stderr, "\n");
 }
+
+/* Get location one beyond the final location in ordinary map IDX.  */
+
+static source_location
+get_end_location (struct line_maps *set, unsigned int idx)
+{
+  if (idx == LINEMAPS_ORDINARY_USED (set) - 1)
+    return set->highest_location;
+
+  struct line_map *next_map = LINEMAPS_ORDINARY_MAP_AT (set, idx + 1);
+  return MAP_START_LOCATION (next_map);
+}
+
+/* Helper function for write_digit_row.  */
+
+static void
+write_digit (FILE *stream, int digit)
+{
+  fputc ('0' + (digit % 10), stream);
+}
+
+/* Helper function for dump_location_info.
+   Write a row of numbers to STREAM, numbering a source line,
+   giving the units, tens, hundreds etc of the column number.  */
+
+static void
+write_digit_row (FILE *stream, int indent,
+		 source_location loc, int max_col, int divisor)
+{
+  fprintf (stream, "%*c", indent, ' ');
+  fprintf (stream, "|");
+  for (int column = 1; column < max_col; column++)
+    {
+      source_location column_loc = loc + column;
+      write_digit (stream, column_loc / divisor);
+    }
+  fprintf (stream, "\n");
+}
+
+/* Write a half-closed (START) / half-open (END) interval of
+   source_location to STREAM.  */
+
+static void
+dump_location_range (FILE *stream,
+		     source_location start, source_location end)
+{
+  fprintf (stream,
+	   "  source_location interval: %u <= loc < %u\n",
+	   start, end);
+}
+
+/* Write a labelled description of a half-closed (START) / half-open (END)
+   interval of source_location to STREAM.  */
+
+static void
+dump_labelled_location_range (FILE *stream,
+			      const char *name,
+			      source_location start, source_location end)
+{
+  fprintf (stream, "%s\n", name);
+  dump_location_range (stream, start, end);
+  fprintf (stream, "\n");
+}
+
+/* Write a visualization of the locations in the line_table to STREAM.  */
+
+void
+dump_location_info (FILE *stream)
+{
+  /* Visualize the reserved locations.  */
+  dump_labelled_location_range (stream, "RESERVED LOCATIONS",
+				0, RESERVED_LOCATION_COUNT);
+
+  /* Visualize the ordinary line_map instances, rendering the sources. */
+  for (unsigned int idx = 0; idx < LINEMAPS_ORDINARY_USED (line_table); idx++)
+    {
+      source_location end_location = get_end_location (line_table, idx);
+      /* half-closed: doesn't include this one. */
+
+      struct line_map *map = LINEMAPS_ORDINARY_MAP_AT (line_table, idx);
+      fprintf (stream, "ORDINARY MAP: %i\n", idx);
+      dump_location_range (stream,
+			   MAP_START_LOCATION (map), end_location);
+      fprintf (stream, "  file: %s\n", ORDINARY_MAP_FILE_NAME (map));
+      fprintf (stream, "  starting at line: %i\n",
+	       ORDINARY_MAP_STARTING_LINE_NUMBER (map));
+      fprintf (stream, "  column bits: %i\n",
+	       ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map));
+
+      /* Render the span of source lines that this "map" covers.  */
+      for (source_location loc = MAP_START_LOCATION (map);
+	   loc < end_location;
+	   loc++)
+	{
+	  expanded_location exploc
+	    = linemap_expand_location (line_table, map, loc);
+
+	  if (0 == exploc.column)
+	    {
+	      /* Beginning of a new source line: draw the line.  */
+
+	      int line_size;
+	      const char *line_text = location_get_source_line (exploc, &line_size);
+	      if (!line_text)
+		break;
+	      fprintf (stream,
+		       "%s:%3i|loc:%5i|%.*s\n",
+		       exploc.file, exploc.line,
+		       loc,
+		       line_size, line_text);
+
+	      /* "loc" is at column 0, which means "the whole line".
+		 Render the locations *within* the line, by underlining
+		 it, showing the source_location numeric values
+		 at each column.  */
+	      int max_col
+		= (1 << ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map)) - 1;
+	      if (max_col > line_size)
+		max_col = line_size + 1;
+
+	      int indent = 14 + strlen (exploc.file);
+
+	      /* Thousands.  */
+	      if (end_location > 999)
+		write_digit_row (stream, indent, loc, max_col, 1000);
+
+	      /* Hundreds.  */
+	      if (end_location > 99)
+		write_digit_row (stream, indent, loc, max_col, 100);
+
+	      /* Tens.  */
+	      write_digit_row (stream, indent, loc, max_col, 10);
+
+	      /* Units.  */
+	      write_digit_row (stream, indent, loc, max_col, 1);
+	    }
+	}
+      fprintf (stream, "\n");
+    }
+
+  /* Visualize unallocated values.  */
+  dump_labelled_location_range (stream, "UNALLOCATED LOCATIONS",
+				line_table->highest_location,
+				LINEMAPS_MACRO_LOWEST_LOCATION (line_table));
+
+  /* Visualize the macro line_map instances, rendering the sources. */
+  for (unsigned int i = 0; i < LINEMAPS_MACRO_USED (line_table); i++)
+    {
+      /* Each macro map that is allocated owns source_location values
+	 that are *lower* that the one before them.
+	 Hence it's meaningful to view them either in order of ascending
+	 source locations, or in order of ascending macro map index.  */
+      const bool ascending_source_locations = true;
+      unsigned int idx = (ascending_source_locations
+			  ? (LINEMAPS_MACRO_USED (line_table) - (i + 1))
+			  : i);
+      struct line_map *map = LINEMAPS_MACRO_MAP_AT (line_table, idx);
+      fprintf (stream, "MACRO %i: %s (%u tokens)\n",
+	       idx,
+	       linemap_map_get_macro_name (map),
+	       MACRO_MAP_NUM_MACRO_TOKENS (map));
+      dump_location_range (stream,
+			   map->start_location,
+			   (map->start_location
+			    + MACRO_MAP_NUM_MACRO_TOKENS (map)));
+      inform (MACRO_MAP_EXPANSION_POINT_LOCATION (map),
+	      "expansion point is location %i",
+	      MACRO_MAP_EXPANSION_POINT_LOCATION (map));
+      fprintf (stream, "  map->start_location: %u\n",
+	       map->start_location);
+
+      fprintf (stream, "  macro_locations:\n");
+      for (unsigned int i = 0; i < MACRO_MAP_NUM_MACRO_TOKENS (map); i++)
+	{
+	  source_location x = MACRO_MAP_LOCATIONS (map)[2 * i];
+	  source_location y = MACRO_MAP_LOCATIONS (map)[(2 * i) + 1];
+
+	  /* linemap_add_macro_token encodes token numbers in an expansion
+	     by putting them after MAP_START_LOCATION. */
+
+	  /* I'm typically seeing 4 uninitialized entries at the end of
+	     0xafafafaf.
+	     This appears to be due to macro.c:replace_args
+	     adding 2 extra args for padding tokens; presumably there may
+	     be a leading and/or trailing padding token injected,
+	     each for 2 more location slots.
+	     This would explain there being up to 4 source_locations slots
+	     that may be uninitialized.  */
+
+	  fprintf (stream, "    %u: %u, %u\n",
+		   i,
+		   x,
+		   y);
+	  if (x == y)
+	    {
+	      if (x < MAP_START_LOCATION (map))
+		inform (x, "token %u has x-location == y-location == %u", i, x);
+	      else
+		fprintf (stream,
+			 "x-location == y-location == %u encodes token # %u\n",
+			 x, x - MAP_START_LOCATION (map));
+		}
+	  else
+	    {
+	      inform (x, "token %u has x-location == %u", i, x);
+	      inform (x, "token %u has y-location == %u", i, y);
+	    }
+	}
+      fprintf (stream, "\n");
+    }
+
+  /* It appears that MAX_SOURCE_LOCATION itself is never assigned to a
+     macro map, presumably due to an off-by-one error somewhere
+     between the logic in linemap_enter_macro and
+     LINEMAPS_MACRO_LOWEST_LOCATION.  */
+  dump_labelled_location_range (stream, "MAX_SOURCE_LOCATION",
+				MAX_SOURCE_LOCATION,
+				MAX_SOURCE_LOCATION + 1);
+
+  /* Visualize ad-hoc values.  */
+  dump_labelled_location_range (stream, "AD-HOC LOCATIONS",
+				MAX_SOURCE_LOCATION + 1, UINT_MAX);
+}
Index: gcc/input.h
===================================================================
--- gcc/input.h	(revision 223162)
+++ gcc/input.h	(revision 223163)
@@ -77,6 +77,8 @@
 
 void dump_line_table_statistics (void);
 
+void dump_location_info (FILE *stream);
+
 void diagnostics_file_cache_fini (void);
 
 #endif
Index: gcc/toplev.c
===================================================================
--- gcc/toplev.c	(revision 223162)
+++ gcc/toplev.c	(revision 223163)
@@ -593,6 +593,9 @@
   timevar_pop (TV_PARSE_GLOBAL);
   timevar_stop (TV_PHASE_PARSING);
 
+  if (flag_dump_locations)
+    dump_location_info (stderr);
+
   /* Compilation is now finished except for writing
      what's left of the symbol table output.  */
 
Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 223162)
+++ gcc/ChangeLog	(revision 223163)
@@ -1,3 +1,16 @@
+2015-05-13  David Malcolm  <dmalcolm@redhat.com>
+
+	* common.opt (fdump-internal-locations): New option.
+	* input.c: Include diagnostic-core.h.
+	(get_end_location): New function.
+	(write_digit): New function.
+	(write_digit_row): New function.
+	(dump_location_range): New function.
+	(dump_labelled_location_range): New function.
+	(dump_location_info): New function.
+	* input.h (dump_location_info): New prototype.
+	* toplev.c (compile_file): Handle flag_dump_locations.
+
 2015-05-13  Eric Botcazou  <ebotcazou@adacore.com>
 
 	* gimple-expr.h (is_gimple_constant): Reorder.
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 223162)
+++ gcc/common.opt	(revision 223163)
@@ -1166,6 +1166,10 @@
 Collect and dump debug information into temporary file if ICE in C/C++
 compiler occured.
 
+fdump-internal-locations
+Common Var(flag_dump_locations) Init(0)
+Dump detailed information on GCC's internal representation of source code locations
+
 fdump-passes
 Common Var(flag_dump_passes) Init(0)
 Dump optimization passes
Index: libcpp/location-example.txt
===================================================================
--- libcpp/location-example.txt	(revision 0)
+++ libcpp/location-example.txt	(revision 223163)
@@ -0,0 +1,216 @@
+Consider compiling test.c, with this content:
+VVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVV
+#include "test.h"
+
+int
+main (int argc, char **argv)
+{
+  int a = PLUS (1,2);
+  int b = PLUS (3,4);
+  return 0;
+}
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+...where test.h has this content:
+VVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVV
+extern int foo ();
+
+#define PLUS(A, B) A + B
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+
+The undocumented -fdump-internal-locations option outputs this information
+to stderr, showing what each source_location value means.  Source code
+lines are quoted, showing both per-line source_location values and
+per-line&column source_location values (written vertically under the
+corresponding character of source code).
+
+VVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVV
+RESERVED LOCATIONS
+  source_location interval: 0 <= loc < 2
+
+ORDINARY MAP: 0
+  source_location interval: 2 <= loc < 3
+  file: test.c
+  starting at line: 1
+  column bits: 7
+test.c:  1|loc:    2|#include "test.h"
+                    |00000001111111111
+                    |34567890123456789
+
+ORDINARY MAP: 1
+  source_location interval: 3 <= loc < 4
+  file: <built-in>
+  starting at line: 0
+  column bits: 0
+
+ORDINARY MAP: 2
+  source_location interval: 4 <= loc < 5
+  file: <command-line>
+  starting at line: 0
+  column bits: 0
+
+ORDINARY MAP: 3
+  source_location interval: 5 <= loc < 5005
+  file: /usr/include/stdc-predef.h
+  starting at line: 1
+  column bits: 7
+(contents of /usr/include/stdc-predef.h snipped for brevity)
+
+ORDINARY MAP: 4
+  source_location interval: 5005 <= loc < 5006
+  file: <command-line>
+  starting at line: 1
+  column bits: 7
+
+ORDINARY MAP: 5
+  source_location interval: 5006 <= loc < 5134
+  file: test.c
+  starting at line: 1
+  column bits: 7
+test.c:  1|loc: 5006|#include "test.h"
+                    |55555555555555555
+                    |00000000000000000
+                    |00011111111112222
+                    |78901234567890123
+
+ORDINARY MAP: 6
+  source_location interval: 5134 <= loc < 5416
+  file: test.h
+  starting at line: 1
+  column bits: 7
+test.h:  1|loc: 5134|extern int foo ();
+                    |555555555555555555
+                    |111111111111111111
+                    |333334444444444555
+                    |567890123456789012
+test.h:  2|loc: 5262|
+                    |
+                    |
+                    |
+                    |
+test.h:  3|loc: 5390|#define PLUS(A, B) A + B
+                    |555555555555555555555555
+                    |333333333444444444444444
+                    |999999999000000000011111
+                    |123456789012345678901234
+
+ORDINARY MAP: 7
+  source_location interval: 5416 <= loc < 6314
+  file: test.c
+  starting at line: 2
+  column bits: 7
+test.c:  2|loc: 5416|
+                    |
+                    |
+                    |
+                    |
+test.c:  3|loc: 5544|int
+                    |555
+                    |555
+                    |444
+                    |567
+test.c:  4|loc: 5672|main (int argc, char **argv)
+                    |5555555555555555555555555555
+                    |6666666666666666666666666667
+                    |7777777888888888899999999990
+                    |3456789012345678901234567890
+test.c:  5|loc: 5800|{
+                    |5
+                    |8
+                    |0
+                    |1
+test.c:  6|loc: 5928|  int a = PLUS (1,2);
+                    |555555555555555555555
+                    |999999999999999999999
+                    |233333333334444444444
+                    |901234567890123456789
+test.c:  7|loc: 6056|  int b = PLUS (3,4);
+                    |666666666666666666666
+                    |000000000000000000000
+                    |555666666666677777777
+                    |789012345678901234567
+test.c:  8|loc: 6184|  return 0;
+                    |66666666666
+                    |11111111111
+                    |88888999999
+                    |56789012345
+test.c:  9|loc: 6312|}
+                    |6
+                    |3
+                    |1
+                    |3
+
+UNALLOCATED LOCATIONS
+  source_location interval: 6314 <= loc < 2147483633
+
+MACRO 1: PLUS (7 tokens)
+  source_location interval: 2147483633 <= loc < 2147483640
+test.c:7:11: note: expansion point is location 6067
+   int b = PLUS (3,4);
+           ^
+  map->start_location: 2147483633
+  macro_locations:
+    0: 6073, 5410
+test.c:7:17: note: token 0 has x-location == 6073
+   int b = PLUS (3,4);
+                 ^
+test.c:7:17: note: token 0 has y-location == 5410
+    1: 5412, 5412
+In file included from test.c:1:0:
+test.h:3:22: note: token 1 has x-location == y-location == 5412
+ #define PLUS(A, B) A + B
+                      ^
+    2: 6075, 5414
+test.c:7:19: note: token 2 has x-location == 6075
+   int b = PLUS (3,4);
+                   ^
+test.c:7:19: note: token 2 has y-location == 5414
+    3: 0, 2947526575
+cc1: note: token 3 has x-location == 0
+cc1: note: token 3 has y-location == 2947526575
+    4: 2947526575, 2947526575
+x-location == y-location == 2947526575 encodes token # 800042942
+    5: 2947526575, 2947526575
+x-location == y-location == 2947526575 encodes token # 800042942
+    6: 2947526575, 2947526575
+x-location == y-location == 2947526575 encodes token # 800042942
+
+MACRO 0: PLUS (7 tokens)
+  source_location interval: 2147483640 <= loc < 2147483647
+test.c:6:11: note: expansion point is location 5939
+   int a = PLUS (1,2);
+           ^
+  map->start_location: 2147483640
+  macro_locations:
+    0: 5945, 5410
+test.c:6:17: note: token 0 has x-location == 5945
+   int a = PLUS (1,2);
+                 ^
+test.c:6:17: note: token 0 has y-location == 5410
+    1: 5412, 5412
+In file included from test.c:1:0:
+test.h:3:22: note: token 1 has x-location == y-location == 5412
+ #define PLUS(A, B) A + B
+                      ^
+    2: 5947, 5414
+test.c:6:19: note: token 2 has x-location == 5947
+   int a = PLUS (1,2);
+                   ^
+test.c:6:19: note: token 2 has y-location == 5414
+    3: 0, 2947526575
+cc1: note: token 3 has x-location == 0
+cc1: note: token 3 has y-location == 2947526575
+    4: 2947526575, 2947526575
+x-location == y-location == 2947526575 encodes token # 800042935
+    5: 2947526575, 2947526575
+x-location == y-location == 2947526575 encodes token # 800042935
+    6: 2947526575, 2947526575
+x-location == y-location == 2947526575 encodes token # 800042935
+
+MAX_SOURCE_LOCATION
+  source_location interval: 2147483647 <= loc < 2147483648
+
+AD-HOC LOCATIONS
+  source_location interval: 2147483648 <= loc < 4294967295
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Index: libcpp/include/line-map.h
===================================================================
--- libcpp/include/line-map.h	(revision 223162)
+++ libcpp/include/line-map.h	(revision 223163)
@@ -113,7 +113,9 @@
   ...        |                               |
   0xffffffff | UINT_MAX                      |
   -----------+-------------------------------+-------------------------------
-  .  */
+
+  To see how this works in practice, see the worked example in
+  libcpp/location-example.txt.  */
 typedef unsigned int source_location;
 
 /* Memory allocation function typedef.  Works like xrealloc.  */
Index: libcpp/ChangeLog
===================================================================
--- libcpp/ChangeLog	(revision 223162)
+++ libcpp/ChangeLog	(revision 223163)
@@ -1,5 +1,11 @@
 2015-05-13  David Malcolm  <dmalcolm@redhat.com>
 
+	* include/line-map.h (source_location): Add a reference to
+	location-example.txt to the descriptive comment.
+	* location-example.txt: New file.
+
+2015-05-13  David Malcolm  <dmalcolm@redhat.com>
+
 	* include/line-map.h (MAX_SOURCE_LOCATION): Convert from a macro
 	to a const source_location.
 	(RESERVED_LOCATION_COUNT): Likewise.

[PATCH 4/4] Replace line_map union with C++ class hierarchy

[David Malcolm]

This patch eliminates the union in struct line_map in favor of
a simple class hierarchy, making struct line_map a base class,
with line_map_ordinary and line_map_macro subclasses.

The patch eliminates all usage of linemap_check_ordinary and
linemap_check_macro from line-map.h, updating return types and
signatures throughout libcpp and gcc's usage of it to use the
appropriate subclasses.

This moves the checking of linemap kind from run-time to
compile-time, and also implicitly documents everywhere where
the code is expecting an ordinary map vs a macro map vs
either kind of map.  I believe it makes the code significantly
simpler: most of the accessor functions in line-map.h become
trivial field-lookups.

I attemped to use templates for maps_info, but was stymied by
gengtype, so in the end I simply split it manually into
maps_info_ordinary and maps_info_macro.  In theory it's just
a vec<>, but vec.h is in gcc, and thus not available
for use from libcpp.

In a similar vein, gcc/is-a.h is presumably not usable
from within libcpp.  If it were, there would be the following
rough equivalences:

---------------------------------  --------------------------------
line-map.h                         is-a.h
---------------------------------  --------------------------------
linemap_check_ordinary (m)         as_a <line_map_ordinary *> (m)
linemap_check_macro (m)            as_a <line_map_macro *> (m)
linemap_macro_expansion_map_p (m)  (M ? is_a <line_map_macro *> (m)
                                      : false)
---------------------------------  --------------------------------

There are numerous places in libcpp that offset a
line_map * using array notation to get the next/prev line_map of the
same kind, e.g.:
MAP_START_LOCATION (&cached[1])
which breaks due to the different sizes of line_map vs its subclasses.

On x86_64 host, before:
(gdb) p sizeof(line_map)
$1 = 40

after:
(gdb) p sizeof(line_map)
$1 = 8
(gdb) p sizeof(line_map_ordinary)
$2 = 32
(gdb) p sizeof(line_map_macro)
$3 = 40

Tracking down all of these array-based offsets to use a pointer to the
appropriate subclass (and thus use the correct offset) was rather
involved, but I believe the patch fixes them all now.

(the patch thus also gives a very modest saving of 8 bytes per ordinary
line map).

I've tried to use the naming convention "ord_map" and "macro_map"
whenever the typesystem ensures we're dealing with such a map,
wherever this is doable without needing to touch lines of code that
would otherwise not need touching by the patch.

gcc/ChangeLog:
	* diagnostic.c (diagnostic_report_current_module): Strengthen
	local "new_map" from const line_map * to
	const line_map_ordinary *.
	* genmatch.c (error_cb): Likewise for local "map".
	(output_line_directive): Likewise for local "map".
	* input.c (expand_location_1): Likewise for local "map".
	Pass NULL rather than &map to
	linemap_unwind_to_first_non_reserved_loc, since the value is never
	read from there, and the value written back not read from here.
	(is_location_from_builtin_token): Strengthen local "map" from
	const line_map * to const line_map_ordinary *.
	(dump_location_info): Strengthen locals "map" from
	line_map *, one to const line_map_ordinary *, the other
	to const line_map_macro *.
	* tree-diagnostic.c (loc_map_pair): Strengthen field "map" from
	const line_map * to const line_map_macro *.
	(maybe_unwind_expanded_macro_loc): Add a call to
	linemap_check_macro when writing to the "map" field of the
	loc_map_pair.
	Introduce local const line_map_ordinary * "ord_map", using it in
	place of "map" in the part of the function where we know we have
	an ordinary map.  Strengthen local "m" from const line_map * to
	const line_map_ordinary *.

gcc/ada/ChangeLog:
	* gcc-interface/trans.c (Sloc_to_locus1): Strenghthen local "map"
	from line_map * to line_map_ordinary *.

gcc/c-family/ChangeLog:
	* c-common.h (fe_file_change): Strengthen param from
	const line_map * to const line_map_ordinary *.
	(pp_file_change): Likewise.
	* c-lex.c (fe_file_change): Likewise.
	(cb_define): Use linemap_check_ordinary when invoking
	SOURCE_LINE.
	(cb_undef): Likewise.
	* c-opts.c (c_finish_options): Use linemap_check_ordinary when
	invoking cb_file_change.
	(c_finish_options): Likewise.
	(push_command_line_include): Likewise.
	(cb_file_change): Strengthen param "new_map" from
	const line_map * to const line_map_ordinary *.
	* c-ppoutput.c (cb_define): Likewise for local "map".
	(pp_file_change): Likewise for param "map" and local "from".

gcc/fortran/ChangeLog:
	* cpp.c (maybe_print_line): Strengthen local "map" from
	const line_map * to const line_map_ordinary *.
	(cb_file_change): Likewise for param "map" and local "from".
	(cb_line_change): Likewise for local "map".

libcpp/ChangeLog:
	* directives.c (do_line): Strengthen local "map" from
	const line_map * to const line_map_ordinary *.
	(do_linemarker): Likewise.
	(_cpp_do_file_change): Assert that we're not dealing with
	a macro map.  Introduce local "ord_map" via a call to
	linemap_check_ordinary, guarded within the check for
	non-NULL.  Use it for typesafety.
	* files.c (cpp_make_system_header): Strengthen local "map" from
	const line_map * to const line_map_ordinary *.
	* include/cpplib.h (struct cpp_callbacks): Likewise for second
	parameter of "file_change" callback.
	* include/line-map.h (struct line_map): Convert from a struct
	containing a union to a base class.
	(struct line_map_ordinary): Convert to a subclass of line_map.
	(struct line_map_macro): Likewise.
	(linemap_check_ordinary): Strengthen return type from line_map *
	to line_map_ordinary *, and add a const-variant.
	(linemap_check_macro): New pair of functions.
	(ORDINARY_MAP_STARTING_LINE_NUMBER): Strengthen param from
	const line_map * to const line_map_ordinary *, eliminating call
	to linemap_check_ordinary.  Likewise for the non-const variant.
	(ORDINARY_MAP_INCLUDER_FILE_INDEX): Likewise.
	(ORDINARY_MAP_IN_SYSTEM_HEADER_P): Likewise.
	(ORDINARY_MAP_NUMBER_OF_COLUMN_BITS): Likewise.
	(ORDINARY_MAP_FILE_NAME): Likewise.
	(MACRO_MAP_MACRO): Strengthen param from const line_map * to
	const line_map_macro *.  Likewise for the non-const variant.
	(MACRO_MAP_NUM_MACRO_TOKENS): Likewise.
	(MACRO_MAP_LOCATIONS): Likewise.
	(MACRO_MAP_EXPANSION_POINT_LOCATION): Likewise.
	(struct maps_info): Replace with...
	(struct maps_info_ordinary):...this and...
	(struct maps_info_macro): ...this.
	(struct line_maps): Convert fields "info_ordinary" and
	"info_macro" to the above new structs.
	(LINEMAPS_MAP_INFO): Delete both functions.
	(LINEMAPS_MAPS): Likewise.
	(LINEMAPS_ALLOCATED): Rewrite both variants to avoid using
	LINEMAPS_MAP_INFO.
	(LINEMAPS_USED): Likewise.
	(LINEMAPS_CACHE): Likewise.
	(LINEMAPS_MAP_AT): Likewise.
	(LINEMAPS_ORDINARY_MAPS): Strengthen return type from line_map *
	to line_map_ordinary *.
	(LINEMAPS_ORDINARY_MAP_AT): Likewise.
	(LINEMAPS_LAST_ORDINARY_MAP): Likewise.
	(LINEMAPS_LAST_ALLOCATED_ORDINARY_MAP): Likewise.
	(LINEMAPS_MACRO_MAPS): Strengthen return type from line_map * to
	line_map_macro *.
	(LINEMAPS_MACRO_MAP_AT): Likewise.
	(LINEMAPS_LAST_MACRO_MAP): Likewise.
	(LINEMAPS_LAST_ALLOCATED_MACRO_MAP): Likewise.
	(linemap_map_get_macro_name): Strengthen param from
	const line_map * to const line_map_macro *.
	(SOURCE_LINE): Strengthen first param from const line_map * to
	const line_map_ordinary *, removing call to
	linemap_check_ordinary.
	(SOURCE_COLUMN): Likewise.
	(LAST_SOURCE_LINE_LOCATION): Likewise.
	(LAST_SOURCE_LINE): Strengthen first param from const line_map *
	to const line_map_ordinary *.
	(LAST_SOURCE_COLUMN): Likewise.
	(INCLUDED_FROM): Strengthen return type from line_map * to
	line_map_ordinary *., and second param from const line_map *
	to const line_map_ordinary *, removing call to
	linemap_check_ordinary.
	(MAIN_FILE_P): Strengthen param from const line_map * to
	const line_map_ordinary *, removing call to
	linemap_check_ordinary.
	(linemap_position_for_line_and_column): Strengthen param from
	const line_map * to const line_map_ordinary *.
	(LINEMAP_FILE): Strengthen param from const line_map * to
	const line_map_ordinary *, removing call to
	linemap_check_ordinary.
	(LINEMAP_LINE): Likewise.
	(LINEMAP_SYSP): Likewise.
	(linemap_resolve_location): Strengthen final param from
	const line_map ** to const line_map_ordinary **.
	* internal.h (CPP_INCREMENT_LINE): Likewise for local "map".
	(linemap_enter_macro): Strengthen return type from
	const line_map * to const line_map_macro *.
	(linemap_add_macro_token): Likewise for first param.
	* line-map.c (linemap_check_files_exited): Strengthen local "map"
	from const line_map * to const line_map_ordinary *.
	(new_linemap): Introduce local "map_size" and use it when
	calculating how large the buffer should be.  Rewrite based
	on change of info_macro and info_ordinary into distinct types.
	(linemap_add): Strengthen locals "map" and "from" from line_map *
	to line_map_ordinary *.
	(linemap_enter_macro): Strengthen return type from
	const line_map * to const line_map_macro *, and local "map" from
	line_map * to line_map_macro *.
	(linemap_add_macro_token): Strengthen param "map" from
	const line_map * to const line_map_macro *.
	(linemap_line_start): Strengthen local "map" from line_map * to
	line_map_ordinary *.
	(linemap_position_for_column): Likewise.
	(linemap_position_for_line_and_column): Strengthen first param
	from const line_map * to const line_map_ordinary *.
	(linemap_position_for_loc_and_offset): Strengthen local "map" from
	const line_map * to const line_map_ordinary *.
	(linemap_ordinary_map_lookup): Likewise for return type and locals
	"cached" and "result".
	(linemap_macro_map_lookup): Strengthen return type and locals
	"cached" and "result" from const line_map * to
	const line_map_macro *.
	(linemap_macro_map_loc_to_exp_point): Likewise for param "map".
	(linemap_macro_map_loc_to_def_point): Likewise.
	(linemap_macro_map_loc_unwind_toward_spelling): Likewise.
	(linemap_get_expansion_line): Strengthen local "map" from
	const line_map * to const line_map_ordinary *.
	(linemap_get_expansion_filename): Likewise.
	(linemap_map_get_macro_name): Strengthen param from
	const line_map * to const line_map_macro *.
	(linemap_location_in_system_header_p): Add call to
	linemap_check_ordinary in region guarded by
	!linemap_macro_expansion_map_p.  Introduce local "macro_map" via
	linemap_check_macro in other region, using it in place of "map"
	for typesafety.
	(first_map_in_common_1): Add calls to linemap_check_macro.
	(trace_include): Strengthen param "map" from const line_map * to
	const line_map_ordinary *.
	(linemap_macro_loc_to_spelling_point): Strengthen final param from
	const line_map ** to const line_map_ordinary **.  Replace a
	C-style cast with a const_cast, and add calls to
	linemap_check_macro and linemap_check_ordinary.
	(linemap_macro_loc_to_def_point): Likewise.
	(linemap_macro_loc_to_exp_point): Likewise.
	(linemap_resolve_location): Strengthen final param from
	const line_map ** to const line_map_ordinary **.
	(linemap_unwind_toward_expansion): Introduce local "macro_map" via
	a checked cast and use it in place of *map.
	(linemap_unwind_to_first_non_reserved_loc): Strengthen local
	"map1" from const line_map * to const line_map_ordinary *.
	(linemap_expand_location): Introduce local "ord_map" via a checked
	cast and use it in place of map.
	(linemap_dump): Make local "map" const.  Strengthen local
	"includer_map" from line_map * to const line_map_ordinary *.
	Introduce locals "ord_map" and "macro_map" via checked casts and
	use them in place of "map" for typesafety.
	(linemap_dump_location): Strengthen local "map" from
	const line_map * to const line_map_ordinary *.
	(linemap_get_file_highest_location): Update for elimination of
	union.
	(linemap_get_statistics): Strengthen local "cur_map" from
	line_map * to const line_map_macro *.  Update uses of sizeof to
	use the appropriate line_map subclasses.
	* macro.c (_cpp_warn_if_unused_macro): Add call to
	linemap_check_ordinary.
	(builtin_macro): Strengthen local "map" from const line_map * to
	const line_map_macro *.
	(enter_macro_context): Likewise.
	(replace_args): Likewise.
	(tokens_buff_put_token_to): Likewise for param "map".
	(tokens_buff_add_token): Likewise.
---
 gcc/ada/gcc-interface/trans.c |   2 +-
 gcc/c-family/c-common.h       |   4 +-
 gcc/c-family/c-lex.c          |   6 +-
 gcc/c-family/c-opts.c         |  14 +-
 gcc/c-family/c-ppoutput.c     |   6 +-
 gcc/diagnostic.c              |   2 +-
 gcc/fortran/cpp.c             |  12 +-
 gcc/genmatch.c                |   4 +-
 gcc/input.c                   |  11 +-
 gcc/tree-diagnostic.c         |   9 +-
 libcpp/directives.c           |  17 +-
 libcpp/files.c                |   2 +-
 libcpp/include/cpplib.h       |   2 +-
 libcpp/include/line-map.h     | 373 ++++++++++++++++++++++--------------------
 libcpp/internal.h             |  12 +-
 libcpp/line-map.c             | 237 +++++++++++++++------------
 libcpp/macro.c                |  18 +-
 17 files changed, 398 insertions(+), 333 deletions(-)

diff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c
index 6ffee06..917a9a6 100644
--- a/gcc/ada/gcc-interface/trans.c
+++ b/gcc/ada/gcc-interface/trans.c
@@ -9316,7 +9316,7 @@ Sloc_to_locus1 (Source_Ptr Sloc, location_t *locus, bool clear_column)
       Source_File_Index file = Get_Source_File_Index (Sloc);
       Logical_Line_Number line = Get_Logical_Line_Number (Sloc);
       Column_Number column = (clear_column ? 0 : Get_Column_Number (Sloc));
-      struct line_map *map = LINEMAPS_ORDINARY_MAP_AT (line_table, file - 1);
+      line_map_ordinary *map = LINEMAPS_ORDINARY_MAP_AT (line_table, file - 1);
 
       /* We can have zero if pragma Source_Reference is in effect.  */
       if (line < 1)
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 603d3f0..b6238aa 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1085,13 +1085,13 @@ extern const unsigned char executable_checksum[16];
 extern void builtin_define_std (const char *macro);
 extern void builtin_define_with_value (const char *, const char *, int);
 extern void c_stddef_cpp_builtins (void);
-extern void fe_file_change (const struct line_map *);
+extern void fe_file_change (const line_map_ordinary *);
 extern void c_parse_error (const char *, enum cpp_ttype, tree, unsigned char);
 
 /* In c-ppoutput.c  */
 extern void init_pp_output (FILE *);
 extern void preprocess_file (cpp_reader *);
-extern void pp_file_change (const struct line_map *);
+extern void pp_file_change (const line_map_ordinary *);
 extern void pp_dir_change (cpp_reader *, const char *);
 extern bool check_missing_format_attribute (tree, tree);
 
diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index bb55be8..85775014 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -201,7 +201,7 @@ cb_line_change (cpp_reader * ARG_UNUSED (pfile), const cpp_token *token,
 }
 
 void
-fe_file_change (const struct line_map *new_map)
+fe_file_change (const line_map_ordinary *new_map)
 {
   if (new_map == NULL)
     return;
@@ -281,7 +281,7 @@ static void
 cb_define (cpp_reader *pfile, source_location loc, cpp_hashnode *node)
 {
   const struct line_map *map = linemap_lookup (line_table, loc);
-  (*debug_hooks->define) (SOURCE_LINE (map, loc),
+  (*debug_hooks->define) (SOURCE_LINE (linemap_check_ordinary (map), loc),
 			  (const char *) cpp_macro_definition (pfile, node));
 }
 
@@ -291,7 +291,7 @@ cb_undef (cpp_reader * ARG_UNUSED (pfile), source_location loc,
 	  cpp_hashnode *node)
 {
   const struct line_map *map = linemap_lookup (line_table, loc);
-  (*debug_hooks->undef) (SOURCE_LINE (map, loc),
+  (*debug_hooks->undef) (SOURCE_LINE (linemap_check_ordinary (map), loc),
 			 (const char *) NODE_NAME (node));
 }
 
diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index 1a67b5a..8a64c06 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -135,7 +135,7 @@ static void handle_deferred_opts (void);
 static void sanitize_cpp_opts (void);
 static void add_prefixed_path (const char *, size_t);
 static void push_command_line_include (void);
-static void cb_file_change (cpp_reader *, const struct line_map *);
+static void cb_file_change (cpp_reader *, const line_map_ordinary *);
 static void cb_dir_change (cpp_reader *, const char *);
 static void c_finish_options (void);
 
@@ -1296,8 +1296,10 @@ c_finish_options (void)
       size_t i;
 
       cb_file_change (parse_in,
-		      linemap_add (line_table, LC_RENAME, 0,
-				   _("<built-in>"), 0));
+		      linemap_check_ordinary (linemap_add (line_table,
+							   LC_RENAME, 0,
+							   _("<built-in>"),
+							   0)));
       /* Make sure all of the builtins about to be declared have
 	 BUILTINS_LOCATION has their source_location.  */
       source_location builtins_loc = BUILTINS_LOCATION;
@@ -1320,8 +1322,8 @@ c_finish_options (void)
       cpp_opts->warn_dollars = (cpp_opts->cpp_pedantic && !cpp_opts->c99);
 
       cb_file_change (parse_in,
-		      linemap_add (line_table, LC_RENAME, 0,
-				   _("<command-line>"), 0));
+		      linemap_check_ordinary (linemap_add (line_table, LC_RENAME, 0,
+							   _("<command-line>"), 0)));
 
       for (i = 0; i < deferred_count; i++)
 	{
@@ -1424,7 +1426,7 @@ push_command_line_include (void)
 /* File change callback.  Has to handle -include files.  */
 static void
 cb_file_change (cpp_reader * ARG_UNUSED (pfile),
-		const struct line_map *new_map)
+		const line_map_ordinary *new_map)
 {
   if (flag_preprocess_only)
     pp_file_change (new_map);
diff --git a/gcc/c-family/c-ppoutput.c b/gcc/c-family/c-ppoutput.c
index 9b9cba0..e936d5d 100644
--- a/gcc/c-family/c-ppoutput.c
+++ b/gcc/c-family/c-ppoutput.c
@@ -502,7 +502,7 @@ cb_ident (cpp_reader *pfile ATTRIBUTE_UNUSED, source_location line,
 static void
 cb_define (cpp_reader *pfile, source_location line, cpp_hashnode *node)
 {
-  const struct line_map *map;
+  const line_map_ordinary *map;
 
   maybe_print_line (line);
   fputs ("#define ", print.outf);
@@ -642,7 +642,7 @@ pp_dir_change (cpp_reader *pfile ATTRIBUTE_UNUSED, const char *dir)
    described in MAP.  */
 
 void
-pp_file_change (const struct line_map *map)
+pp_file_change (const line_map_ordinary *map)
 {
   const char *flags = "";
 
@@ -664,7 +664,7 @@ pp_file_change (const struct line_map *map)
 	  /* Bring current file to correct line when entering a new file.  */
 	  if (map->reason == LC_ENTER)
 	    {
-	      const struct line_map *from = INCLUDED_FROM (line_table, map);
+	      const line_map_ordinary *from = INCLUDED_FROM (line_table, map);
 	      maybe_print_line (LAST_SOURCE_LINE_LOCATION (from));
 	    }
 	  if (map->reason == LC_ENTER)
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 2196406..66e38e1 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -557,7 +557,7 @@ diagnostic_action_after_output (diagnostic_context *context,
 void
 diagnostic_report_current_module (diagnostic_context *context, location_t where)
 {
-  const struct line_map *map = NULL;
+  const line_map_ordinary *map = NULL;
 
   if (pp_needs_newline (context->printer))
     {
diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c
index e239f21..e0e1f74 100644
--- a/gcc/fortran/cpp.c
+++ b/gcc/fortran/cpp.c
@@ -147,7 +147,7 @@ static void scan_translation_unit_trad (cpp_reader *);
 
 /* Callback routines for the parser. Most of these are active only
    in specific modes.  */
-static void cb_file_change (cpp_reader *, const struct line_map *);
+static void cb_file_change (cpp_reader *, const line_map_ordinary *);
 static void cb_line_change (cpp_reader *, const cpp_token *, int);
 static void cb_define (cpp_reader *, source_location, cpp_hashnode *);
 static void cb_undef (cpp_reader *, source_location, cpp_hashnode *);
@@ -807,7 +807,8 @@ scan_translation_unit_trad (cpp_reader *pfile)
 static void
 maybe_print_line (source_location src_loc)
 {
-  const struct line_map *map = linemap_lookup (line_table, src_loc);
+  const line_map_ordinary *map
+    = linemap_check_ordinary (linemap_lookup (line_table, src_loc));
   int src_line = SOURCE_LINE (map, src_loc);
 
   /* End the previous line of text.  */
@@ -874,7 +875,7 @@ print_line (source_location src_loc, const char *special_flags)
 }
 
 static void
-cb_file_change (cpp_reader * ARG_UNUSED (pfile), const struct line_map *map)
+cb_file_change (cpp_reader * ARG_UNUSED (pfile), const line_map_ordinary *map)
 {
   const char *flags = "";
 
@@ -896,7 +897,7 @@ cb_file_change (cpp_reader * ARG_UNUSED (pfile), const struct line_map *map)
 	  /* Bring current file to correct line when entering a new file.  */
 	  if (map->reason == LC_ENTER)
 	    {
-	      const struct line_map *from = INCLUDED_FROM (line_table, map);
+	      const line_map_ordinary *from = INCLUDED_FROM (line_table, map);
 	      maybe_print_line (LAST_SOURCE_LINE_LOCATION (from));
 	    }
 	  if (map->reason == LC_ENTER)
@@ -930,7 +931,8 @@ cb_line_change (cpp_reader *pfile, const cpp_token *token,
      ought to care.  Some things do care; the fault lies with them.  */
   if (!CPP_OPTION (pfile, traditional))
     {
-      const struct line_map *map = linemap_lookup (line_table, src_loc);
+      const line_map_ordinary *map
+	= linemap_check_ordinary (linemap_lookup (line_table, src_loc));
       int spaces = SOURCE_COLUMN (map, src_loc) - 2;
       print.printed = 1;
 
diff --git a/gcc/genmatch.c b/gcc/genmatch.c
index fbd12a5..bca7909 100644
--- a/gcc/genmatch.c
+++ b/gcc/genmatch.c
@@ -58,7 +58,7 @@ __attribute__((format (printf, 6, 0)))
 error_cb (cpp_reader *, int errtype, int, source_location location,
 	  unsigned int, const char *msg, va_list *ap)
 {
-  const line_map *map;
+  const line_map_ordinary *map;
   linemap_resolve_location (line_table, location, LRK_SPELLING_LOCATION, &map);
   expanded_location loc = linemap_expand_location (line_table, map, location);
   fprintf (stderr, "%s:%d:%d %s: ", loc.file, loc.line, loc.column,
@@ -134,7 +134,7 @@ static void
 output_line_directive (FILE *f, source_location location,
 		       bool dumpfile = false)
 {
-  const line_map *map;
+  const line_map_ordinary *map;
   linemap_resolve_location (line_table, location, LRK_SPELLING_LOCATION, &map);
   expanded_location loc = linemap_expand_location (line_table, map, location);
   if (dumpfile)
diff --git a/gcc/input.c b/gcc/input.c
index 922c2f1..20eee88 100644
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -134,7 +134,7 @@ expand_location_1 (source_location loc,
 		   bool expansion_point_p)
 {
   expanded_location xloc;
-  const struct line_map *map;
+  const line_map_ordinary *map;
   enum location_resolution_kind lrk = LRK_MACRO_EXPANSION_POINT;
   tree block = NULL;
 
@@ -158,7 +158,7 @@ expand_location_1 (source_location loc,
 	     location (toward the expansion point) that is not reserved;
 	     that is, the first location that is in real source code.  */
 	  loc = linemap_unwind_to_first_non_reserved_loc (line_table,
-							  loc, &map);
+							  loc, NULL);
 	  lrk = LRK_SPELLING_LOCATION;
 	}
       loc = linemap_resolve_location (line_table, loc,
@@ -724,7 +724,7 @@ location_get_source_line (expanded_location xloc,
 bool
 is_location_from_builtin_token (source_location loc)
 {
-  const line_map *map = NULL;
+  const line_map_ordinary *map = NULL;
   loc = linemap_resolve_location (line_table, loc,
 				  LRK_SPELLING_LOCATION, &map);
   return loc == BUILTINS_LOCATION;
@@ -955,7 +955,8 @@ dump_location_info (FILE *stream)
       source_location end_location = get_end_location (line_table, idx);
       /* half-closed: doesn't include this one. */
 
-      struct line_map *map = LINEMAPS_ORDINARY_MAP_AT (line_table, idx);
+      const line_map_ordinary *map
+	= LINEMAPS_ORDINARY_MAP_AT (line_table, idx);
       fprintf (stream, "ORDINARY MAP: %i\n", idx);
       dump_location_range (stream,
 			   MAP_START_LOCATION (map), end_location);
@@ -1032,7 +1033,7 @@ dump_location_info (FILE *stream)
       unsigned int idx = (ascending_source_locations
 			  ? (LINEMAPS_MACRO_USED (line_table) - (i + 1))
 			  : i);
-      struct line_map *map = LINEMAPS_MACRO_MAP_AT (line_table, idx);
+      const line_map_macro *map = LINEMAPS_MACRO_MAP_AT (line_table, idx);
       fprintf (stream, "MACRO %i: %s (%u tokens)\n",
 	       idx,
 	       linemap_map_get_macro_name (map),
diff --git a/gcc/tree-diagnostic.c b/gcc/tree-diagnostic.c
index 99d47cb..a4bbfa9 100644
--- a/gcc/tree-diagnostic.c
+++ b/gcc/tree-diagnostic.c
@@ -67,7 +67,7 @@ default_tree_diagnostic_starter (diagnostic_context *context,
    below.  */
 typedef struct
 {
-  const struct line_map *map;
+  const line_map_macro *map;
   source_location where;
 } loc_map_pair;
 
@@ -133,7 +133,7 @@ maybe_unwind_expanded_macro_loc (diagnostic_context *context,
   do
     {
       loc.where = where;
-      loc.map = map;
+      loc.map = linemap_check_macro (map);
 
       loc_vec.safe_push (loc);
 
@@ -148,6 +148,7 @@ maybe_unwind_expanded_macro_loc (diagnostic_context *context,
 
   /* Now map is set to the map of the location in the source that
      first triggered the macro expansion.  This must be an ordinary map.  */
+  const line_map_ordinary *ord_map = linemap_check_ordinary (map);
 
   /* Walk LOC_VEC and print the macro expansion trace, unless the
      first macro which expansion triggered this trace was expanded
@@ -155,7 +156,7 @@ maybe_unwind_expanded_macro_loc (diagnostic_context *context,
   int saved_location_line =
     expand_location_to_spelling_point (diagnostic->location).line;
 
-  if (!LINEMAP_SYSP (map))
+  if (!LINEMAP_SYSP (ord_map))
     FOR_EACH_VEC_ELT (loc_vec, ix, iter)
       {
 	/* Sometimes, in the unwound macro expansion trace, we want to
@@ -195,7 +196,7 @@ maybe_unwind_expanded_macro_loc (diagnostic_context *context,
 
 	/* Don't print trace for locations that are reserved or from
 	   within a system header.  */
-        const struct line_map *m = NULL;
+        const line_map_ordinary *m = NULL;
         source_location l = 
           linemap_resolve_location (line_table, resolved_def_loc,
                                     LRK_SPELLING_LOCATION,  &m);
diff --git a/libcpp/directives.c b/libcpp/directives.c
index 2a824e6..6f0dcc5 100644
--- a/libcpp/directives.c
+++ b/libcpp/directives.c
@@ -912,7 +912,7 @@ static void
 do_line (cpp_reader *pfile)
 {
   const struct line_maps *line_table = pfile->line_table;
-  const struct line_map *map = LINEMAPS_LAST_ORDINARY_MAP (line_table);
+  const line_map_ordinary *map = LINEMAPS_LAST_ORDINARY_MAP (line_table);
 
   /* skip_rest_of_line() may cause line table to be realloc()ed so note down
      sysp right now.  */
@@ -974,7 +974,7 @@ static void
 do_linemarker (cpp_reader *pfile)
 {
   const struct line_maps *line_table = pfile->line_table;
-  const struct line_map *map = LINEMAPS_LAST_ORDINARY_MAP (line_table);
+  const line_map_ordinary *map = LINEMAPS_LAST_ORDINARY_MAP (line_table);
   const cpp_token *token;
   const char *new_file = ORDINARY_MAP_FILE_NAME (map);
   linenum_type new_lineno;
@@ -1063,15 +1063,20 @@ _cpp_do_file_change (cpp_reader *pfile, enum lc_reason reason,
 		     const char *to_file, linenum_type file_line,
 		     unsigned int sysp)
 {
+  linemap_assert (reason != LC_ENTER_MACRO);
   const struct line_map *map = linemap_add (pfile->line_table, reason, sysp,
 					    to_file, file_line);
+  const line_map_ordinary *ord_map = NULL;
   if (map != NULL)
-    linemap_line_start (pfile->line_table,
-			ORDINARY_MAP_STARTING_LINE_NUMBER (map),
-			127);
+    {
+      ord_map = linemap_check_ordinary (map);
+      linemap_line_start (pfile->line_table,
+			  ORDINARY_MAP_STARTING_LINE_NUMBER (ord_map),
+			  127);
+    }
 
   if (pfile->cb.file_change)
-    pfile->cb.file_change (pfile, map);
+    pfile->cb.file_change (pfile, ord_map);
 }
 
 /* Report a warning or error detected by the program we are
diff --git a/libcpp/files.c b/libcpp/files.c
index 2f49122..8c388d8 100644
--- a/libcpp/files.c
+++ b/libcpp/files.c
@@ -1326,7 +1326,7 @@ cpp_make_system_header (cpp_reader *pfile, int syshdr, int externc)
 {
   int flags = 0;
   const struct line_maps *line_table = pfile->line_table;
-  const struct line_map *map = LINEMAPS_LAST_ORDINARY_MAP (line_table);
+  const line_map_ordinary *map = LINEMAPS_LAST_ORDINARY_MAP (line_table);
   /* 1 = system header, 2 = system header to be treated as C.  */
   if (syshdr)
     flags = 1 + (externc != 0);
diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h
index 5e08014..7ba8265 100644
--- a/libcpp/include/cpplib.h
+++ b/libcpp/include/cpplib.h
@@ -546,7 +546,7 @@ struct cpp_callbacks
      The line_map is for the new file.  It is NULL if there is no new file.
      (In C this happens when done with <built-in>+<command line> and also
      when done with a main file.)  This can be used for resource cleanup.  */
-  void (*file_change) (cpp_reader *, const struct line_map *);
+  void (*file_change) (cpp_reader *, const line_map_ordinary *);
 
   void (*dir_change) (cpp_reader *, const char *);
   void (*include) (cpp_reader *, source_location, const unsigned char *,
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 9dfc396..25cee54 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -125,6 +125,39 @@ typedef void *(*line_map_realloc) (void *, size_t);
    for a given requested allocation.  */
 typedef size_t (*line_map_round_alloc_size_func) (size_t);
 
+/* A line_map encodes a sequence of locations.
+   There are two kinds of maps. Ordinary maps and macro expansion
+   maps, a.k.a macro maps.
+
+   A macro map encodes source locations of tokens that are part of a
+   macro replacement-list, at a macro expansion point. E.g, in:
+
+            #define PLUS(A,B) A + B
+
+   No macro map is going to be created there, because we are not at a
+   macro expansion point. We are at a macro /definition/ point. So the
+   locations of the tokens of the macro replacement-list (i.e, A + B)
+   will be locations in an ordinary map, not a macro map.
+
+   On the other hand, if we later do:
+
+        int a = PLUS (1,2);
+
+   The invocation of PLUS here is a macro expansion. So we are at a
+   macro expansion point. The preprocessor expands PLUS (1,2) and
+   replaces it with the tokens of its replacement-list: 1 + 2. A macro
+   map is going to be created to hold (or rather to map, haha ...) the
+   locations of the tokens 1, + and 2. The macro map also records the
+   location of the expansion point of PLUS. That location is mapped in
+   the map that is active right before the location of the invocation
+   of PLUS.  */
+struct GTY((tag ("0"), desc ("%h.reason == LC_ENTER_MACRO ? 2 : 1"))) line_map {
+  source_location start_location;
+
+  /* The reason for creation of this line map.  */
+  ENUM_BITFIELD (lc_reason) reason : CHAR_BIT;
+};
+
 /* An ordinary line map encodes physical source locations. Those
    physical source locations are called "spelling locations".
    
@@ -136,7 +169,7 @@ typedef size_t (*line_map_round_alloc_size_func) (size_t);
    means "entire file/line" or "unknown line/column" or "not applicable".)
 
    The highest possible source location is MAX_SOURCE_LOCATION.  */
-struct GTY(()) line_map_ordinary {
+struct GTY((tag ("1"))) line_map_ordinary : public line_map {
   const char *to_file;
   linenum_type to_line;
 
@@ -164,13 +197,9 @@ struct cpp_hashnode;
 /* A macro line map encodes location of tokens coming from a macro
    expansion.
    
-   Please note that this struct line_map_macro is a field of struct
-   line_map below, go read the comments of struct line_map below and
-   then come back here.
-   
    The offset from START_LOCATION is used to index into
    MACRO_LOCATIONS; this holds the original location of the token.  */
-struct GTY(()) line_map_macro {
+struct GTY((tag ("2"))) line_map_macro : public line_map {
   /* The cpp macro which expansion gave birth to this macro map.  */
   struct cpp_hashnode * GTY ((nested_ptr (union tree_node,
 				   "%h ? CPP_HASHNODE (GCC_IDENT_TO_HT_IDENT (%h)) : NULL",
@@ -243,44 +272,6 @@ struct GTY(()) line_map_macro {
   source_location expansion;
 };
 
-/* A line_map encodes a sequence of locations.
-   There are two kinds of maps. Ordinary maps and macro expansion
-   maps, a.k.a macro maps.
-
-   A macro map encodes source locations of tokens that are part of a
-   macro replacement-list, at a macro expansion point. E.g, in:
-
-            #define PLUS(A,B) A + B
-
-   No macro map is going to be created there, because we are not at a
-   macro expansion point. We are at a macro /definition/ point. So the
-   locations of the tokens of the macro replacement-list (i.e, A + B)
-   will be locations in an ordinary map, not a macro map.
-
-   On the other hand, if we later do:
-
-        int a = PLUS (1,2);
-
-   The invocation of PLUS here is a macro expansion. So we are at a
-   macro expansion point. The preprocessor expands PLUS (1,2) and
-   replaces it with the tokens of its replacement-list: 1 + 2. A macro
-   map is going to be created to hold (or rather to map, haha ...) the
-   locations of the tokens 1, + and 2. The macro map also records the
-   location of the expansion point of PLUS. That location is mapped in
-   the map that is active right before the location of the invocation
-   of PLUS.  */
-struct GTY(()) line_map {
-  source_location start_location;
-
-  /* The reason for creation of this line map.  */
-  ENUM_BITFIELD (lc_reason) reason : CHAR_BIT;
-
-  union map_u {
-    struct line_map_ordinary GTY((tag ("0"))) ordinary;
-    struct line_map_macro GTY((tag ("1"))) macro;
-  } GTY((desc ("%1.reason == LC_ENTER_MACRO"))) d;
-};
-
 #if defined ENABLE_CHECKING && (GCC_VERSION >= 2007)
 
 /* Assertion macro to be used in line-map code.  */
@@ -311,18 +302,34 @@ struct GTY(()) line_map {
 bool linemap_macro_expansion_map_p (const struct line_map *);
 
 /* Assert that MAP encodes locations of tokens that are not part of
-   the replacement-list of a macro expansion.  */
-inline struct line_map *linemap_check_ordinary (struct line_map *map)
+   the replacement-list of a macro expansion and downcast to the
+   appropriate subclass.  */
+inline line_map_ordinary *linemap_check_ordinary (struct line_map *map)
 {
   linemap_assert (!linemap_macro_expansion_map_p (map));
-  return map;
+  return (line_map_ordinary *)map;
 }
 
-inline const struct line_map *
+inline const line_map_ordinary *
 linemap_check_ordinary (const struct line_map *map)
 {
   linemap_assert (!linemap_macro_expansion_map_p (map));
-  return map;
+  return (const line_map_ordinary *)map;
+}
+
+/* Assert that MAP is a macro expansion and downcast to the appropriate
+   subclass.  */
+inline line_map_macro *linemap_check_macro (line_map *map)
+{
+  linemap_assert (linemap_macro_expansion_map_p (map));
+  return (line_map_macro *)map;
+}
+
+inline const line_map_macro *
+linemap_check_macro (const line_map *map)
+{
+  linemap_assert (linemap_macro_expansion_map_p (map));
+  return (const line_map_macro *)map;
 }
 
 inline source_location
@@ -338,122 +345,140 @@ MAP_START_LOCATION (line_map *map)
 }
 
 inline linenum_type
-ORDINARY_MAP_STARTING_LINE_NUMBER (const line_map *map)
+ORDINARY_MAP_STARTING_LINE_NUMBER (const line_map_ordinary *ord_map)
 {
-  return linemap_check_ordinary (map)->d.ordinary.to_line;
+  return ord_map->to_line;
 }
 
 inline linenum_type&
-ORDINARY_MAP_STARTING_LINE_NUMBER (line_map *map)
+ORDINARY_MAP_STARTING_LINE_NUMBER (line_map_ordinary *ord_map)
 {
-  return linemap_check_ordinary (map)->d.ordinary.to_line;
+  return ord_map->to_line;
 }
 
 inline int
-ORDINARY_MAP_INCLUDER_FILE_INDEX (const line_map *map)
+ORDINARY_MAP_INCLUDER_FILE_INDEX (const line_map_ordinary *ord_map)
 {
-  return linemap_check_ordinary (map)->d.ordinary.included_from;
+  return ord_map->included_from;
 }
 
 inline int&
-ORDINARY_MAP_INCLUDER_FILE_INDEX (line_map *map)
+ORDINARY_MAP_INCLUDER_FILE_INDEX (line_map_ordinary *ord_map)
 {
-  return linemap_check_ordinary (map)->d.ordinary.included_from;
+  return ord_map->included_from;
 }
 
 inline unsigned char
-ORDINARY_MAP_IN_SYSTEM_HEADER_P (const line_map *map)
+ORDINARY_MAP_IN_SYSTEM_HEADER_P (const line_map_ordinary *ord_map)
 {
-  return linemap_check_ordinary (map)->d.ordinary.sysp;
+  return ord_map->sysp;
 }
 
 inline unsigned char &
-ORDINARY_MAP_IN_SYSTEM_HEADER_P (line_map *map)
+ORDINARY_MAP_IN_SYSTEM_HEADER_P (line_map_ordinary *ord_map)
 {
-  return linemap_check_ordinary (map)->d.ordinary.sysp;
+  return ord_map->sysp;
 }
 
 inline unsigned char
-ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (const line_map *map)
+ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (const line_map_ordinary *ord_map)
 {
-  return linemap_check_ordinary (map)->d.ordinary.column_bits;
+  return ord_map->column_bits;
 }
 inline void
-SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (line_map *map, int col_bits)
+SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (line_map_ordinary *ord_map,
+					int col_bits)
 {
-  linemap_check_ordinary (map)->d.ordinary.column_bits = col_bits;
+  ord_map->column_bits = col_bits;
 }
 
 inline const char *
-ORDINARY_MAP_FILE_NAME (const line_map *map)
+ORDINARY_MAP_FILE_NAME (const line_map_ordinary *ord_map)
 {
-  return linemap_check_ordinary (map)->d.ordinary.to_file;
+  return ord_map->to_file;
 }
 
 inline const char * &
-ORDINARY_MAP_FILE_NAME (line_map *map)
+ORDINARY_MAP_FILE_NAME (line_map_ordinary *ord_map)
 {
-  return linemap_check_ordinary (map)->d.ordinary.to_file;
+  return ord_map->to_file;
 }
 
 inline cpp_hashnode *
-MACRO_MAP_MACRO (const line_map *map)
+MACRO_MAP_MACRO (const line_map_macro *macro_map)
 {
-  return map->d.macro.macro;
+  return macro_map->macro;
 }
 
 inline cpp_hashnode * &
-MACRO_MAP_MACRO (line_map *map)
+MACRO_MAP_MACRO (line_map_macro *macro_map)
 {
-  return map->d.macro.macro;
+  return macro_map->macro;
 }
 
 inline unsigned int
-MACRO_MAP_NUM_MACRO_TOKENS (const line_map *map)
+MACRO_MAP_NUM_MACRO_TOKENS (const line_map_macro *macro_map)
 {
-  return map->d.macro.n_tokens;
+  return macro_map->n_tokens;
 }
 
 inline unsigned int &
-MACRO_MAP_NUM_MACRO_TOKENS (line_map *map)
+MACRO_MAP_NUM_MACRO_TOKENS (line_map_macro *macro_map)
 {
-  return map->d.macro.n_tokens;
+  return macro_map->n_tokens;
 }
 
 inline source_location *
-MACRO_MAP_LOCATIONS (const line_map *map)
+MACRO_MAP_LOCATIONS (const line_map_macro *macro_map)
 {
-  return map->d.macro.macro_locations;
+  return macro_map->macro_locations;
 }
 
 inline source_location * &
-MACRO_MAP_LOCATIONS (line_map *map)
+MACRO_MAP_LOCATIONS (line_map_macro *macro_map)
 {
-  return map->d.macro.macro_locations;
+  return macro_map->macro_locations;
 }
 
 inline source_location
-MACRO_MAP_EXPANSION_POINT_LOCATION (const line_map *map)
+MACRO_MAP_EXPANSION_POINT_LOCATION (const line_map_macro *macro_map)
 {
-  return map->d.macro.expansion;
+  return macro_map->expansion;
 }
 
 inline source_location &
-MACRO_MAP_EXPANSION_POINT_LOCATION (line_map *map)
+MACRO_MAP_EXPANSION_POINT_LOCATION (line_map_macro *macro_map)
 {
-  return map->d.macro.expansion;
+  return macro_map->expansion;
 }
 
 /* The abstraction of a set of location maps. There can be several
    types of location maps. This abstraction contains the attributes
-   that are independent from the type of the map.  */
-struct GTY(()) maps_info {
-  /* This array contains the different line maps.
-     A line map is created for the following events:
-       - when a new preprocessing unit start. 
-       - when a preprocessing unit ends.
-       - when a macro expansion occurs.  */
-  struct line_map * GTY ((length ("%h.used"))) maps;
+   that are independent from the type of the map.
+
+   Essentially this is just a vector of T_linemap_subclass,
+   which can only ever grow in size.  */
+
+struct GTY(()) maps_info_ordinary {
+  /* This array contains the "ordinary" line maps, for all
+     events other than macro expansion
+     (e.g. when a new preprocessing unit starts or ends).  */
+  line_map_ordinary * GTY ((length ("%h.used"))) maps;
+
+  /* The total number of allocated maps.  */
+  unsigned int allocated;
+
+  /* The number of elements used in maps. This number is smaller
+     or equal to ALLOCATED.  */
+  unsigned int used;
+
+  unsigned int cache;
+};
+
+struct GTY(()) maps_info_macro {
+  /* This array contains the macro line maps.
+     A macro line map is created whenever a macro expansion occurs.  */
+  line_map_macro * GTY ((length ("%h.used"))) maps;
 
   /* The total number of allocated maps.  */
   unsigned int allocated;
@@ -493,9 +518,9 @@ struct GTY(()) location_adhoc_data_map {
 /* A set of chronological line_map structures.  */
 struct GTY(()) line_maps {
   
-  struct maps_info info_ordinary;
+  maps_info_ordinary info_ordinary;
 
-  struct maps_info info_macro;
+  maps_info_macro info_macro;
 
   /* Depth of the include stack, including the current file.  */
   unsigned int depth;
@@ -528,52 +553,24 @@ struct GTY(()) line_maps {
   source_location builtin_location;
 };
 
-/* Returns the pointer to the memory region where information about
-   maps are stored in the line table SET. MACRO_MAP_P is a flag
-   telling if we want macro or ordinary maps.  */
-inline struct maps_info *
-LINEMAPS_MAP_INFO (line_maps *set, bool macro_map_p)
-{
-  return (macro_map_p
-	  ? &(set->info_macro)
-	  : &(set->info_ordinary));
-}
-
-inline const struct maps_info *
-LINEMAPS_MAP_INFO (const line_maps *set, bool macro_map_p)
-{
-  return (macro_map_p
-	  ? &(set->info_macro)
-	  : &(set->info_ordinary));
-}
-
-/* Returns the pointer to the memory region where maps are stored in
-   the line table SET. MAP_KIND shall be TRUE if we are interested in
-   macro maps false otherwise.  */
-inline line_map *
-LINEMAPS_MAPS (const line_maps *set, bool map_kind)
-{
-  return LINEMAPS_MAP_INFO (set, map_kind)->maps;
-}
-
-inline line_map * &
-LINEMAPS_MAPS (line_maps *set, bool map_kind)
-{
-  return LINEMAPS_MAP_INFO (set, map_kind)->maps;
-}
-
 /* Returns the number of allocated maps so far. MAP_KIND shall be TRUE
    if we are interested in macro maps, FALSE otherwise.  */
 inline unsigned int
 LINEMAPS_ALLOCATED (const line_maps *set, bool map_kind)
 {
-  return LINEMAPS_MAP_INFO (set, map_kind)->allocated;
+  if (map_kind)
+    return set->info_macro.allocated;
+  else
+    return set->info_ordinary.allocated;
 }
 
 inline unsigned int &
 LINEMAPS_ALLOCATED (line_maps *set, bool map_kind)
 {
-  return LINEMAPS_MAP_INFO (set, map_kind)->allocated;
+  if (map_kind)
+    return set->info_macro.allocated;
+  else
+    return set->info_ordinary.allocated;
 }
 
 /* Returns the number of used maps so far. MAP_KIND shall be TRUE if
@@ -581,13 +578,19 @@ LINEMAPS_ALLOCATED (line_maps *set, bool map_kind)
 inline unsigned int
 LINEMAPS_USED (const line_maps *set, bool map_kind)
 {
-  return LINEMAPS_MAP_INFO (set, map_kind)->used;
+  if (map_kind)
+    return set->info_macro.used;
+  else
+    return set->info_ordinary.used;
 }
 
 inline unsigned int &
 LINEMAPS_USED (line_maps *set, bool map_kind)
 {
-  return LINEMAPS_MAP_INFO (set, map_kind)->used;
+  if (map_kind)
+    return set->info_macro.used;
+  else
+    return set->info_ordinary.used;
 }
 
 /* Returns the index of the last map that was looked up with
@@ -596,20 +599,29 @@ LINEMAPS_USED (line_maps *set, bool map_kind)
 inline unsigned int
 LINEMAPS_CACHE (const line_maps *set, bool map_kind)
 {
-  return LINEMAPS_MAP_INFO (set, map_kind)->cache;
+  if (map_kind)
+    return set->info_macro.cache;
+  else
+    return set->info_ordinary.cache;
 }
 
 inline unsigned int &
 LINEMAPS_CACHE (line_maps *set, bool map_kind)
 {
-  return LINEMAPS_MAP_INFO (set, map_kind)->cache;
+  if (map_kind)
+    return set->info_macro.cache;
+  else
+    return set->info_ordinary.cache;
 }
 
 /* Return the map at a given index.  */
 inline line_map *
 LINEMAPS_MAP_AT (const line_maps *set, bool map_kind, int index)
 {
-  return &(LINEMAPS_MAPS (set, map_kind)[index]);
+  if (map_kind)
+    return &set->info_macro.maps[index];
+  else
+    return &set->info_ordinary.maps[index];
 }
 
 /* Returns the last map used in the line table SET. MAP_KIND
@@ -634,17 +646,19 @@ LINEMAPS_LAST_ALLOCATED_MAP (const line_maps *set, bool map_kind)
 
 /* Returns a pointer to the memory region where ordinary maps are
    allocated in the line table SET.  */
-inline line_map *
+inline line_map_ordinary *
 LINEMAPS_ORDINARY_MAPS (const line_maps *set)
 {
-  return LINEMAPS_MAPS (set, false);
+  return set->info_ordinary.maps;
 }
 
 /* Returns the INDEXth ordinary map.  */
-inline line_map *
+inline line_map_ordinary *
 LINEMAPS_ORDINARY_MAP_AT (const line_maps *set, int index)
 {
-  return LINEMAPS_MAP_AT (set, false, index);
+  linemap_assert (index >= 0);
+  linemap_assert ((unsigned int)index < set->info_ordinary.used);
+  return &set->info_ordinary.maps[index];
 }
 
 /* Return the number of ordinary maps allocated in the line table
@@ -678,32 +692,35 @@ LINEMAPS_ORDINARY_CACHE (line_maps *set)
 
 /* Returns a pointer to the last ordinary map used in the line table
    SET.  */
-inline line_map *
+inline line_map_ordinary *
 LINEMAPS_LAST_ORDINARY_MAP (const line_maps *set)
 {
-  return LINEMAPS_LAST_MAP (set, false);
+  return (line_map_ordinary *)LINEMAPS_LAST_MAP (set, false);
 }
 
 /* Returns a pointer to the last ordinary map allocated the line table
    SET.  */
-inline line_map *
+inline line_map_ordinary *
 LINEMAPS_LAST_ALLOCATED_ORDINARY_MAP (const line_maps *set)
 {
-  return LINEMAPS_LAST_ALLOCATED_MAP (set, false);
+  return (line_map_ordinary *)LINEMAPS_LAST_ALLOCATED_MAP (set, false);
 }
 
 /* Returns a pointer to the beginning of the region where macro maps
    are allcoated.  */
-inline line_map *
+inline line_map_macro *
 LINEMAPS_MACRO_MAPS (const line_maps *set)
 {
-  return LINEMAPS_MAPS (set, true);
+  return set->info_macro.maps;
 }
 
 /* Returns the INDEXth macro map.  */
-inline line_map *LINEMAPS_MACRO_MAP_AT (const line_maps *set, int index)
+inline line_map_macro *
+LINEMAPS_MACRO_MAP_AT (const line_maps *set, int index)
 {
-  return LINEMAPS_MAP_AT (set, true, index);
+  linemap_assert (index >= 0);
+  linemap_assert ((unsigned int)index < set->info_macro.used);
+  return &set->info_macro.maps[index];
 }
 
 /* Returns the number of macro maps that were allocated in the line
@@ -736,10 +753,10 @@ LINEMAPS_MACRO_CACHE (line_maps *set)
 }
 
 /* Returns the last macro map used in the line table SET.  */
-inline line_map *
+inline line_map_macro *
 LINEMAPS_LAST_MACRO_MAP (const line_maps *set)
 {
-  return LINEMAPS_LAST_MAP (set, true);
+  return (line_map_macro *)LINEMAPS_LAST_MAP (set, true);
 }
 
 /* Returns the lowest location [of a token resulting from macro
@@ -753,10 +770,10 @@ LINEMAPS_MACRO_LOWEST_LOCATION (const line_maps *set)
 }
 
 /* Returns the last macro map allocated in the line table SET.  */
-inline line_map *
+inline line_map_macro *
 LINEMAPS_LAST_ALLOCATED_MACRO_MAP (const line_maps *set)
 {
-  return LINEMAPS_LAST_ALLOCATED_MAP (set, true);
+  return (line_map_macro *)LINEMAPS_LAST_ALLOCATED_MAP (set, true);
 }
 
 extern void location_adhoc_data_fini (struct line_maps *);
@@ -832,7 +849,7 @@ extern const struct line_map *linemap_lookup
 bool linemap_tracks_macro_expansion_locs_p (struct line_maps *);
 
 /* Return the name of the macro associated to MACRO_MAP.  */
-const char* linemap_map_get_macro_name (const struct line_map*);
+const char* linemap_map_get_macro_name (const line_map_macro *);
 
 /* Return a positive value if LOCATION is the locus of a token that is
    located in a system header, O otherwise. It returns 1 if LOCATION
@@ -859,42 +876,42 @@ const int RESERVED_LOCATION_COUNT = 2;
 
 /* Converts a map and a source_location to source line.  */
 inline linenum_type
-SOURCE_LINE (const struct line_map *map, source_location loc)
+SOURCE_LINE (const line_map_ordinary *ord_map, source_location loc)
 {
-  return ((loc - linemap_check_ordinary (map)->start_location)
-	  >> map->d.ordinary.column_bits) + map->d.ordinary.to_line;
+  return ((loc - ord_map->start_location)
+	  >> ord_map->column_bits) + ord_map->to_line;
 }
 
 /* Convert a map and source_location to source column number.  */
 inline linenum_type
-SOURCE_COLUMN (const struct line_map *map, source_location loc)
+SOURCE_COLUMN (const line_map_ordinary *ord_map, source_location loc)
 {
-  return ((loc - linemap_check_ordinary (map)->start_location)
-	  & ((1 << map->d.ordinary.column_bits) - 1));
+  return ((loc - ord_map->start_location)
+	  & ((1 << ord_map->column_bits) - 1));
 }
 
 /* Return the location of the last source line within an ordinary
    map.  */
 inline source_location
-LAST_SOURCE_LINE_LOCATION (const struct line_map *map)
+LAST_SOURCE_LINE_LOCATION (const line_map_ordinary *map)
 {
-  return (((linemap_check_ordinary (map)[1].start_location - 1
+  return (((map[1].start_location - 1
 	    - map->start_location)
-	   & ~((1 << map->d.ordinary.column_bits) - 1))
+	   & ~((1 << map->column_bits) - 1))
 	  + map->start_location);
 }
 
 /* Returns the last source line number within an ordinary map.  This
    is the (last) line of the #include, or other directive, that caused
    a map change.  */
-inline linenum_type LAST_SOURCE_LINE (const struct line_map *map)
+inline linenum_type LAST_SOURCE_LINE (const line_map_ordinary *map)
 {
   return SOURCE_LINE (map, LAST_SOURCE_LINE_LOCATION (map));
 }
 
 /* Return the last column number within an ordinary map.  */
 
-inline linenum_type LAST_SOURCE_COLUMN (const struct line_map *map)
+inline linenum_type LAST_SOURCE_COLUMN (const line_map_ordinary *map)
 {
   return SOURCE_COLUMN (map, LAST_SOURCE_LINE_LOCATION (map));
 }
@@ -902,18 +919,18 @@ inline linenum_type LAST_SOURCE_COLUMN (const struct line_map *map)
 /* Returns the map a given map was included from, or NULL if the map
    belongs to the main file, i.e, a file that wasn't included by
    another one.  */
-inline struct line_map *
-INCLUDED_FROM (struct line_maps *set, const struct line_map *map)
+inline line_map_ordinary *
+INCLUDED_FROM (struct line_maps *set, const line_map_ordinary *ord_map)
 {
-  return ((linemap_check_ordinary (map)->d.ordinary.included_from == -1)
+  return ((ord_map->included_from == -1)
 	  ? NULL
-	  : (&LINEMAPS_ORDINARY_MAPS (set)[(map)->d.ordinary.included_from]));
+	  : LINEMAPS_ORDINARY_MAP_AT (set, ord_map->included_from));
 }
 
 /* True if the map is at the bottom of the include stack.  */
-inline bool MAIN_FILE_P (const struct line_map *map)
+inline bool MAIN_FILE_P (const line_map_ordinary *ord_map)
 {
-  return linemap_check_ordinary (map)->d.ordinary.included_from < 0;
+  return ord_map->included_from < 0;
 }
 
 /* Encode and return a source_location from a column number. The
@@ -926,7 +943,7 @@ linemap_position_for_column (struct line_maps *, unsigned int);
 /* Encode and return a source location from a given line and
    column.  */
 source_location
-linemap_position_for_line_and_column (const struct line_map *,
+linemap_position_for_line_and_column (const line_map_ordinary *,
 				      linenum_type, unsigned int);
 
 /* Encode and return a source_location starting from location LOC and
@@ -938,24 +955,24 @@ linemap_position_for_loc_and_offset (struct line_maps *set,
 				     unsigned int offset);
 
 /* Return the file this map is for.  */
-inline const char * LINEMAP_FILE (const struct line_map *map)
+inline const char * LINEMAP_FILE (const line_map_ordinary *ord_map)
 {
-  return linemap_check_ordinary (map)->d.ordinary.to_file;
+  return ord_map->to_file;
 }
 
 /* Return the line number this map started encoding location from.  */
-inline linenum_type LINEMAP_LINE (const struct line_map *map)
+inline linenum_type LINEMAP_LINE (const line_map_ordinary *ord_map)
 {
-  return linemap_check_ordinary (map)->d.ordinary.to_line;
+  return ord_map->to_line;
 }
 
 /* Return a positive value if map encodes locations from a system
    header, 0 otherwise. Returns 1 if MAP encodes locations in a
    system header and 2 if it encodes locations in a C system header
    that therefore needs to be extern "C" protected in C++.  */
-inline unsigned char LINEMAP_SYSP (const struct line_map *map)
+inline unsigned char LINEMAP_SYSP (const line_map_ordinary *ord_map)
 {
-  return linemap_check_ordinary (map)->d.ordinary.sysp;
+  return ord_map->sysp;
 }
 
 /* Return a positive value if PRE denotes the location of a token that
@@ -1053,7 +1070,7 @@ enum location_resolution_kind
 source_location linemap_resolve_location (struct line_maps *,
 					  source_location loc,
 					  enum location_resolution_kind lrk,
-					  const struct line_map **loc_map);
+					  const line_map_ordinary **loc_map);
 
 /* Suppose that LOC is the virtual location of a token coming from the
    expansion of a macro M.  This function then steps up to get the
diff --git a/libcpp/internal.h b/libcpp/internal.h
index c2d0816..95cf9c2 100644
--- a/libcpp/internal.h
+++ b/libcpp/internal.h
@@ -68,7 +68,7 @@ struct cset_converter
 
 #define CPP_INCREMENT_LINE(PFILE, COLS_HINT) do { \
     const struct line_maps *line_table = PFILE->line_table; \
-    const struct line_map *map = \
+    const struct line_map_ordinary *map = \
       LINEMAPS_LAST_ORDINARY_MAP (line_table); \
     linenum_type line = SOURCE_LINE (map, line_table->highest_line); \
     linemap_line_start (PFILE->line_table, line + 1, COLS_HINT); \
@@ -833,10 +833,10 @@ ufputs (const unsigned char *s, FILE *f)
    of the macro, rather than the the location of the first character
    of the macro.  NUM_TOKENS is the number of tokens that are part of
    the replacement-list of MACRO.  */
-const struct line_map *linemap_enter_macro (struct line_maps *,
-					    struct cpp_hashnode*,
-					    source_location,
-					    unsigned int);
+const line_map_macro *linemap_enter_macro (struct line_maps *,
+					   struct cpp_hashnode*,
+					   source_location,
+					   unsigned int);
 
 /* Create and return a virtual location for a token that is part of a
    macro expansion-list at a macro expansion point.  See the comment
@@ -860,7 +860,7 @@ const struct line_map *linemap_enter_macro (struct line_maps *,
    MACRO_DEFINITION_LOC is the location in the macro definition,
    either of the token itself or of a macro parameter that it
    replaces.  */
-source_location linemap_add_macro_token (const struct line_map *,
+source_location linemap_add_macro_token (const line_map_macro *,
 					 unsigned int,
 					 source_location,
 					 source_location);
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index fd16c24..e262df4 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -26,25 +26,25 @@ along with this program; see the file COPYING3.  If not see
 #include "internal.h"
 #include "hashtab.h"
 
-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 void trace_include (const struct line_maps *, const line_map_ordinary *);
+static const line_map_ordinary * linemap_ordinary_map_lookup (struct line_maps *,
+							      source_location);
+static const line_map_macro* 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);
+(const line_map_macro *, source_location);
 static source_location linemap_macro_map_loc_unwind_toward_spelling
-(const struct line_map*, source_location);
+(const line_map_macro *, source_location);
 static source_location linemap_macro_map_loc_to_exp_point
-(const struct line_map*, source_location);
+(const line_map_macro *, source_location);
 static source_location linemap_macro_loc_to_spelling_point
-(struct line_maps *, source_location, const struct line_map **);
+(struct line_maps *, source_location, const line_map_ordinary **);
 static source_location linemap_macro_loc_to_def_point (struct line_maps *,
 						       source_location,
-						       const struct line_map **);
+						       const line_map_ordinary **);
 static source_location linemap_macro_loc_to_exp_point (struct line_maps *,
 						       source_location,
-						       const struct line_map **);
+						       const line_map_ordinary **);
 
 /* Counters defined in macro.c.  */
 extern unsigned num_expanded_macros_counter;
@@ -191,7 +191,7 @@ linemap_init (struct line_maps *set,
 void
 linemap_check_files_exited (struct line_maps *set)
 {
-  const struct line_map *map;
+  const line_map_ordinary *map;
   /* Depending upon whether we are handling preprocessed input or
      not, this can be a user error or an ICE.  */
   for (map = LINEMAPS_LAST_ORDINARY_MAP (set);
@@ -227,6 +227,10 @@ new_linemap (struct line_maps *set,
       line_map_round_alloc_size_func round_alloc_size =
 	set->round_alloc_size;
 
+      size_t map_size = (macro_map_p
+			 ? sizeof (line_map_macro)
+			 : sizeof (line_map_ordinary));
+
       /* We are going to execute some dance to try to reduce the
 	 overhead of the memory allocator, in case we are using the
 	 ggc-page.c one.
@@ -237,7 +241,7 @@ new_linemap (struct line_maps *set,
 
       alloc_size =
 	(2 * LINEMAPS_ALLOCATED (set, macro_map_p) +  256)
-	* sizeof (struct line_map);
+	* map_size;
 
       /* Get the actual size of memory that is going to be allocated
 	 by the allocator.  */
@@ -248,25 +252,37 @@ new_linemap (struct line_maps *set,
 	 Let's get back to the number of macro map that amounts
 	 to.  */
       LINEMAPS_ALLOCATED (set, macro_map_p) =
-	alloc_size / (sizeof (struct line_map));
+	alloc_size / map_size;
 
       /* And now let's really do the re-allocation.  */
-      LINEMAPS_MAPS (set, macro_map_p) =
-	(struct line_map *) (*reallocator)
-	(LINEMAPS_MAPS (set, macro_map_p),
-	 (LINEMAPS_ALLOCATED (set, macro_map_p)
-	  * sizeof (struct line_map)));
-
-      result =
-	&LINEMAPS_MAPS (set, macro_map_p)[LINEMAPS_USED (set, macro_map_p)];
+      if (macro_map_p)
+	{
+	  set->info_macro.maps
+	    = (line_map_macro *) (*reallocator) (set->info_macro.maps,
+						 (LINEMAPS_ALLOCATED (set, macro_map_p)
+						  * map_size));
+	  result = &set->info_macro.maps[LINEMAPS_USED (set, macro_map_p)];
+	}
+      else
+	{
+	  set->info_ordinary.maps =
+	    (line_map_ordinary *) (*reallocator) (set->info_ordinary.maps,
+						  (LINEMAPS_ALLOCATED (set, macro_map_p)
+						   * map_size));
+	  result = &set->info_ordinary.maps[LINEMAPS_USED (set, macro_map_p)];
+	}
       memset (result, 0,
 	      ((LINEMAPS_ALLOCATED (set, macro_map_p)
 		- LINEMAPS_USED (set, macro_map_p))
-	       * sizeof (struct line_map)));
+	       * map_size));
     }
   else
-    result =
-      &LINEMAPS_MAPS (set, macro_map_p)[LINEMAPS_USED (set, macro_map_p)];
+    {
+      if (macro_map_p)
+	result = &set->info_macro.maps[LINEMAPS_USED (set, macro_map_p)];
+      else
+	result = &set->info_ordinary.maps[LINEMAPS_USED (set, macro_map_p)];
+    }
 
   LINEMAPS_USED (set, macro_map_p)++;
 
@@ -291,7 +307,6 @@ const struct line_map *
 linemap_add (struct line_maps *set, enum lc_reason reason,
 	     unsigned int sysp, const char *to_file, linenum_type to_line)
 {
-  struct line_map *map;
   source_location start_location = set->highest_location + 1;
 
   linemap_assert (!(LINEMAPS_ORDINARY_USED (set)
@@ -311,7 +326,8 @@ linemap_add (struct line_maps *set, enum lc_reason reason,
       return NULL;
     }
 
-  map = new_linemap (set, reason);
+  linemap_assert (reason != LC_ENTER_MACRO);
+  line_map_ordinary *map = linemap_check_ordinary (new_linemap (set, reason));
 
   if (to_file && *to_file == '\0' && reason != LC_RENAME_VERBATIM)
     to_file = "<stdin>";
@@ -325,7 +341,7 @@ linemap_add (struct line_maps *set, enum lc_reason reason,
 	 location inside the "includer" right after the #include
 	 "included", this variable points the map in use right before the
 	 #include "included", inside the same "includer" file.  */
-      struct line_map *from;
+      line_map_ordinary *from;
       bool error;
 
       if (MAIN_FILE_P (map - 1))
@@ -365,7 +381,6 @@ linemap_add (struct line_maps *set, enum lc_reason reason,
 	}
     }
 
-  linemap_assert (reason != LC_ENTER_MACRO);
   ORDINARY_MAP_IN_SYSTEM_HEADER_P (map) = sysp;
   MAP_START_LOCATION (map) = start_location;
   ORDINARY_MAP_FILE_NAME (map) = to_file;
@@ -429,11 +444,11 @@ linemap_tracks_macro_expansion_locs_p (struct line_maps *set)
    this function cannot encode {line,column} pairs into locations of
    macro tokens anymore.  */
 
-const struct line_map *
+const line_map_macro *
 linemap_enter_macro (struct line_maps *set, struct cpp_hashnode *macro_node,
 		     source_location expansion, unsigned int num_tokens)
 {
-  struct line_map *map;
+  line_map_macro *map;
   source_location start_location;
   /* Cast away extern "C" from the type of xrealloc.  */
   line_map_realloc reallocator = (set->reallocator
@@ -447,7 +462,7 @@ linemap_enter_macro (struct line_maps *set, struct cpp_hashnode *macro_node,
     /* We ran out of macro map space.   */
     return NULL;
 
-  map = new_linemap (set, LC_ENTER_MACRO);
+  map = linemap_check_macro (new_linemap (set, LC_ENTER_MACRO));
 
   MAP_START_LOCATION (map) = start_location;
   MACRO_MAP_MACRO (map) = macro_node;
@@ -489,7 +504,7 @@ linemap_enter_macro (struct line_maps *set, struct cpp_hashnode *macro_node,
    replaces.  */
 
 source_location
-linemap_add_macro_token (const struct line_map *map,
+linemap_add_macro_token (const line_map_macro *map,
 			 unsigned int token_no,
 			 source_location orig_loc,
 			 source_location orig_parm_replacement_loc)
@@ -516,7 +531,7 @@ source_location
 linemap_line_start (struct line_maps *set, linenum_type to_line,
 		    unsigned int max_column_hint)
 {
-  struct line_map *map = LINEMAPS_LAST_ORDINARY_MAP (set);
+  line_map_ordinary *map = LINEMAPS_LAST_ORDINARY_MAP (set);
   source_location highest = set->highest_location;
   source_location r;
   linenum_type last_line =
@@ -559,11 +574,12 @@ linemap_line_start (struct line_maps *set, linenum_type to_line,
       if (line_delta < 0
 	  || last_line != ORDINARY_MAP_STARTING_LINE_NUMBER (map)
 	  || SOURCE_COLUMN (map, highest) >= (1U << column_bits))
-	map = (struct line_map *) linemap_add (set, LC_RENAME,
-					       ORDINARY_MAP_IN_SYSTEM_HEADER_P
-					       (map),
-					       ORDINARY_MAP_FILE_NAME (map),
-					       to_line);
+	map = linemap_check_ordinary
+	        (const_cast <line_map *>
+		  (linemap_add (set, LC_RENAME,
+				ORDINARY_MAP_IN_SYSTEM_HEADER_P (map),
+				ORDINARY_MAP_FILE_NAME (map),
+				to_line)));
       SET_ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map, column_bits);
       r = (MAP_START_LOCATION (map)
 	   + ((to_line - ORDINARY_MAP_STARTING_LINE_NUMBER (map))
@@ -607,7 +623,7 @@ linemap_position_for_column (struct line_maps *set, unsigned int to_column)
 	}
       else
 	{
-	  struct line_map *map = LINEMAPS_LAST_ORDINARY_MAP (set);
+	  line_map_ordinary *map = LINEMAPS_LAST_ORDINARY_MAP (set);
 	  r = linemap_line_start (set, SOURCE_LINE (map, r), to_column + 50);
 	}
     }
@@ -621,16 +637,16 @@ linemap_position_for_column (struct line_maps *set, unsigned int to_column)
    column.  */
 
 source_location
-linemap_position_for_line_and_column (const struct line_map *map,
+linemap_position_for_line_and_column (const line_map_ordinary *ord_map,
 				      linenum_type line,
 				      unsigned column)
 {
-  linemap_assert (ORDINARY_MAP_STARTING_LINE_NUMBER (map) <= line);
+  linemap_assert (ORDINARY_MAP_STARTING_LINE_NUMBER (ord_map) <= line);
 
-  return (MAP_START_LOCATION (map)
-	  + ((line - ORDINARY_MAP_STARTING_LINE_NUMBER (map))
-	     << ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map))
-	  + (column & ((1 << ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map)) - 1)));
+  return (MAP_START_LOCATION (ord_map)
+	  + ((line - ORDINARY_MAP_STARTING_LINE_NUMBER (ord_map))
+	     << ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (ord_map))
+	  + (column & ((1 << ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (ord_map)) - 1)));
 }
 
 /* Encode and return a source_location starting from location LOC and
@@ -642,7 +658,7 @@ linemap_position_for_loc_and_offset (struct line_maps *set,
 				     source_location loc,
 				     unsigned int offset)
 {
-  const struct line_map * map = NULL;
+  const line_map_ordinary * map = NULL;
 
   /* This function does not support virtual locations yet.  */
   if (linemap_assert_fails
@@ -671,7 +687,8 @@ linemap_position_for_loc_and_offset (struct line_maps *set,
       return loc;
 
   offset += SOURCE_COLUMN (map, loc);
-  if (linemap_assert_fails (offset < (1u << map->d.ordinary.column_bits)))
+  if (linemap_assert_fails
+        (offset < (1u << map->column_bits)))
     return loc;
 
   source_location r = 
@@ -703,11 +720,11 @@ linemap_lookup (struct line_maps *set, source_location line)
    monotonic increasing, and so the list is sorted and we can use a
    binary search.  */
 
-static const struct line_map *
+static const line_map_ordinary *
 linemap_ordinary_map_lookup (struct line_maps *set, source_location line)
 {
   unsigned int md, mn, mx;
-  const struct line_map *cached, *result;
+  const line_map_ordinary *cached, *result;
 
   if (IS_ADHOC_LOC (line))
     line = set->location_adhoc_data_map.data[line & MAX_SOURCE_LOCATION].locus;
@@ -751,11 +768,11 @@ linemap_ordinary_map_lookup (struct line_maps *set, source_location line)
    monotonic decreasing, and so the list is sorted and we can use a
    binary search.  */
 
-static const struct line_map*
+static const line_map_macro *
 linemap_macro_map_lookup (struct line_maps *set, source_location line)
 {
   unsigned int md, mn, mx;
-  const struct line_map *cached, *result;
+  const struct line_map_macro *cached, *result;
 
   if (IS_ADHOC_LOC (line))
     line = set->location_adhoc_data_map.data[line & MAX_SOURCE_LOCATION].locus;
@@ -811,7 +828,7 @@ linemap_macro_expansion_map_p (const struct line_map *map)
    line-map.h to understand what a macro expansion point is.  */
 
 static source_location
-linemap_macro_map_loc_to_exp_point (const struct line_map *map,
+linemap_macro_map_loc_to_exp_point (const line_map_macro *map,
 				    source_location location ATTRIBUTE_UNUSED)
 {
   linemap_assert (linemap_macro_expansion_map_p (map)
@@ -831,7 +848,7 @@ linemap_macro_map_loc_to_exp_point (const struct line_map *map,
    macro.  */
 
 static source_location
-linemap_macro_map_loc_to_def_point (const struct line_map *map,
+linemap_macro_map_loc_to_def_point (const line_map_macro *map,
 				    source_location location)
 {
   unsigned token_no;
@@ -855,7 +872,7 @@ linemap_macro_map_loc_to_def_point (const struct line_map *map,
    In other words, this returns the xI location presented in the
    comments of line_map_macro above.  */
 source_location
-linemap_macro_map_loc_unwind_toward_spelling (const struct line_map* map,
+linemap_macro_map_loc_unwind_toward_spelling (const line_map_macro* map,
 					      source_location location)
 {
   unsigned token_no;
@@ -882,7 +899,7 @@ int
 linemap_get_expansion_line (struct line_maps *set,
 			    source_location location)
 {
-  const struct line_map *map = NULL;
+  const line_map_ordinary *map = NULL;
 
   if (IS_ADHOC_LOC (location))
     location = set->location_adhoc_data_map.data[location
@@ -910,7 +927,7 @@ const char*
 linemap_get_expansion_filename (struct line_maps *set,
 				source_location location)
 {
-  const struct line_map *map = NULL;
+  const struct line_map_ordinary *map = NULL;
 
   if (IS_ADHOC_LOC (location))
     location = set->location_adhoc_data_map.data[location
@@ -928,7 +945,7 @@ linemap_get_expansion_filename (struct line_maps *set,
 /* Return the name of the macro associated to MACRO_MAP.  */
 
 const char*
-linemap_map_get_macro_name (const struct line_map* macro_map)
+linemap_map_get_macro_name (const line_map_macro *macro_map)
 {
   linemap_assert (macro_map && linemap_macro_expansion_map_p (macro_map));
   return (const char*) NODE_NAME (MACRO_MAP_MACRO (macro_map));
@@ -965,16 +982,18 @@ linemap_location_in_system_header_p (struct line_maps *set,
 	{
 	  if (!linemap_macro_expansion_map_p (map))
 	    /* It's a normal token.  */
-	    return LINEMAP_SYSP (map);
+	    return LINEMAP_SYSP (linemap_check_ordinary (map));
 	  else
 	    {
+	      const line_map_macro *macro_map = linemap_check_macro (map);
+
 	      /* It's a token resulting from a macro expansion.  */
 	      source_location loc =
-		linemap_macro_map_loc_unwind_toward_spelling (map, location);
+		linemap_macro_map_loc_unwind_toward_spelling (macro_map, location);
 	      if (loc < RESERVED_LOCATION_COUNT)
 		/* This token might come from a built-in macro.  Let's
 		   look at where that macro got expanded.  */
-		location = linemap_macro_map_loc_to_exp_point (map, location);
+		location = linemap_macro_map_loc_to_exp_point (macro_map, location);
 	      else
 		location = loc;
 	    }
@@ -1025,12 +1044,14 @@ first_map_in_common_1 (struct line_maps *set,
     {
       if (MAP_START_LOCATION (map0) < MAP_START_LOCATION (map1))
 	{
-	  l0 = linemap_macro_map_loc_to_exp_point (map0, l0);
+	  l0 = linemap_macro_map_loc_to_exp_point (linemap_check_macro (map0),
+						   l0);
 	  map0 = linemap_lookup (set, l0);
 	}
       else
 	{
-	  l1 = linemap_macro_map_loc_to_exp_point (map1, l1);
+	  l1 = linemap_macro_map_loc_to_exp_point (linemap_check_macro (map1),
+						   l1);
 	  map1 = linemap_lookup (set, l1);
 	}
     }
@@ -1120,7 +1141,7 @@ linemap_compare_locations (struct line_maps *set,
 /* Print an include trace, for e.g. the -H option of the preprocessor.  */
 
 static void
-trace_include (const struct line_maps *set, const struct line_map *map)
+trace_include (const struct line_maps *set, const line_map_ordinary *map)
 {
   unsigned int i = set->depth;
 
@@ -1138,7 +1159,7 @@ trace_include (const struct line_maps *set, const struct line_map *map)
 static source_location
 linemap_macro_loc_to_spelling_point (struct line_maps *set,
 				     source_location location,
-				     const struct line_map **original_map)
+				     const line_map_ordinary **original_map)
 {
   struct line_map *map;
 
@@ -1150,16 +1171,18 @@ linemap_macro_loc_to_spelling_point (struct line_maps *set,
 
   while (true)
     {
-      map = (struct line_map*) linemap_lookup (set, location);
+      map = const_cast <line_map *> (linemap_lookup (set, location));
       if (!linemap_macro_expansion_map_p (map))
 	break;
 
-      location =
-	linemap_macro_map_loc_unwind_toward_spelling (map, location);
+      location
+	= linemap_macro_map_loc_unwind_toward_spelling
+	    (linemap_check_macro (map),
+	     location);
     }
 
   if (original_map)
-    *original_map = map;
+    *original_map = linemap_check_ordinary (map);
   return location;
 }
 
@@ -1176,7 +1199,7 @@ linemap_macro_loc_to_spelling_point (struct line_maps *set,
 static source_location
 linemap_macro_loc_to_def_point (struct line_maps *set,
 				source_location location,
-				const struct line_map **original_map)
+				const line_map_ordinary **original_map)
 {
   struct line_map *map;
 
@@ -1188,16 +1211,17 @@ linemap_macro_loc_to_def_point (struct line_maps *set,
 
   while (true)
     {
-      map = (struct line_map*) linemap_lookup (set, location);
+      map = const_cast <line_map *> (linemap_lookup (set, location));
       if (!linemap_macro_expansion_map_p (map))
 	break;
 
       location =
-	linemap_macro_map_loc_to_def_point (map, location);
+	linemap_macro_map_loc_to_def_point (linemap_check_macro (map),
+					    location);
     }
 
   if (original_map)
-    *original_map = map;
+    *original_map = linemap_check_ordinary (map);
   return location;
 }
 
@@ -1218,7 +1242,7 @@ linemap_macro_loc_to_def_point (struct line_maps *set,
 static source_location
 linemap_macro_loc_to_exp_point (struct line_maps *set,
 				source_location location,
-				const struct line_map **original_map)
+				const line_map_ordinary **original_map)
 {
   struct line_map *map;
 
@@ -1230,14 +1254,15 @@ linemap_macro_loc_to_exp_point (struct line_maps *set,
 
   while (true)
     {
-      map = (struct line_map*) linemap_lookup (set, location);
+      map = const_cast <line_map *> (linemap_lookup (set, location));
       if (!linemap_macro_expansion_map_p (map))
 	break;
-      location = linemap_macro_map_loc_to_exp_point (map, location);
+      location = linemap_macro_map_loc_to_exp_point (linemap_check_macro (map),
+						     location);
     }
 
   if (original_map)
-    *original_map = map;
+    *original_map = linemap_check_ordinary (map);
   return location;
 }
 
@@ -1293,7 +1318,7 @@ source_location
 linemap_resolve_location (struct line_maps *set,
 			  source_location loc,
 			  enum location_resolution_kind lrk,
-			  const struct line_map **map)
+			  const line_map_ordinary **map)
 {
   if (IS_ADHOC_LOC (loc))
     loc = set->location_adhoc_data_map.data[loc & MAX_SOURCE_LOCATION].locus;
@@ -1343,18 +1368,19 @@ linemap_unwind_toward_expansion (struct line_maps *set,
 				 const struct line_map **map)
 {
   source_location resolved_location;
+  const line_map_macro *macro_map = linemap_check_macro (*map);
   const struct line_map *resolved_map;
 
   if (IS_ADHOC_LOC (loc))
     loc = set->location_adhoc_data_map.data[loc & MAX_SOURCE_LOCATION].locus;
 
   resolved_location =
-    linemap_macro_map_loc_unwind_toward_spelling (*map, loc);
+    linemap_macro_map_loc_unwind_toward_spelling (macro_map, loc);
   resolved_map = linemap_lookup (set, resolved_location);
 
   if (!linemap_macro_expansion_map_p (resolved_map))
     {
-      resolved_location = linemap_macro_map_loc_to_exp_point (*map, loc);
+      resolved_location = linemap_macro_map_loc_to_exp_point (macro_map, loc);
       resolved_map = linemap_lookup (set, resolved_location);
     }
 
@@ -1382,7 +1408,8 @@ linemap_unwind_to_first_non_reserved_loc (struct line_maps *set,
 					  const struct line_map **map)
 {
   source_location resolved_loc;
-  const struct line_map *map0 = NULL, *map1 = NULL;
+  const struct line_map *map0 = NULL;
+  const line_map_ordinary *map1 = NULL;
 
   if (IS_ADHOC_LOC (loc))
     loc = set->location_adhoc_data_map.data[loc & MAX_SOURCE_LOCATION].locus;
@@ -1453,10 +1480,12 @@ linemap_expand_location (struct line_maps *set,
       if (linemap_location_from_macro_expansion_p (set, loc))
 	abort ();
 
-      xloc.file = LINEMAP_FILE (map);
-      xloc.line = SOURCE_LINE (map, loc);
-      xloc.column = SOURCE_COLUMN (map, loc);
-      xloc.sysp = LINEMAP_SYSP (map) != 0;
+      const line_map_ordinary *ord_map = linemap_check_ordinary (map);
+
+      xloc.file = LINEMAP_FILE (ord_map);
+      xloc.line = SOURCE_LINE (ord_map, loc);
+      xloc.column = SOURCE_COLUMN (ord_map, loc);
+      xloc.sysp = LINEMAP_SYSP (ord_map) != 0;
     }
 
   return xloc;
@@ -1474,7 +1503,7 @@ linemap_dump (FILE *stream, struct line_maps *set, unsigned ix, bool is_macro)
       = { "LC_ENTER", "LC_LEAVE", "LC_RENAME", "LC_RENAME_VERBATIM",
 	  "LC_ENTER_MACRO" };
   const char *reason;
-  struct line_map *map;
+  const line_map *map;
 
   if (stream == NULL)
     stream = stderr;
@@ -1488,26 +1517,32 @@ linemap_dump (FILE *stream, struct line_maps *set, unsigned ix, bool is_macro)
 
   fprintf (stream, "Map #%u [%p] - LOC: %u - REASON: %s - SYSP: %s\n",
 	   ix, (void *) map, map->start_location, reason,
-	   (!is_macro && ORDINARY_MAP_IN_SYSTEM_HEADER_P (map)) ? "yes" : "no");
+	   ((!is_macro
+	     && ORDINARY_MAP_IN_SYSTEM_HEADER_P (linemap_check_ordinary (map)))
+	    ? "yes" : "no"));
   if (!is_macro)
     {
+      const line_map_ordinary *ord_map = linemap_check_ordinary (map);
       unsigned includer_ix;
-      struct line_map *includer_map;
+      const line_map_ordinary *includer_map;
 
-      includer_ix = ORDINARY_MAP_INCLUDER_FILE_INDEX (map);
+      includer_ix = ORDINARY_MAP_INCLUDER_FILE_INDEX (ord_map);
       includer_map = includer_ix < LINEMAPS_ORDINARY_USED (set)
 		     ? LINEMAPS_ORDINARY_MAP_AT (set, includer_ix)
 		     : NULL;
 
-      fprintf (stream, "File: %s:%d\n", ORDINARY_MAP_FILE_NAME (map),
-	       ORDINARY_MAP_STARTING_LINE_NUMBER (map));
+      fprintf (stream, "File: %s:%d\n", ORDINARY_MAP_FILE_NAME (ord_map),
+	       ORDINARY_MAP_STARTING_LINE_NUMBER (ord_map));
       fprintf (stream, "Included from: [%d] %s\n", includer_ix,
 	       includer_map ? ORDINARY_MAP_FILE_NAME (includer_map) : "None");
     }
   else
-    fprintf (stream, "Macro: %s (%u tokens)\n",
-	     linemap_map_get_macro_name (map),
-	     MACRO_MAP_NUM_MACRO_TOKENS (map));
+    {
+      const line_map_macro *macro_map = linemap_check_macro (map);
+      fprintf (stream, "Macro: %s (%u tokens)\n",
+	       linemap_map_get_macro_name (macro_map),
+	       MACRO_MAP_NUM_MACRO_TOKENS (macro_map));
+    }
 
   fprintf (stream, "\n");
 }
@@ -1521,7 +1556,7 @@ linemap_dump_location (struct line_maps *set,
 		       source_location loc,
 		       FILE *stream)
 {
-  const struct line_map *map;
+  const line_map_ordinary *map;
   source_location location;
   const char *path = "", *from = "";
   int l = -1, c = -1, s = -1, e = -1;
@@ -1578,7 +1613,7 @@ linemap_get_file_highest_location (struct line_maps *set,
   int i;
   for (i = set->info_ordinary.used - 1; i >= 0; --i)
     {
-      const char *fname = set->info_ordinary.maps[i].d.ordinary.to_file;
+      const char *fname = set->info_ordinary.maps[i].to_file;
       if (fname && !filename_cmp (fname, file_name))
 	break;
     }
@@ -1610,16 +1645,16 @@ linemap_get_statistics (struct line_maps *set,
     macro_maps_allocated_size, macro_maps_used_size,
     macro_maps_locations_size = 0, duplicated_macro_maps_locations_size = 0;
 
-  struct line_map *cur_map;
+  const line_map_macro *cur_map;
 
   ordinary_maps_allocated_size =
-    LINEMAPS_ORDINARY_ALLOCATED (set) * sizeof (struct line_map);
+    LINEMAPS_ORDINARY_ALLOCATED (set) * sizeof (struct line_map_ordinary);
 
   ordinary_maps_used_size =
-    LINEMAPS_ORDINARY_USED (set) * sizeof (struct line_map);
+    LINEMAPS_ORDINARY_USED (set) * sizeof (struct line_map_ordinary);
 
   macro_maps_allocated_size =
-    LINEMAPS_MACRO_ALLOCATED (set) * sizeof (struct line_map);
+    LINEMAPS_MACRO_ALLOCATED (set) * sizeof (struct line_map_macro);
 
   for (cur_map = LINEMAPS_MACRO_MAPS (set);
        cur_map && cur_map <= LINEMAPS_LAST_MACRO_MAP (set);
@@ -1642,7 +1677,7 @@ linemap_get_statistics (struct line_maps *set,
     }
 
   macro_maps_used_size =
-    LINEMAPS_MACRO_USED (set) * sizeof (struct line_map);
+    LINEMAPS_MACRO_USED (set) * sizeof (struct line_map_macro);
 
   s->num_ordinary_maps_allocated = LINEMAPS_ORDINARY_ALLOCATED (set);
   s->num_ordinary_maps_used = LINEMAPS_ORDINARY_USED (set);
diff --git a/libcpp/macro.c b/libcpp/macro.c
index 1e0a0b5..f76e10b 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -141,7 +141,7 @@ static inline const cpp_token **tokens_buff_put_token_to (const cpp_token **,
                                                           const cpp_token *,
                                                           source_location,
                                                           source_location,
-                                                          const struct line_map *,
+                                                          const line_map_macro *,
                                                           unsigned int);
 
 static const cpp_token **tokens_buff_add_token (_cpp_buff *,
@@ -149,7 +149,7 @@ static const cpp_token **tokens_buff_add_token (_cpp_buff *,
 						const cpp_token *,
 						source_location,
 						source_location,
-						const struct line_map *,
+						const line_map_macro *,
 						unsigned int);
 static inline void tokens_buff_remove_last_token (_cpp_buff *);
 static void replace_args (cpp_reader *, cpp_hashnode *, cpp_macro *,
@@ -195,7 +195,9 @@ _cpp_warn_if_unused_macro (cpp_reader *pfile, cpp_hashnode *node,
       cpp_macro *macro = node->value.macro;
 
       if (!macro->used
-	  && MAIN_FILE_P (linemap_lookup (pfile->line_table, macro->line)))
+	  && MAIN_FILE_P (linemap_check_ordinary
+			    (linemap_lookup (pfile->line_table,
+					     macro->line))))
 	cpp_warning_with_line (pfile, CPP_W_UNUSED_MACROS, macro->line, 0,
 			       "macro \"%s\" is not used", NODE_NAME (node));
     }
@@ -453,7 +455,7 @@ builtin_macro (cpp_reader *pfile, cpp_hashnode *node, source_location loc)
 	 macro.  */
       source_location *virt_locs = NULL;
       _cpp_buff *token_buf = tokens_buff_new (pfile, 1, &virt_locs);
-      const line_map * map =
+      const line_map_macro * map =
 	linemap_enter_macro (pfile->line_table, node,
 					    token->src_loc, 1);
       tokens_buff_add_token (token_buf, virt_locs, token,
@@ -1167,7 +1169,7 @@ enter_macro_context (cpp_reader *pfile, cpp_hashnode *node,
 	    {
 	      unsigned int i;
 	      const cpp_token *src = macro->exp.tokens;
-	      const struct line_map *map;
+	      const line_map_macro *map;
 	      source_location *virt_locs = NULL;
 	      _cpp_buff *macro_tokens
 		= tokens_buff_new (pfile, tokens_count, &virt_locs);
@@ -1535,7 +1537,7 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
   _cpp_buff *buff = NULL;
   source_location *virt_locs = NULL;
   unsigned int exp_count;
-  const struct line_map *map = NULL;
+  const line_map_macro *map = NULL;
   int track_macro_exp;
 
   /* First, fully macro-expand arguments, calculating the number of
@@ -2066,7 +2068,7 @@ tokens_buff_put_token_to (const cpp_token **dest,
 			  const cpp_token *token,
 			  source_location virt_loc,
 			  source_location parm_def_loc,			  
-			  const struct line_map *map,
+			  const line_map_macro *map,
 			  unsigned int macro_token_index)
 {
   source_location macro_loc = virt_loc;
@@ -2111,7 +2113,7 @@ tokens_buff_add_token (_cpp_buff *buffer,
 		       const cpp_token *token,
 		       source_location virt_loc,
 		       source_location parm_def_loc,
-		       const struct line_map *map,
+		       const line_map_macro *map,
 		       unsigned int macro_token_index)
 {
   const cpp_token **result;
-- 
1.8.5.3

Re: [PATCH 4/4] Replace line_map union with C++ class hierarchy

[Jeff Law]

On 05/01/2015 06:56 PM, David Malcolm wrote:
> This patch eliminates the union in struct line_map in favor of
> a simple class hierarchy, making struct line_map a base class,
> with line_map_ordinary and line_map_macro subclasses.
>
> The patch eliminates all usage of linemap_check_ordinary and
> linemap_check_macro from line-map.h, updating return types and
> signatures throughout libcpp and gcc's usage of it to use the
> appropriate subclasses.
>
> This moves the checking of linemap kind from run-time to
> compile-time, and also implicitly documents everywhere where
> the code is expecting an ordinary map vs a macro map vs
> either kind of map.  I believe it makes the code significantly
> simpler: most of the accessor functions in line-map.h become
> trivial field-lookups.
>
> I attemped to use templates for maps_info, but was stymied by
> gengtype, so in the end I simply split it manually into
> maps_info_ordinary and maps_info_macro.  In theory it's just
> a vec<>, but vec.h is in gcc, and thus not available
> for use from libcpp.
>
> In a similar vein, gcc/is-a.h is presumably not usable
> from within libcpp.  If it were, there would be the following
> rough equivalences:
>
> ---------------------------------  --------------------------------
> line-map.h                         is-a.h
> ---------------------------------  --------------------------------
> linemap_check_ordinary (m)         as_a <line_map_ordinary *> (m)
> linemap_check_macro (m)            as_a <line_map_macro *> (m)
> linemap_macro_expansion_map_p (m)  (M ? is_a <line_map_macro *> (m)
>                                        : false)
> ---------------------------------  --------------------------------
>
> There are numerous places in libcpp that offset a
> line_map * using array notation to get the next/prev line_map of the
> same kind, e.g.:
> MAP_START_LOCATION (&cached[1])
> which breaks due to the different sizes of line_map vs its subclasses.
>
> On x86_64 host, before:
> (gdb) p sizeof(line_map)
> $1 = 40
>
> after:
> (gdb) p sizeof(line_map)
> $1 = 8
> (gdb) p sizeof(line_map_ordinary)
> $2 = 32
> (gdb) p sizeof(line_map_macro)
> $3 = 40
>
> Tracking down all of these array-based offsets to use a pointer to the
> appropriate subclass (and thus use the correct offset) was rather
> involved, but I believe the patch fixes them all now.
>
> (the patch thus also gives a very modest saving of 8 bytes per ordinary
> line map).
>
> I've tried to use the naming convention "ord_map" and "macro_map"
> whenever the typesystem ensures we're dealing with such a map,
> wherever this is doable without needing to touch lines of code that
> would otherwise not need touching by the patch.
>
> gcc/ChangeLog:
> 	* diagnostic.c (diagnostic_report_current_module): Strengthen
> 	local "new_map" from const line_map * to
> 	const line_map_ordinary *.
> 	* genmatch.c (error_cb): Likewise for local "map".
> 	(output_line_directive): Likewise for local "map".
> 	* input.c (expand_location_1): Likewise for local "map".
> 	Pass NULL rather than &map to
> 	linemap_unwind_to_first_non_reserved_loc, since the value is never
> 	read from there, and the value written back not read from here.
> 	(is_location_from_builtin_token): Strengthen local "map" from
> 	const line_map * to const line_map_ordinary *.
> 	(dump_location_info): Strengthen locals "map" from
> 	line_map *, one to const line_map_ordinary *, the other
> 	to const line_map_macro *.
> 	* tree-diagnostic.c (loc_map_pair): Strengthen field "map" from
> 	const line_map * to const line_map_macro *.
> 	(maybe_unwind_expanded_macro_loc): Add a call to
> 	linemap_check_macro when writing to the "map" field of the
> 	loc_map_pair.
> 	Introduce local const line_map_ordinary * "ord_map", using it in
> 	place of "map" in the part of the function where we know we have
> 	an ordinary map.  Strengthen local "m" from const line_map * to
> 	const line_map_ordinary *.
>
> gcc/ada/ChangeLog:
> 	* gcc-interface/trans.c (Sloc_to_locus1): Strenghthen local "map"
> 	from line_map * to line_map_ordinary *.
>
> gcc/c-family/ChangeLog:
> 	* c-common.h (fe_file_change): Strengthen param from
> 	const line_map * to const line_map_ordinary *.
> 	(pp_file_change): Likewise.
> 	* c-lex.c (fe_file_change): Likewise.
> 	(cb_define): Use linemap_check_ordinary when invoking
> 	SOURCE_LINE.
> 	(cb_undef): Likewise.
> 	* c-opts.c (c_finish_options): Use linemap_check_ordinary when
> 	invoking cb_file_change.
> 	(c_finish_options): Likewise.
> 	(push_command_line_include): Likewise.
> 	(cb_file_change): Strengthen param "new_map" from
> 	const line_map * to const line_map_ordinary *.
> 	* c-ppoutput.c (cb_define): Likewise for local "map".
> 	(pp_file_change): Likewise for param "map" and local "from".
>
> gcc/fortran/ChangeLog:
> 	* cpp.c (maybe_print_line): Strengthen local "map" from
> 	const line_map * to const line_map_ordinary *.
> 	(cb_file_change): Likewise for param "map" and local "from".
> 	(cb_line_change): Likewise for local "map".
>
> libcpp/ChangeLog:
> 	* directives.c (do_line): Strengthen local "map" from
> 	const line_map * to const line_map_ordinary *.
> 	(do_linemarker): Likewise.
> 	(_cpp_do_file_change): Assert that we're not dealing with
> 	a macro map.  Introduce local "ord_map" via a call to
> 	linemap_check_ordinary, guarded within the check for
> 	non-NULL.  Use it for typesafety.
> 	* files.c (cpp_make_system_header): Strengthen local "map" from
> 	const line_map * to const line_map_ordinary *.
> 	* include/cpplib.h (struct cpp_callbacks): Likewise for second
> 	parameter of "file_change" callback.
> 	* include/line-map.h (struct line_map): Convert from a struct
> 	containing a union to a base class.
> 	(struct line_map_ordinary): Convert to a subclass of line_map.
> 	(struct line_map_macro): Likewise.
> 	(linemap_check_ordinary): Strengthen return type from line_map *
> 	to line_map_ordinary *, and add a const-variant.
> 	(linemap_check_macro): New pair of functions.
> 	(ORDINARY_MAP_STARTING_LINE_NUMBER): Strengthen param from
> 	const line_map * to const line_map_ordinary *, eliminating call
> 	to linemap_check_ordinary.  Likewise for the non-const variant.
> 	(ORDINARY_MAP_INCLUDER_FILE_INDEX): Likewise.
> 	(ORDINARY_MAP_IN_SYSTEM_HEADER_P): Likewise.
> 	(ORDINARY_MAP_NUMBER_OF_COLUMN_BITS): Likewise.
> 	(ORDINARY_MAP_FILE_NAME): Likewise.
> 	(MACRO_MAP_MACRO): Strengthen param from const line_map * to
> 	const line_map_macro *.  Likewise for the non-const variant.
> 	(MACRO_MAP_NUM_MACRO_TOKENS): Likewise.
> 	(MACRO_MAP_LOCATIONS): Likewise.
> 	(MACRO_MAP_EXPANSION_POINT_LOCATION): Likewise.
> 	(struct maps_info): Replace with...
> 	(struct maps_info_ordinary):...this and...
> 	(struct maps_info_macro): ...this.
> 	(struct line_maps): Convert fields "info_ordinary" and
> 	"info_macro" to the above new structs.
> 	(LINEMAPS_MAP_INFO): Delete both functions.
> 	(LINEMAPS_MAPS): Likewise.
> 	(LINEMAPS_ALLOCATED): Rewrite both variants to avoid using
> 	LINEMAPS_MAP_INFO.
> 	(LINEMAPS_USED): Likewise.
> 	(LINEMAPS_CACHE): Likewise.
> 	(LINEMAPS_MAP_AT): Likewise.
> 	(LINEMAPS_ORDINARY_MAPS): Strengthen return type from line_map *
> 	to line_map_ordinary *.
> 	(LINEMAPS_ORDINARY_MAP_AT): Likewise.
> 	(LINEMAPS_LAST_ORDINARY_MAP): Likewise.
> 	(LINEMAPS_LAST_ALLOCATED_ORDINARY_MAP): Likewise.
> 	(LINEMAPS_MACRO_MAPS): Strengthen return type from line_map * to
> 	line_map_macro *.
> 	(LINEMAPS_MACRO_MAP_AT): Likewise.
> 	(LINEMAPS_LAST_MACRO_MAP): Likewise.
> 	(LINEMAPS_LAST_ALLOCATED_MACRO_MAP): Likewise.
> 	(linemap_map_get_macro_name): Strengthen param from
> 	const line_map * to const line_map_macro *.
> 	(SOURCE_LINE): Strengthen first param from const line_map * to
> 	const line_map_ordinary *, removing call to
> 	linemap_check_ordinary.
> 	(SOURCE_COLUMN): Likewise.
> 	(LAST_SOURCE_LINE_LOCATION): Likewise.
> 	(LAST_SOURCE_LINE): Strengthen first param from const line_map *
> 	to const line_map_ordinary *.
> 	(LAST_SOURCE_COLUMN): Likewise.
> 	(INCLUDED_FROM): Strengthen return type from line_map * to
> 	line_map_ordinary *., and second param from const line_map *
> 	to const line_map_ordinary *, removing call to
> 	linemap_check_ordinary.
> 	(MAIN_FILE_P): Strengthen param from const line_map * to
> 	const line_map_ordinary *, removing call to
> 	linemap_check_ordinary.
> 	(linemap_position_for_line_and_column): Strengthen param from
> 	const line_map * to const line_map_ordinary *.
> 	(LINEMAP_FILE): Strengthen param from const line_map * to
> 	const line_map_ordinary *, removing call to
> 	linemap_check_ordinary.
> 	(LINEMAP_LINE): Likewise.
> 	(LINEMAP_SYSP): Likewise.
> 	(linemap_resolve_location): Strengthen final param from
> 	const line_map ** to const line_map_ordinary **.
> 	* internal.h (CPP_INCREMENT_LINE): Likewise for local "map".
> 	(linemap_enter_macro): Strengthen return type from
> 	const line_map * to const line_map_macro *.
> 	(linemap_add_macro_token): Likewise for first param.
> 	* line-map.c (linemap_check_files_exited): Strengthen local "map"
> 	from const line_map * to const line_map_ordinary *.
> 	(new_linemap): Introduce local "map_size" and use it when
> 	calculating how large the buffer should be.  Rewrite based
> 	on change of info_macro and info_ordinary into distinct types.
> 	(linemap_add): Strengthen locals "map" and "from" from line_map *
> 	to line_map_ordinary *.
> 	(linemap_enter_macro): Strengthen return type from
> 	const line_map * to const line_map_macro *, and local "map" from
> 	line_map * to line_map_macro *.
> 	(linemap_add_macro_token): Strengthen param "map" from
> 	const line_map * to const line_map_macro *.
> 	(linemap_line_start): Strengthen local "map" from line_map * to
> 	line_map_ordinary *.
> 	(linemap_position_for_column): Likewise.
> 	(linemap_position_for_line_and_column): Strengthen first param
> 	from const line_map * to const line_map_ordinary *.
> 	(linemap_position_for_loc_and_offset): Strengthen local "map" from
> 	const line_map * to const line_map_ordinary *.
> 	(linemap_ordinary_map_lookup): Likewise for return type and locals
> 	"cached" and "result".
> 	(linemap_macro_map_lookup): Strengthen return type and locals
> 	"cached" and "result" from const line_map * to
> 	const line_map_macro *.
> 	(linemap_macro_map_loc_to_exp_point): Likewise for param "map".
> 	(linemap_macro_map_loc_to_def_point): Likewise.
> 	(linemap_macro_map_loc_unwind_toward_spelling): Likewise.
> 	(linemap_get_expansion_line): Strengthen local "map" from
> 	const line_map * to const line_map_ordinary *.
> 	(linemap_get_expansion_filename): Likewise.
> 	(linemap_map_get_macro_name): Strengthen param from
> 	const line_map * to const line_map_macro *.
> 	(linemap_location_in_system_header_p): Add call to
> 	linemap_check_ordinary in region guarded by
> 	!linemap_macro_expansion_map_p.  Introduce local "macro_map" via
> 	linemap_check_macro in other region, using it in place of "map"
> 	for typesafety.
> 	(first_map_in_common_1): Add calls to linemap_check_macro.
> 	(trace_include): Strengthen param "map" from const line_map * to
> 	const line_map_ordinary *.
> 	(linemap_macro_loc_to_spelling_point): Strengthen final param from
> 	const line_map ** to const line_map_ordinary **.  Replace a
> 	C-style cast with a const_cast, and add calls to
> 	linemap_check_macro and linemap_check_ordinary.
> 	(linemap_macro_loc_to_def_point): Likewise.
> 	(linemap_macro_loc_to_exp_point): Likewise.
> 	(linemap_resolve_location): Strengthen final param from
> 	const line_map ** to const line_map_ordinary **.
> 	(linemap_unwind_toward_expansion): Introduce local "macro_map" via
> 	a checked cast and use it in place of *map.
> 	(linemap_unwind_to_first_non_reserved_loc): Strengthen local
> 	"map1" from const line_map * to const line_map_ordinary *.
> 	(linemap_expand_location): Introduce local "ord_map" via a checked
> 	cast and use it in place of map.
> 	(linemap_dump): Make local "map" const.  Strengthen local
> 	"includer_map" from line_map * to const line_map_ordinary *.
> 	Introduce locals "ord_map" and "macro_map" via checked casts and
> 	use them in place of "map" for typesafety.
> 	(linemap_dump_location): Strengthen local "map" from
> 	const line_map * to const line_map_ordinary *.
> 	(linemap_get_file_highest_location): Update for elimination of
> 	union.
> 	(linemap_get_statistics): Strengthen local "cur_map" from
> 	line_map * to const line_map_macro *.  Update uses of sizeof to
> 	use the appropriate line_map subclasses.
> 	* macro.c (_cpp_warn_if_unused_macro): Add call to
> 	linemap_check_ordinary.
> 	(builtin_macro): Strengthen local "map" from const line_map * to
> 	const line_map_macro *.
> 	(enter_macro_context): Likewise.
> 	(replace_args): Likewise.
> 	(tokens_buff_put_token_to): Likewise for param "map".
> 	(tokens_buff_add_token): Likewise.
> ---


> @@ -158,7 +158,7 @@ expand_location_1 (source_location loc,
>   	     location (toward the expansion point) that is not reserved;
>   	     that is, the first location that is in real source code.  */
>   	  loc = linemap_unwind_to_first_non_reserved_loc (line_table,
> -							  loc, &map);
> +							  loc, NULL);

Is that really correct?


   /* Return the map at a given index.  */
>   inline line_map *
>   LINEMAPS_MAP_AT (const line_maps *set, bool map_kind, int index)
>   {
> -  return &(LINEMAPS_MAPS (set, map_kind)[index]);
> +  if (map_kind)
> +    return &set->info_macro.maps[index];
> +  else
> +    return &set->info_ordinary.maps[index];
>   }
I see this pattern repeated regularly.  Any thoughts on how painful 
it'll be to drop the map_kind argument and instead not vary the kind?

I just spot checked all the mechanical changes.  Looks reasonable me to me.

Just need to sort out if the hunk with the call to l_u_t_f_n_r_l is 
correct and get the pre-reqs installed and I think this is good to go.


Jeff

Re: [PATCH 4/4] Replace line_map union with C++ class hierarchy

[David Malcolm]

On Mon, 2015-05-04 at 14:45 -0600, Jeff Law wrote:
> On 05/01/2015 06:56 PM, David Malcolm wrote:
> > This patch eliminates the union in struct line_map in favor of
> > a simple class hierarchy, making struct line_map a base class,
> > with line_map_ordinary and line_map_macro subclasses.
> >
> > The patch eliminates all usage of linemap_check_ordinary and
> > linemap_check_macro from line-map.h, updating return types and
> > signatures throughout libcpp and gcc's usage of it to use the
> > appropriate subclasses.
> >
> > This moves the checking of linemap kind from run-time to
> > compile-time, and also implicitly documents everywhere where
> > the code is expecting an ordinary map vs a macro map vs
> > either kind of map.  I believe it makes the code significantly
> > simpler: most of the accessor functions in line-map.h become
> > trivial field-lookups.
> >
> > I attemped to use templates for maps_info, but was stymied by
> > gengtype, so in the end I simply split it manually into
> > maps_info_ordinary and maps_info_macro.  In theory it's just
> > a vec<>, but vec.h is in gcc, and thus not available
> > for use from libcpp.
> >
> > In a similar vein, gcc/is-a.h is presumably not usable
> > from within libcpp.  If it were, there would be the following
> > rough equivalences:
> >
> > ---------------------------------  --------------------------------
> > line-map.h                         is-a.h
> > ---------------------------------  --------------------------------
> > linemap_check_ordinary (m)         as_a <line_map_ordinary *> (m)
> > linemap_check_macro (m)            as_a <line_map_macro *> (m)
> > linemap_macro_expansion_map_p (m)  (M ? is_a <line_map_macro *> (m)
> >                                        : false)
> > ---------------------------------  --------------------------------
> >
> > There are numerous places in libcpp that offset a
> > line_map * using array notation to get the next/prev line_map of the
> > same kind, e.g.:
> > MAP_START_LOCATION (&cached[1])
> > which breaks due to the different sizes of line_map vs its subclasses.
> >
> > On x86_64 host, before:
> > (gdb) p sizeof(line_map)
> > $1 = 40
> >
> > after:
> > (gdb) p sizeof(line_map)
> > $1 = 8
> > (gdb) p sizeof(line_map_ordinary)
> > $2 = 32
> > (gdb) p sizeof(line_map_macro)
> > $3 = 40
> >
> > Tracking down all of these array-based offsets to use a pointer to the
> > appropriate subclass (and thus use the correct offset) was rather
> > involved, but I believe the patch fixes them all now.
> >
> > (the patch thus also gives a very modest saving of 8 bytes per ordinary
> > line map).
> >
> > I've tried to use the naming convention "ord_map" and "macro_map"
> > whenever the typesystem ensures we're dealing with such a map,
> > wherever this is doable without needing to touch lines of code that
> > would otherwise not need touching by the patch.
> >
> > gcc/ChangeLog:
> > 	* diagnostic.c (diagnostic_report_current_module): Strengthen
> > 	local "new_map" from const line_map * to
> > 	const line_map_ordinary *.
> > 	* genmatch.c (error_cb): Likewise for local "map".
> > 	(output_line_directive): Likewise for local "map".
> > 	* input.c (expand_location_1): Likewise for local "map".
> > 	Pass NULL rather than &map to
> > 	linemap_unwind_to_first_non_reserved_loc, since the value is never
> > 	read from there, and the value written back not read from here.
> > 	(is_location_from_builtin_token): Strengthen local "map" from
> > 	const line_map * to const line_map_ordinary *.
> > 	(dump_location_info): Strengthen locals "map" from
> > 	line_map *, one to const line_map_ordinary *, the other
> > 	to const line_map_macro *.
> > 	* tree-diagnostic.c (loc_map_pair): Strengthen field "map" from
> > 	const line_map * to const line_map_macro *.
> > 	(maybe_unwind_expanded_macro_loc): Add a call to
> > 	linemap_check_macro when writing to the "map" field of the
> > 	loc_map_pair.
> > 	Introduce local const line_map_ordinary * "ord_map", using it in
> > 	place of "map" in the part of the function where we know we have
> > 	an ordinary map.  Strengthen local "m" from const line_map * to
> > 	const line_map_ordinary *.
> >
> > gcc/ada/ChangeLog:
> > 	* gcc-interface/trans.c (Sloc_to_locus1): Strenghthen local "map"
> > 	from line_map * to line_map_ordinary *.
> e>
> > gcc/c-family/ChangeLog:
> > 	* c-common.h (fe_file_change): Strengthen param from
> > 	const line_map * to const line_map_ordinary *.
> > 	(pp_file_change): Likewise.
> > 	* c-lex.c (fe_file_change): Likewise.
> > 	(cb_define): Use linemap_check_ordinary when invoking
> > 	SOURCE_LINE.
> > 	(cb_undef): Likewise.
> > 	* c-opts.c (c_finish_options): Use linemap_check_ordinary when
> > 	invoking cb_file_change.
> > 	(c_finish_options): Likewise.
> > 	(push_command_line_include): Likewise.
> > 	(cb_file_change): Strengthen param "new_map" from
> > 	const line_map * to const line_map_ordinary *.
> > 	* c-ppoutput.c (cb_define): Likewise for local "map".
> > 	(pp_file_change): Likewise for param "map" and local "from".
> >
> > gcc/fortran/ChangeLog:
> > 	* cpp.c (maybe_print_line): Strengthen local "map" from
> > 	const line_map * to const line_map_ordinary *.
> > 	(cb_file_change): Likewise for param "map" and local "from".
> > 	(cb_line_change): Likewise for local "map".
> >
> > libcpp/ChangeLog:
> > 	* directives.c (do_line): Strengthen local "map" from
> > 	const line_map * to const line_map_ordinary *.
> > 	(do_linemarker): Likewise.
> > 	(_cpp_do_file_change): Assert that we're not dealing with
> > 	a macro map.  Introduce local "ord_map" via a call to
> > 	linemap_check_ordinary, guarded within the check for
> > 	non-NULL.  Use it for typesafety.
> > 	* files.c (cpp_make_system_header): Strengthen local "map" from
> > 	const line_map * to const line_map_ordinary *.
> > 	* include/cpplib.h (struct cpp_callbacks): Likewise for second
> > 	parameter of "file_change" callback.
> > 	* include/line-map.h (struct line_map): Convert from a struct
> > 	containing a union to a base class.
> > 	(struct line_map_ordinary): Convert to a subclass of line_map.
> > 	(struct line_map_macro): Likewise.
> > 	(linemap_check_ordinary): Strengthen return type from line_map *
> > 	to line_map_ordinary *, and add a const-variant.
> > 	(linemap_check_macro): New pair of functions.
> > 	(ORDINARY_MAP_STARTING_LINE_NUMBER): Strengthen param from
> > 	const line_map * to const line_map_ordinary *, eliminating call
> > 	to linemap_check_ordinary.  Likewise for the non-const variant.
> > 	(ORDINARY_MAP_INCLUDER_FILE_INDEX): Likewise.
> > 	(ORDINARY_MAP_IN_SYSTEM_HEADER_P): Likewise.
> > 	(ORDINARY_MAP_NUMBER_OF_COLUMN_BITS): Likewise.
> > 	(ORDINARY_MAP_FILE_NAME): Likewise.
> > 	(MACRO_MAP_MACRO): Strengthen param from const line_map * to
> > 	const line_map_macro *.  Likewise for the non-const variant.
> > 	(MACRO_MAP_NUM_MACRO_TOKENS): Likewise.
> > 	(MACRO_MAP_LOCATIONS): Likewise.
> > 	(MACRO_MAP_EXPANSION_POINT_LOCATION): Likewise.
> > 	(struct maps_info): Replace with...
> > 	(struct maps_info_ordinary):...this and...
> > 	(struct maps_info_macro): ...this.
> > 	(struct line_maps): Convert fields "info_ordinary" and
> > 	"info_macro" to the above new structs.
> > 	(LINEMAPS_MAP_INFO): Delete both functions.
> > 	(LINEMAPS_MAPS): Likewise.
> > 	(LINEMAPS_ALLOCATED): Rewrite both variants to avoid using
> > 	LINEMAPS_MAP_INFO.
> > 	(LINEMAPS_USED): Likewise.
> > 	(LINEMAPS_CACHE): Likewise.
> > 	(LINEMAPS_MAP_AT): Likewise.
> > 	(LINEMAPS_ORDINARY_MAPS): Strengthen return type from line_map *
> > 	to line_map_ordinary *.
> > 	(LINEMAPS_ORDINARY_MAP_AT): Likewise.
> > 	(LINEMAPS_LAST_ORDINARY_MAP): Likewise.
> > 	(LINEMAPS_LAST_ALLOCATED_ORDINARY_MAP): Likewise.
> > 	(LINEMAPS_MACRO_MAPS): Strengthen return type from line_map * to
> > 	line_map_macro *.
> > 	(LINEMAPS_MACRO_MAP_AT): Likewise.
> > 	(LINEMAPS_LAST_MACRO_MAP): Likewise.
> > 	(LINEMAPS_LAST_ALLOCATED_MACRO_MAP): Likewise.
> > 	(linemap_map_get_macro_name): Strengthen param from
> > 	const line_map * to const line_map_macro *.
> > 	(SOURCE_LINE): Strengthen first param from const line_map * to
> > 	const line_map_ordinary *, removing call to
> > 	linemap_check_ordinary.
> > 	(SOURCE_COLUMN): Likewise.
> > 	(LAST_SOURCE_LINE_LOCATION): Likewise.
> > 	(LAST_SOURCE_LINE): Strengthen first param from const line_map *
> > 	to const line_map_ordinary *.
> > 	(LAST_SOURCE_COLUMN): Likewise.
> > 	(INCLUDED_FROM): Strengthen return type from line_map * to
> > 	line_map_ordinary *., and second param from const line_map *
> > 	to const line_map_ordinary *, removing call to
> > 	linemap_check_ordinary.
> > 	(MAIN_FILE_P): Strengthen param from const line_map * to
> > 	const line_map_ordinary *, removing call to
> > 	linemap_check_ordinary.
> > 	(linemap_position_for_line_and_column): Strengthen param from
> > 	const line_map * to const line_map_ordinary *.
> > 	(LINEMAP_FILE): Strengthen param from const line_map * to
> > 	const line_map_ordinary *, removing call to
> > 	linemap_check_ordinary.
> > 	(LINEMAP_LINE): Likewise.
> > 	(LINEMAP_SYSP): Likewise.
> > 	(linemap_resolve_location): Strengthen final param from
> > 	const line_map ** to const line_map_ordinary **.
> > 	* internal.h (CPP_INCREMENT_LINE): Likewise for local "map".
> > 	(linemap_enter_macro): Strengthen return type from
> > 	const line_map * to const line_map_macro *.
> > 	(linemap_add_macro_token): Likewise for first param.
> > 	* line-map.c (linemap_check_files_exited): Strengthen local "map"
> > 	from const line_map * to const line_map_ordinary *.
> > 	(new_linemap): Introduce local "map_size" and use it when
> > 	calculating how large the buffer should be.  Rewrite based
> > 	on change of info_macro and info_ordinary into distinct types.
> > 	(linemap_add): Strengthen locals "map" and "from" from line_map *
> > 	to line_map_ordinary *.
> > 	(linemap_enter_macro): Strengthen return type from
> > 	const line_map * to const line_map_macro *, and local "map" from
> > 	line_map * to line_map_macro *.
> > 	(linemap_add_macro_token): Strengthen param "map" from
> > 	const line_map * to const line_map_macro *.
> > 	(linemap_line_start): Strengthen local "map" from line_map * to
> > 	line_map_ordinary *.
> > 	(linemap_position_for_column): Likewise.
> > 	(linemap_position_for_line_and_column): Strengthen first param
> > 	from const line_map * to const line_map_ordinary *.
> > 	(linemap_position_for_loc_and_offset): Strengthen local "map" from
> > 	const line_map * to const line_map_ordinary *.
> > 	(linemap_ordinary_map_lookup): Likewise for return type and locals
> > 	"cached" and "result".
> > 	(linemap_macro_map_lookup): Strengthen return type and locals
> > 	"cached" and "result" from const line_map * to
> > 	const line_map_macro *.
> > 	(linemap_macro_map_loc_to_exp_point): Likewise for param "map".
> > 	(linemap_macro_map_loc_to_def_point): Likewise.
> > 	(linemap_macro_map_loc_unwind_toward_spelling): Likewise.
> > 	(linemap_get_expansion_line): Strengthen local "map" from
> > 	const line_map * to const line_map_ordinary *.
> > 	(linemap_get_expansion_filename): Likewise.
> > 	(linemap_map_get_macro_name): Strengthen param from
> > 	const line_map * to const line_map_macro *.
> > 	(linemap_location_in_system_header_p): Add call to
> > 	linemap_check_ordinary in region guarded by
> > 	!linemap_macro_expansion_map_p.  Introduce local "macro_map" via
> > 	linemap_check_macro in other region, using it in place of "map"
> > 	for typesafety.
> > 	(first_map_in_common_1): Add calls to linemap_check_macro.
> > 	(trace_include): Strengthen param "map" from const line_map * to
> > 	const line_map_ordinary *.
> > 	(linemap_macro_loc_to_spelling_point): Strengthen final param from
> > 	const line_map ** to const line_map_ordinary **.  Replace a
> > 	C-style cast with a const_cast, and add calls to
> > 	linemap_check_macro and linemap_check_ordinary.
> > 	(linemap_macro_loc_to_def_point): Likewise.
> > 	(linemap_macro_loc_to_exp_point): Likewise.
> > 	(linemap_resolve_location): Strengthen final param from
> > 	const line_map ** to const line_map_ordinary **.
> > 	(linemap_unwind_toward_expansion): Introduce local "macro_map" via
> > 	a checked cast and use it in place of *map.
> > 	(linemap_unwind_to_first_non_reserved_loc): Strengthen local
> > 	"map1" from const line_map * to const line_map_ordinary *.
> > 	(linemap_expand_location): Introduce local "ord_map" via a checked
> > 	cast and use it in place of map.
> > 	(linemap_dump): Make local "map" const.  Strengthen local
> > 	"includer_map" from line_map * to const line_map_ordinary *.
> > 	Introduce locals "ord_map" and "macro_map" via checked casts and
> > 	use them in place of "map" for typesafety.
> > 	(linemap_dump_location): Strengthen local "map" from
> > 	const line_map * to const line_map_ordinary *.
> > 	(linemap_get_file_highest_location): Update for elimination of
> > 	union.
> > 	(linemap_get_statistics): Strengthen local "cur_map" from
> > 	line_map * to const line_map_macro *.  Update uses of sizeof to
> > 	use the appropriate line_map subclasses.
> > 	* macro.c (_cpp_warn_if_unused_macro): Add call to
> > 	linemap_check_ordinary.
> > 	(builtin_macro): Strengthen local "map" from const line_map * to
> > 	const line_map_macro *.
> > 	(enter_macro_context): Likewise.
> > 	(replace_args): Likewise.
> > 	(tokens_buff_put_token_to): Likewise for param "map".
> > 	(tokens_buff_add_token): Likewise.
> > ---
> 
> 
> > @@ -158,7 +158,7 @@ expand_location_1 (source_location loc,
> >   	     location (toward the expansion point) that is not reserved;
> >   	     that is, the first location that is in real source code.  */
> >   	  loc = linemap_unwind_to_first_non_reserved_loc (line_table,
> > -							  loc, &map);
> > +							  loc, NULL);
> 
> Is that really correct?


I believe so.  The old code had (in gcc/input.c's expand_location_1):

  const struct line_map *map;


  [...snip...]

  if (loc >= RESERVED_LOCATION_COUNT)
    {
      if (!expansion_point_p)
	{
	  [...snip...]
	  loc = linemap_unwind_to_first_non_reserved_loc (line_table,
A>>>							  loc, &map); 
	  lrk = LRK_SPELLING_LOCATION;
	}
      loc = linemap_resolve_location (line_table, loc,
B>>>				      lrk, &map);
C>>>  xloc = linemap_expand_location (line_table, map, loc);
    }

I've highlighted the three lines referring to the local "map".

Consider line A, the call to linemap_unwind_to_first_non_reserved_loc.
This function takes a ptr to a ptr to a map; in this case &map.  It does
not read from this ptr; it can merely write to it in the final line of
one of the exit paths:

  if (map != NULL)
    *map = map0;
  return loc;
}

and it appears that the ptr that's written back can point to  either
kind of map (both ordinary and macro).

So in the old code, line A's the passing in of "&map" lead to map
sometimes being written to.


Consider line B, the call to linemap_resolve_location.

linemap_resolve_location takes a ptr to a ptr to a map.  It can call:
    case LRK_MACRO_EXPANSION_POINT:
      loc = linemap_macro_loc_to_exp_point (set, loc, map);
         (doesn't read *map; for non-NULL map, *always* writes to it,
	   either NULL or an ordinary map)
      break;
    case LRK_SPELLING_LOCATION:
      loc = linemap_macro_loc_to_spelling_point (set, loc, map);
         (as above: doesn't read *map, ; for non-NULL map,
	   *always* writes to it, either NULL or an ordinary map)
      break;
    case LRK_MACRO_DEFINITION_LOCATION:
      loc = linemap_macro_loc_to_def_point (set, loc, map);
         (as above)

Hence linemap_resolve_location shares this behavior of never reading its
*map, it definitely will write back to *map (assuming the map is
non-NULL), and it writes back an ordinary map to *map.

Hence at line B, the old code was always overwriting whatever value (if
any) had been written to map by line A.

Hence I believe it's safe to change line A's &map to simply NULL, and
strengthen the local map from a
  const line_map *
to a
  const line_map_ordinary *.

Assuming I've read all of this code correctly...


>    /* Return the map at a given index.  */
> >   inline line_map *
> >   LINEMAPS_MAP_AT (const line_maps *set, bool map_kind, int index)
> >   {
> > -  return &(LINEMAPS_MAPS (set, map_kind)[index]);
> > +  if (map_kind)
> > +    return &set->info_macro.maps[index];
> > +  else
> > +    return &set->info_ordinary.maps[index];
> >   }
> I see this pattern repeated regularly.  Any thoughts on how painful 
> it'll be to drop the map_kind argument and instead not vary the kind?

I'm not sure I understand your question.

Essentially we have two vecs here, a vec<line_map_ordinary> and a
vec<line_map_macro>, without using vec.h, and the code already assumes
in various places that the maps of one kind are laid out next to each
other in memory, with the nit that the list members have different sizes
(as noted above).

I attempted both
  (a) to turn the maps_info into a template, but ran afoul of gengtype
and it got overcomplicated, and
  (b) to try to move vec.h to libiberty, but it was non-trivial, and I
got nervous about pushback about putting C++ code in libiberty, so it
seemed simpler to manually duplicate the type, the result was much
simpler than either (a) or (b).

Note that LINEMAPS_MAPS_AT already took a map_kind argument; before
these changes, it looked like this:
#define LINEMAPS_MAP_AT(SET, MAP_KIND, INDEX)	\
  (&((LINEMAPS_MAPS (SET, MAP_KIND))[(INDEX)]))

and if you manually expand all those macros, it's basically doing the
same thing, with that inline function replacing:

#define LINEMAPS_MAPS(SET, MAP_KIND) \
  (LINEMAPS_MAP_INFO (SET, MAP_KIND))->maps

#define LINEMAPS_MAP_INFO(SET, MACRO_MAP_P)				\
  ((MACRO_MAP_P)							\
   ? &((SET)->info_macro)						\
   : &((SET)->info_ordinary))

etc (all of these nested macros were one area where I found the existing
code hard to follow).

> I just spot checked all the mechanical changes.  Looks reasonable me to me.
> 
> Just need to sort out if the hunk with the call to l_u_t_f_n_r_l is 
> correct and get the pre-reqs installed and I think this is good to go.
> 
> 
> Jeff

Re: [PATCH 4/4] Replace line_map union with C++ class hierarchy

[Jeff Law]

On 05/04/2015 03:27 PM, David Malcolm wrote:
>>> @@ -158,7 +158,7 @@ expand_location_1 (source_location loc,
>>>    	     location (toward the expansion point) that is not reserved;
>>>    	     that is, the first location that is in real source code.  */
>>>    	  loc = linemap_unwind_to_first_non_reserved_loc (line_table,
>>> -							  loc, &map);
>>> +							  loc, NULL);
>>
>> Is that really correct?
>
>
> I believe so.  The old code had (in gcc/input.c's expand_location_1):
[ ... ]
THanks for walking through it.  Mostly it just stuck out as "odd".

Given that we're depending on "not read" behaviour and latter calls 
writing into that object again (thus making the original object in 
memory "dead"), it'd be good to try and clean this up a little.

You could argue that passing in NULL like you did *is* actually that 
cleanup and I'd probably agree with the hesitation that for it to be a 
valid cleanup, linemap_resolve_location must never read what's in that 
location.

linemap_resolve_location's function header indicates it's an output, so 
I think we're OK.

So, concern eliminated :-)


>
>>     /* Return the map at a given index.  */
>>>    inline line_map *
>>>    LINEMAPS_MAP_AT (const line_maps *set, bool map_kind, int index)
>>>    {
>>> -  return &(LINEMAPS_MAPS (set, map_kind)[index]);
>>> +  if (map_kind)
>>> +    return &set->info_macro.maps[index];
>>> +  else
>>> +    return &set->info_ordinary.maps[index];
>>>    }
>> I see this pattern repeated regularly.  Any thoughts on how painful
>> it'll be to drop the map_kind argument and instead not vary the kind?
>
> I'm not sure I understand your question.
Given that kind of code, I'd be looking at the callers to determine if 
there's a reasonable way to bubble up the if (map_kind) test.

This is particularly good if bubbling it up ultimately results in 
somewhere in the call chain a statically determined map_kind.

I don't consider resolving this a requirement to go forward and it looks 
like you tried implementing this stuff with templates and ran afoul of 
gengtype.  Fair enough :-0

So I think at this point the patch is good to go.

Jeff