[PATCH] libcpp: Replace some macro usage with C++ constructs

[PATCH] libcpp: Replace some 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 hoped it would be.

In an attempt at making the code easier to follow, the following patch
converts some 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)
  $4 = 0x13b8b37 "<command-line>"

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

Caveats:
  * I didn't mark the inline functions as "static"; should they be?

  * I didn't run performance testing on the parser; is there
    a recommended way to do this for this kind of change?

OK for trunk?

libcpp/
	* include/line-map.h (MAX_SOURCE_LOCATION): Convert from a macro
	to a const source_location.
	(RESERVED_LOCATION_COUNT): Convert from a macro to a const int.

	(LINEMAPS_MAP_INFO): Convert from a macro to a pair of inline
	functions, for const/non-const arguments.
	(linemap_check_ordinary): Likewise.

	(IS_ADHOC_LOC): Convert from a macro to an inline function.
	(COMBINE_LOCATION_DATA): Likewise.
	(SOURCE_LINE): Likewise.
	(SOURCE_COLUMN): Likewise.
	(LAST_SOURCE_LINE_LOCATION): Likewise.
	(LAST_SOURCE_LINE): Likewise.
	(LAST_SOURCE_COLUMN): Likewise.
	(INCLUDED_FROM): Likewise.
	(MAIN_FILE_P): Likewise.
	(source_location): 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.
---
 libcpp/include/line-map.h | 177 ++++++++++++++++++++++++++++++----------------
 libcpp/line-map.c         |   2 +-
 2 files changed, 118 insertions(+), 61 deletions(-)

diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 9886314..4dfb0ae 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -88,7 +88,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;
 
@@ -320,10 +320,21 @@ 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
@@ -441,9 +452,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 *);
 
@@ -521,66 +541,93 @@ bool linemap_location_from_macro_expansion_p (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;
+
+#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)
+#else
+#define linemap_assert(EXPR)
+#endif
+
+/* 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;
+}
 
 /* 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)
+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
-#define linemap_assert(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
@@ -595,19 +642,25 @@ source_location linemap_position_for_line_and_column (struct line_map *,
 						      linenum_type,
 						      unsigned int);
 /* 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
@@ -620,8 +673,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 f9a7658..4902fb1 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -189,7 +189,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);
-- 
1.8.5.3