public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 2/8] Rename symbol_substitution_name
  2018-05-03 18:42 [PATCH 0/8] Convert C compile to C++ Keith Seitz
@ 2018-05-03 18:41 ` Keith Seitz
  2018-05-03 18:42 ` [PATCH 1/8] Return unique_xmalloc_ptr for generate_c_for_variable_locations Keith Seitz
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Keith Seitz @ 2018-05-03 18:41 UTC (permalink / raw)
  To: gdb-patches

This patch simply adds a "c_" prefix to symbol_substitution_name to clarify
that this is a C language-related function.

gdb/ChangeLog:

	* compile/compile-c-symbols.c (symbol_substitution_name): Rename to ...
	(c_symbol_substitution_name): ... this.
	Update all callers.
---
 gdb/compile/compile-c-symbols.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c
index 6987fa375e..9364fc27ac 100644
--- a/gdb/compile/compile-c-symbols.c
+++ b/gdb/compile/compile-c-symbols.c
@@ -127,7 +127,7 @@ error_symbol_once (struct compile_c_instance *context,
    address.  */
 
 static gdb::unique_xmalloc_ptr<char>
-symbol_substitution_name (struct symbol *sym)
+c_symbol_substitution_name (struct symbol *sym)
 {
   return gdb::unique_xmalloc_ptr<char>
     (concat ("__", SYMBOL_NATURAL_NAME (sym), "_ptr", (char *) NULL));
@@ -266,7 +266,7 @@ convert_one_symbol (struct compile_c_instance *context,
 	case LOC_LOCAL:
 	substitution:
 	  kind = GCC_C_SYMBOL_VARIABLE;
-	  symbol_name = symbol_substitution_name (sym.symbol);
+	  symbol_name = c_symbol_substitution_name (sym.symbol);
 	  break;
 
 	case LOC_STATIC:
@@ -657,7 +657,7 @@ generate_c_for_for_one_variable (struct compile_c_instance *compiler,
       if (SYMBOL_COMPUTED_OPS (sym) != NULL)
 	{
 	  gdb::unique_xmalloc_ptr<char> generated_name
-	    = symbol_substitution_name (sym);
+	    = c_symbol_substitution_name (sym);
 	  /* We need to emit to a temporary buffer in case an error
 	     occurs in the middle.  */
 	  string_file local_file;
-- 
2.13.6

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 0/8] Convert C compile to C++
@ 2018-05-03 18:42 Keith Seitz
  2018-05-03 18:41 ` [PATCH 2/8] Rename symbol_substitution_name Keith Seitz
                   ` (8 more replies)
  0 siblings, 9 replies; 21+ messages in thread
From: Keith Seitz @ 2018-05-03 18:42 UTC (permalink / raw)
  To: gdb-patches

This patch series begins the move to C++ for the compile feature,
specifically targeting C type conversion.  This work is the basis for ongoing
C++ compile work, which I hope to submit in the not-too-distant future.

The main drive of the patches is to convert compile_instance and
compile_c_instance into classes, but there are several pre-cursor patches
which attempt to reorganize the code a little bit.  Generic compile
functionality is put into compile-internal.h/compile.c, and all other
C-specific code is moved into compile-c-*.c/h files.

Keith Seitz (8):
  Return unique_xmalloc_ptr for generate_c_for_variable_locations
  Rename symbol_substitution_name
  Move C-related declarations to compile-c.h
  Add a C++ wrapper for GCC C plug-in
  Change compile_instance/compile_c_instance into classes
  Use std::unordered_map instead of htab_t.
  Move compile_instance to compile.c
  Use policies for code generation

 gdb/Makefile.in                 |   9 +-
 gdb/c-lang.h                    |   4 +-
 gdb/compile/compile-c-support.c | 419 +++++++++++++++++++++++++---------------
 gdb/compile/compile-c-symbols.c | 180 ++++-------------
 gdb/compile/compile-c-types.c   | 317 +++++++++++-------------------
 gdb/compile/compile-c.h         |  95 +++++++++
 gdb/compile/compile-internal.h  | 177 +++++++++--------
 gdb/compile/compile-loc2c.c     |   1 +
 gdb/compile/compile.c           | 198 ++++++++++++++-----
 gdb/compile/gcc-c-plugin.h      |  64 ++++++
 gdb/language.h                  |   6 +-
 11 files changed, 828 insertions(+), 642 deletions(-)
 create mode 100644 gdb/compile/compile-c.h
 create mode 100644 gdb/compile/gcc-c-plugin.h

-- 
2.13.6

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/8] Return unique_xmalloc_ptr for generate_c_for_variable_locations
  2018-05-03 18:42 [PATCH 0/8] Convert C compile to C++ Keith Seitz
  2018-05-03 18:41 ` [PATCH 2/8] Rename symbol_substitution_name Keith Seitz
@ 2018-05-03 18:42 ` Keith Seitz
  2018-05-03 18:42 ` [PATCH 3/8] Move C-related declarations to compile-c.h Keith Seitz
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Keith Seitz @ 2018-05-03 18:42 UTC (permalink / raw)
  To: gdb-patches

This patch eliminates two cleanups in compile/ by changing
generate_c_for_variable_locations so that it returns a unique_ptr.

gdb/ChangeLog:

	* compile/compile-c-support.c (c_compute_program): Use
	unique_xmalloc_ptr to eliminate cleanup.
	* compile/compile-c-symbols.c (generate_c_for_variable_locations):
	Return a unique_xmalloc_ptr and eliminate cleanup.
	* compile/compile-internal.h (generate_c_for_variable_locations):
	Return unique_xmalloc_ptr and update description.
---
 gdb/compile/compile-c-support.c | 10 ++++------
 gdb/compile/compile-c-symbols.c | 11 ++++-------
 gdb/compile/compile-internal.h  |  9 +++++----
 3 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index e694648288..696bb9fced 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -351,17 +351,15 @@ c_compute_program (struct compile_instance *inst,
      and the user's code may only refer to globals.  */
   if (inst->scope != COMPILE_I_RAW_SCOPE)
     {
-      unsigned char *registers_used;
       int i;
 
       /* Generate the code to compute variable locations, but do it
 	 before generating the function header, so we can define the
 	 register struct before the function body.  This requires a
 	 temporary stream.  */
-      registers_used = generate_c_for_variable_locations (context,
-							  var_stream, gdbarch,
-							  expr_block, expr_pc);
-      make_cleanup (xfree, registers_used);
+      gdb::unique_xmalloc_ptr<unsigned char> registers_used
+	= generate_c_for_variable_locations (context, var_stream, gdbarch,
+					     expr_block, expr_pc);
 
       buf.puts ("typedef unsigned int"
 		" __attribute__ ((__mode__(__pointer__)))"
@@ -382,7 +380,7 @@ c_compute_program (struct compile_instance *inst,
 		      mode, mode);
 	}
 
-      generate_register_struct (&buf, gdbarch, registers_used);
+      generate_register_struct (&buf, gdbarch, registers_used.get ());
     }
 
   add_code_header (inst->scope, &buf);
diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c
index 43de7df024..6987fa375e 100644
--- a/gdb/compile/compile-c-symbols.c
+++ b/gdb/compile/compile-c-symbols.c
@@ -708,24 +708,22 @@ generate_c_for_for_one_variable (struct compile_c_instance *compiler,
 
 /* See compile-internal.h.  */
 
-unsigned char *
+gdb::unique_xmalloc_ptr<unsigned char>
 generate_c_for_variable_locations (struct compile_c_instance *compiler,
 				   string_file &stream,
 				   struct gdbarch *gdbarch,
 				   const struct block *block,
 				   CORE_ADDR pc)
 {
-  struct cleanup *outer;
   const struct block *static_block = block_static_block (block);
-  unsigned char *registers_used;
 
   /* If we're already in the static or global block, there is nothing
      to write.  */
   if (static_block == NULL || block == static_block)
     return NULL;
 
-  registers_used = XCNEWVEC (unsigned char, gdbarch_num_regs (gdbarch));
-  outer = make_cleanup (xfree, registers_used);
+  gdb::unique_xmalloc_ptr<unsigned char> registers_used
+    (XCNEWVEC (unsigned char, gdbarch_num_regs (gdbarch)));
 
   /* Ensure that a given name is only entered once.  This reflects the
      reality of shadowing.  */
@@ -745,7 +743,7 @@ generate_c_for_variable_locations (struct compile_c_instance *compiler,
 	{
 	  if (!symbol_seen (symhash.get (), sym))
 	    generate_c_for_for_one_variable (compiler, stream, gdbarch,
-					     registers_used, pc, sym);
+					     registers_used.get (), pc, sym);
 	}
 
       /* If we just finished the outermost block of a function, we're
@@ -755,6 +753,5 @@ generate_c_for_variable_locations (struct compile_c_instance *compiler,
       block = BLOCK_SUPERBLOCK (block);
     }
 
-  discard_cleanups (outer);
   return registers_used;
 }
diff --git a/gdb/compile/compile-internal.h b/gdb/compile/compile-internal.h
index c92cb645f6..01beb1dddd 100644
--- a/gdb/compile/compile-internal.h
+++ b/gdb/compile/compile-internal.h
@@ -128,11 +128,12 @@ extern gcc_c_symbol_address_function gcc_symbol_address;
 extern struct compile_instance *new_compile_instance (struct gcc_c_context *fe);
 
 /* Emit code to compute the address for all the local variables in
-   scope at PC in BLOCK.  Returns a malloc'd vector, indexed by gdb
-   register number, where each element indicates if the corresponding
-   register is needed to compute a local variable.  */
+   scope at PC in BLOCK.  Returns a vector, indexed by gdb register
+   number, where each element indicates if the corresponding register
+   is needed to compute a local variable.  */
 
-extern unsigned char *generate_c_for_variable_locations
+extern gdb::unique_xmalloc_ptr<unsigned char>
+  generate_c_for_variable_locations
      (struct compile_c_instance *compiler,
       string_file &stream,
       struct gdbarch *gdbarch,
-- 
2.13.6

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 3/8] Move C-related declarations to compile-c.h
  2018-05-03 18:42 [PATCH 0/8] Convert C compile to C++ Keith Seitz
  2018-05-03 18:41 ` [PATCH 2/8] Rename symbol_substitution_name Keith Seitz
  2018-05-03 18:42 ` [PATCH 1/8] Return unique_xmalloc_ptr for generate_c_for_variable_locations Keith Seitz
@ 2018-05-03 18:42 ` Keith Seitz
  2018-06-06 14:34   ` Pedro Alves
  2018-05-03 18:49 ` [PATCH 4/8] Add a C++ wrapper for GCC C plug-in Keith Seitz
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Keith Seitz @ 2018-05-03 18:42 UTC (permalink / raw)
  To: gdb-patches

This patch simply moves a bunch of C language-related declarations from
the various compile header files into a new C-specific header, compile-c.h.

gdb/ChangeLog:

	* Makefile.in (SUBDIR_GCC_COMPILE_SRCS): Move header files ...
	(HFILES_NO_SRCDIR): ... to here.
	Add compile-internal.h and compile-c.h.
	* compile/compile-c-support.c: Include compile-c.h.
	* compile/compile-c-symbols.c: Include compile-c.h.
	(generate_c_for_variable_locations): Update comment.
	* compile/compile-c-types.c: Include compile-c.h.
	* compile/compile-c.h: New file -- moved C language declarations
	from other files here.
	* compile/compile-internal.h: Do not include hashtab.h or
	common/enum-flags.h.
	(gcc_qualifiers_flags, struct compile_c_instance, C_CTX)
	(gcc_convert_symbol, gcc_symbol_address)
	(generate_c_for_variable_locations, c_get_mode_for_size)
	(c_get_range_decl_name): Definitions moved to compile-c.h.
	* compile/compile-loc2c.c: Include compile-c.h.
---
 gdb/Makefile.in                 |  8 ++--
 gdb/compile/compile-c-support.c |  1 +
 gdb/compile/compile-c-symbols.c |  3 +-
 gdb/compile/compile-c-types.c   |  1 +
 gdb/compile/compile-c.h         | 81 +++++++++++++++++++++++++++++++++++++++++
 gdb/compile/compile-internal.h  | 60 ------------------------------
 gdb/compile/compile-loc2c.c     |  1 +
 7 files changed, 91 insertions(+), 64 deletions(-)
 create mode 100644 gdb/compile/compile-c.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index be769280a9..67fff669ba 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -319,9 +319,7 @@ SUBDIR_GCC_COMPILE_SRCS = \
 	compile/compile-c-types.c \
 	compile/compile-loc2c.c \
 	compile/compile-object-load.c \
-	compile/compile-object-load.h \
-	compile/compile-object-run.c \
-	compile/compile-object-run.h
+	compile/compile-object-run.c
 
 SUBDIR_GCC_COMPILE_OBS = $(patsubst %.c,%.o,$(filter %.c,$(SUBDIR_GCC_COMPILE_SRCS)))
 
@@ -1453,7 +1451,11 @@ HFILES_NO_SRCDIR = \
 	common/version.h \
 	common/x86-xstate.h \
 	common/xml-utils.h \
+	compile/compile-c.h \
 	compile/compile.h \
+	compile/compile-internal.h \
+	compile/compile-object-load.h \
+	compile/compile-object-run.h \
 	config/nm-linux.h \
 	config/nm-nto.h \
 	config/djgpp/langinfo.h \
diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index 696bb9fced..41fead9ad1 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -19,6 +19,7 @@
 
 #include "defs.h"
 #include "compile-internal.h"
+#include "compile-c.h"
 #include "compile.h"
 #include "gdb-dlfcn.h"
 #include "c-lang.h"
diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c
index 9364fc27ac..c82e008659 100644
--- a/gdb/compile/compile-c-symbols.c
+++ b/gdb/compile/compile-c-symbols.c
@@ -20,6 +20,7 @@
 
 #include "defs.h"
 #include "compile-internal.h"
+#include "compile-c.h"
 #include "symtab.h"
 #include "parser-defs.h"
 #include "block.h"
@@ -706,7 +707,7 @@ generate_c_for_for_one_variable (struct compile_c_instance *compiler,
   END_CATCH
 }
 
-/* See compile-internal.h.  */
+/* See compile-c.h.  */
 
 gdb::unique_xmalloc_ptr<unsigned char>
 generate_c_for_variable_locations (struct compile_c_instance *compiler,
diff --git a/gdb/compile/compile-c-types.c b/gdb/compile/compile-c-types.c
index 212cfe66be..46b9f2b3b4 100644
--- a/gdb/compile/compile-c-types.c
+++ b/gdb/compile/compile-c-types.c
@@ -21,6 +21,7 @@
 #include "defs.h"
 #include "gdbtypes.h"
 #include "compile-internal.h"
+#include "compile-c.h"
 #include "objfiles.h"
 
 /* An object that maps a gdb type to a gcc type.  */
diff --git a/gdb/compile/compile-c.h b/gdb/compile/compile-c.h
new file mode 100644
index 0000000000..cff2aef906
--- /dev/null
+++ b/gdb/compile/compile-c.h
@@ -0,0 +1,81 @@
+/* Header file for GDB compile C-language support.
+   Copyright (C) 2014-2018 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDB_COMPILE_C_H
+#define GDB_COMPILE_C_H
+
+#include "common/enum-flags.h"
+#include "hashtab.h"
+
+/* enum-flags wrapper.  */
+
+DEF_ENUM_FLAGS_TYPE (enum gcc_qualifiers, gcc_qualifiers_flags);
+
+/* A callback suitable for use as the GCC C symbol oracle.  */
+
+extern gcc_c_oracle_function gcc_convert_symbol;
+
+/* A callback suitable for use as the GCC C address oracle.  */
+
+extern gcc_c_symbol_address_function gcc_symbol_address;
+
+/* A subclass of compile_instance that is specific to the C front
+   end.  */
+
+struct compile_c_instance
+{
+  /* Base class.  Note that the base class vtable actually points to a
+     gcc_c_fe_vtable.  */
+  struct compile_instance base;
+
+  /* Map from gdb types to gcc types.  */
+  htab_t type_map;
+
+  /* Map from gdb symbols to gcc error messages to emit.  */
+  htab_t symbol_err_map;
+};
+
+/* A helper macro that takes a compile_c_instance and returns its
+   corresponding gcc_c_context.  */
+
+#define C_CTX(I) ((struct gcc_c_context *) ((I)->base.fe))
+
+/* Emit code to compute the address for all the local variables in
+   scope at PC in BLOCK.  Returns a malloc'd vector, indexed by gdb
+   register number, where each element indicates if the corresponding
+   register is needed to compute a local variable.  */
+
+extern gdb::unique_xmalloc_ptr<unsigned char>
+  generate_c_for_variable_locations
+     (struct compile_c_instance *compiler,
+      string_file &stream,
+      struct gdbarch *gdbarch,
+      const struct block *block,
+      CORE_ADDR pc);
+
+/* Get the GCC mode attribute value for a given type size.  */
+
+extern const char *c_get_mode_for_size (int size);
+
+/* Given a dynamic property, return an xmallocd name that is used to
+   represent its size.  The result must be freed by the caller.  The
+   contents of the resulting string will be the same each time for
+   each call with the same argument.  */
+
+struct dynamic_prop;
+extern std::string c_get_range_decl_name (const struct dynamic_prop *prop);
+
+#endif /* GDB_COMPILE_C_H  */
diff --git a/gdb/compile/compile-internal.h b/gdb/compile/compile-internal.h
index 01beb1dddd..afe20e5141 100644
--- a/gdb/compile/compile-internal.h
+++ b/gdb/compile/compile-internal.h
@@ -17,12 +17,7 @@
 #ifndef GDB_COMPILE_INTERNAL_H
 #define GDB_COMPILE_INTERNAL_H
 
-#include "hashtab.h"
 #include "gcc-c-interface.h"
-#include "common/enum-flags.h"
-
-/* enum-flags wrapper.  */
-DEF_ENUM_FLAGS_TYPE (enum gcc_qualifiers, gcc_qualifiers_flags);
 
 /* Debugging flag for the "compile" family of commands.  */
 
@@ -57,29 +52,6 @@ struct compile_instance
   void (*destroy) (struct compile_instance *);
 };
 
-/* A subclass of compile_instance that is specific to the C front
-   end.  */
-struct compile_c_instance
-{
-  /* Base class.  Note that the base class vtable actually points to a
-     gcc_c_fe_vtable.  */
-
-  struct compile_instance base;
-
-  /* Map from gdb types to gcc types.  */
-
-  htab_t type_map;
-
-  /* Map from gdb symbols to gcc error messages to emit.  */
-
-  htab_t symbol_err_map;
-};
-
-/* A helper macro that takes a compile_c_instance and returns its
-   corresponding gcc_c_context.  */
-
-#define C_CTX(I) ((struct gcc_c_context *) ((I)->base.fe))
-
 /* Define header and footers for different scopes.  */
 
 /* A simple scope just declares a function named "_gdb_expr", takes no
@@ -114,43 +86,11 @@ struct type;
 extern gcc_type convert_type (struct compile_c_instance *context,
 			      struct type *type);
 
-/* A callback suitable for use as the GCC C symbol oracle.  */
-
-extern gcc_c_oracle_function gcc_convert_symbol;
-
-/* A callback suitable for use as the GCC C address oracle.  */
-
-extern gcc_c_symbol_address_function gcc_symbol_address;
-
 /* Instantiate a GDB object holding state for the GCC context FE.  The
    new object is returned.  */
 
 extern struct compile_instance *new_compile_instance (struct gcc_c_context *fe);
 
-/* Emit code to compute the address for all the local variables in
-   scope at PC in BLOCK.  Returns a vector, indexed by gdb register
-   number, where each element indicates if the corresponding register
-   is needed to compute a local variable.  */
-
-extern gdb::unique_xmalloc_ptr<unsigned char>
-  generate_c_for_variable_locations
-     (struct compile_c_instance *compiler,
-      string_file &stream,
-      struct gdbarch *gdbarch,
-      const struct block *block,
-      CORE_ADDR pc);
-
-/* Get the GCC mode attribute value for a given type size.  */
-
-extern const char *c_get_mode_for_size (int size);
-
-/* Given a dynamic property, return a name that is used to represent
-   its size.  The contents of the resulting string will be the same
-   each time for each call with the same argument.  */
-
-struct dynamic_prop;
-extern std::string c_get_range_decl_name (const struct dynamic_prop *prop);
-
 /* Type used to hold and pass around the source and object file names
    to use for compilation.  */
 class compile_file_names
diff --git a/gdb/compile/compile-loc2c.c b/gdb/compile/compile-loc2c.c
index aba791a20f..bd080f85fb 100644
--- a/gdb/compile/compile-loc2c.c
+++ b/gdb/compile/compile-loc2c.c
@@ -24,6 +24,7 @@
 #include "ui-file.h"
 #include "utils.h"
 #include "compile-internal.h"
+#include "compile-c.h"
 #include "compile.h"
 #include "block.h"
 #include "dwarf2-frame.h"
-- 
2.13.6

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 6/8] Use std::unordered_map instead of htab_t.
  2018-05-03 18:42 [PATCH 0/8] Convert C compile to C++ Keith Seitz
                   ` (3 preceding siblings ...)
  2018-05-03 18:49 ` [PATCH 4/8] Add a C++ wrapper for GCC C plug-in Keith Seitz
@ 2018-05-03 18:49 ` Keith Seitz
  2018-06-06 14:35   ` Pedro Alves
  2018-05-03 18:50 ` [PATCH 8/8] Use policies for code generation Keith Seitz
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Keith Seitz @ 2018-05-03 18:49 UTC (permalink / raw)
  To: gdb-patches

This patch simply removes the use of libiberty's hashtab facility in favor
of C++'s unordered_map.

gdb/ChangeLog:

	* compile/compile-c-symbols.c (symbol_error, hash_symbol_error)
	(eq_symbol_error, del_symbol_error): Remove.
	(compile_instance::insert_symbol_error)
	(compile_instance::error_symbol_once): Update to use
	std::unordered_map instead of htab_t.
	* compile/compile-c-types.c (type_map_instance, hash_type_map_instance)
	(eq_type_map_instance, compile_instance::compile_instance): Remove.
	(compile_instance::insert_type, compile_instance::convert_type):
	Update to use std::unordered_map.
	* compile/compile-internal.h: Include string and unordered_map.
	(compile_instance::compile_instance): Don't call htab_delete.
	<type_map_t, symbol_err_map_t>: Make std::unordered_map.
---
 gdb/compile/compile-c-symbols.c | 90 +++++------------------------------------
 gdb/compile/compile-c-types.c   | 86 +++++++--------------------------------
 gdb/compile/compile-internal.h  | 20 ++++-----
 3 files changed, 34 insertions(+), 162 deletions(-)

diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c
index 7bc30a5b3d..0b74e4b4cd 100644
--- a/gdb/compile/compile-c-symbols.c
+++ b/gdb/compile/compile-c-symbols.c
@@ -33,81 +33,16 @@
 
 \f
 
-/* Object of this type are stored in the compiler's symbol_err_map.  */
-
-struct symbol_error
-{
-  /* The symbol.  */
-
-  const struct symbol *sym;
-
-  /* The error message to emit.  This is malloc'd and owned by the
-     hash table.  */
-
-  char *message;
-};
-
-/* Hash function for struct symbol_error.  */
-
-static hashval_t
-hash_symbol_error (const void *a)
-{
-  const struct symbol_error *se = (const struct symbol_error *) a;
-
-  return htab_hash_pointer (se->sym);
-}
-
-/* Equality function for struct symbol_error.  */
-
-static int
-eq_symbol_error (const void *a, const void *b)
-{
-  const struct symbol_error *sea = (const struct symbol_error *) a;
-  const struct symbol_error *seb = (const struct symbol_error *) b;
-
-  return sea->sym == seb->sym;
-}
-
-/* Deletion function for struct symbol_error.  */
-
-static void
-del_symbol_error (void *a)
-{
-  struct symbol_error *se = (struct symbol_error *) a;
-
-  xfree (se->message);
-  xfree (se);
-}
-
 /* See compile-internal.h.  */
 
 void
 compile_instance::insert_symbol_error (const struct symbol *sym,
-				       const char *text)
+				       std::string text)
 {
-  struct symbol_error e;
-  void **slot;
+  symbol_err_map_t::iterator pos = m_symbol_err_map.find (sym);
 
-  if (m_symbol_err_map == NULL)
-    {
-      m_symbol_err_map = htab_create_alloc (10,
-					    hash_symbol_error,
-					    eq_symbol_error,
-					    del_symbol_error,
-					    xcalloc,
-					    xfree);
-    }
-
-  e.sym = sym;
-  slot = htab_find_slot (m_symbol_err_map, &e, INSERT);
-  if (*slot == NULL)
-    {
-      struct symbol_error *e = XNEW (struct symbol_error);
-
-      e->sym = sym;
-      e->message = xstrdup (text);
-      *slot = e;
-    }
+  if (pos == m_symbol_err_map.end ())
+    m_symbol_err_map.insert (std::make_pair (sym, text));
 }
 
 /* See compile-internal.h.  */
@@ -115,20 +50,13 @@ compile_instance::insert_symbol_error (const struct symbol *sym,
 void
 compile_instance::error_symbol_once (const struct symbol *sym)
 {
-  struct symbol_error search;
-  struct symbol_error *err;
-
-  if (m_symbol_err_map == NULL)
-    return;
-
-  search.sym = sym;
-  err = (struct symbol_error *) htab_find (m_symbol_err_map, &search);
-  if (err == NULL || err->message == NULL)
+  symbol_err_map_t::iterator pos = m_symbol_err_map.find (sym);
+  if (pos == m_symbol_err_map.end () || pos->second.length () == 0)
     return;
 
-  gdb::unique_xmalloc_ptr<char> message (err->message);
-  err->message = NULL;
-  error (_("%s"), message.get ());
+  std::string message (pos->second);
+  pos->second.clear ();
+  ::error (_("%s"), message.c_str ());
 }
 
 \f
diff --git a/gdb/compile/compile-c-types.c b/gdb/compile/compile-c-types.c
index 44b8317739..1ad6c2a4da 100644
--- a/gdb/compile/compile-c-types.c
+++ b/gdb/compile/compile-c-types.c
@@ -24,78 +24,24 @@
 #include "compile-c.h"
 #include "objfiles.h"
 
-/* An object that maps a gdb type to a gcc type.  */
-
-struct type_map_instance
-{
-  /* The gdb type.  */
-
-  struct type *type;
-
-  /* The corresponding gcc type handle.  */
-
-  gcc_type gcc_type_handle;
-};
-
-/* Hash a type_map_instance.  */
-
-static hashval_t
-hash_type_map_instance (const void *p)
-{
-  const struct type_map_instance *inst = (const struct type_map_instance *) p;
-
-  return htab_hash_pointer (inst->type);
-}
-
-/* Check two type_map_instance objects for equality.  */
-
-static int
-eq_type_map_instance (const void *a, const void *b)
-{
-  const struct type_map_instance *insta = (const struct type_map_instance *) a;
-  const struct type_map_instance *instb = (const struct type_map_instance *) b;
-
-  return insta->type == instb->type;
-}
-
-/* Constructor for compile_instance.  */
-
-compile_instance::compile_instance (struct gcc_base_context *gcc_fe,
-				    const char *options)
-  : m_gcc_fe (gcc_fe), m_gcc_target_options (options),
-    m_symbol_err_map (NULL)
-{
-  m_type_map = htab_create_alloc (10, hash_type_map_instance,
-				  eq_type_map_instance,
-				  xfree, xcalloc, xfree);
-}
-
-\f
-
 /* See compile-internal.h.  */
 
 void
 compile_instance::insert_type (struct type *type, gcc_type gcc_type)
 {
-  struct type_map_instance inst, *add;
-  void **slot;
-
-  inst.type = type;
-  inst.gcc_type_handle = gcc_type;
-  slot = htab_find_slot (m_type_map, &inst, INSERT);
+  type_map_t::iterator pos = m_type_map.find (type);
 
-  add = (struct type_map_instance *) *slot;
-  /* The type might have already been inserted in order to handle
-     recursive types.  */
-  if (add != NULL && add->gcc_type_handle != gcc_type)
-    error (_("Unexpected type id from GCC, check you use recent enough GCC."));
-
-  if (add == NULL)
+  if (pos != m_type_map.end ())
     {
-      add = XNEW (struct type_map_instance);
-      *add = inst;
-      *slot = add;
+      /* The type might have already been inserted in order to handle
+         recursive types.  */
+      if (pos->second != gcc_type)
+        error (_("Unexpected type id from GCC, check for recent "
+                 "enough GCC."));
     }
+  else
+    m_type_map.insert (std::make_pair (type, gcc_type));
+
 }
 
 /* Convert a pointer type to its gcc representation.  */
@@ -422,19 +368,15 @@ const char *compile_c_instance::m_default_cflags = "-std=gnu11"
 gcc_type
 compile_c_instance::convert_type (struct type *type)
 {
-  struct type_map_instance inst, *found;
-  gcc_type result;
-
   /* We don't ever have to deal with typedefs in this code, because
      those are only needed as symbols by the C compiler.  */
   type = check_typedef (type);
 
-  inst.type = type;
-  found = (struct type_map_instance *) htab_find (m_type_map, &inst);
-  if (found != NULL)
-    return found->gcc_type_handle;
+  type_map_t::iterator pos = m_type_map.find (type);
+  if (pos != m_type_map.end ())
+    return pos->second;
 
-  result = convert_type_basic (this, type);
+  gcc_type result = convert_type_basic (this, type);
   insert_type (type, result);
   return result;
 }
diff --git a/gdb/compile/compile-internal.h b/gdb/compile/compile-internal.h
index 8cfb214c02..e7b428da3f 100644
--- a/gdb/compile/compile-internal.h
+++ b/gdb/compile/compile-internal.h
@@ -19,6 +19,9 @@
 
 #include "gcc-c-interface.h"
 
+#include <string>
+#include <unordered_map>
+
 /* Debugging flag for the "compile" family of commands.  */
 
 extern int compile_debug;
@@ -31,15 +34,13 @@ struct block;
 class compile_instance
 {
 public:
-  compile_instance (struct gcc_base_context *gcc_fe, const char *options);
-
-  virtual ~compile_instance ()
+  compile_instance (struct gcc_base_context *gcc_fe, const char *options)
+      : m_gcc_fe (gcc_fe), m_gcc_target_options (options)
   {
-    htab_delete (m_type_map);
-    if (m_symbol_err_map != NULL)
-      htab_delete (m_symbol_err_map);
   }
 
+  virtual ~compile_instance () = default;
+
   /* Returns the GCC options to be passed during compilation.  */
   const std::string &gcc_target_options () const
   {
@@ -53,7 +54,7 @@ public:
   void insert_type (struct type *type, gcc_type gcc_type);
 
   /* Associate SYMBOL with some error text.  */
-  void insert_symbol_error (const struct symbol *sym, const char *text);
+  void insert_symbol_error (const struct symbol *sym, std::string text);
 
   /* Emit the error message corresponding to SYM, if one exists, and
      arrange for it not to be emitted again.  */
@@ -117,8 +118,9 @@ public:
 protected:
   /* Map types used by the compile instance for caching type conversions.
      and error tracking.  */
-  typedef htab_t type_map_t;
-  typedef htab_t symbol_err_map_t;
+  typedef std::unordered_map<const struct type *, gcc_type> type_map_t;
+  typedef std::unordered_map<const struct symbol *, std::string>
+    symbol_err_map_t;
 
   /* The GCC front end.  */
   struct gcc_base_context *m_gcc_fe;
-- 
2.13.6

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 4/8] Add a C++ wrapper for GCC C plug-in
  2018-05-03 18:42 [PATCH 0/8] Convert C compile to C++ Keith Seitz
                   ` (2 preceding siblings ...)
  2018-05-03 18:42 ` [PATCH 3/8] Move C-related declarations to compile-c.h Keith Seitz
@ 2018-05-03 18:49 ` Keith Seitz
  2018-06-06 14:34   ` Pedro Alves
  2018-05-03 18:49 ` [PATCH 6/8] Use std::unordered_map instead of htab_t Keith Seitz
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Keith Seitz @ 2018-05-03 18:49 UTC (permalink / raw)
  To: gdb-patches

This patch introduces a new class which wraps the GCC C compile plug-in.
It is a little unfortunate that this all happened in between the time that
GCC moved to C++ and GDB moved to C++, leaving us with an ABI promise to
support a C-like interface.  The hope is to isolate GDB from some of this
should it change in the future.

Broadly, what this does is replace calls like:

  C_CTX (context)->c_ops->operation (C_CTX (context), ...);

with calls that now look like:

  context->c_plugin->operation (...);

This API will be further refined in following patches when struct
compile_instance/compile_c_instance are changed into classes.

gdb/ChangeLog:

	* Makefile.in (HFILES_NO_SRCDIR): Add compile/gcc-c-plugin.h.
	* compile/compile-c-types.c: Define GCC_METHODN macros and include
	gcc-c-fe.def to define C plugin.
	(delete_instance): Delete `c_plugin'.
	(new_compile_instance): Initialize `c_plugin'.
	* compile/compile-c.h: Include gcc_c_plugin.h.
	(struct compile_c_instance) <c_plugin>: New member.
	* gcc-c-plugin.h: New file.
	Update all callers with API change.
---
 gdb/Makefile.in                 |   1 +
 gdb/compile/compile-c-symbols.c |  30 ++++----
 gdb/compile/compile-c-types.c   | 153 ++++++++++++++++++++++------------------
 gdb/compile/compile-c.h         |   9 ++-
 gdb/compile/gcc-c-plugin.h      |  64 +++++++++++++++++
 5 files changed, 168 insertions(+), 89 deletions(-)
 create mode 100644 gdb/compile/gcc-c-plugin.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 67fff669ba..56883a0acd 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1456,6 +1456,7 @@ HFILES_NO_SRCDIR = \
 	compile/compile-internal.h \
 	compile/compile-object-load.h \
 	compile/compile-object-run.h \
+	compile/gcc-c-plugin.h \
 	config/nm-linux.h \
 	config/nm-nto.h \
 	config/djgpp/langinfo.h \
diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c
index c82e008659..abd829838b 100644
--- a/gdb/compile/compile-c-symbols.c
+++ b/gdb/compile/compile-c-symbols.c
@@ -161,9 +161,8 @@ convert_one_symbol (struct compile_c_instance *context,
   if (SYMBOL_DOMAIN (sym.symbol) == STRUCT_DOMAIN)
     {
       /* Binding a tag, so we don't need to build a decl.  */
-      C_CTX (context)->c_ops->tagbind (C_CTX (context),
-				       SYMBOL_NATURAL_NAME (sym.symbol),
-				       sym_type, filename, line);
+      context->c_plugin->tagbind (SYMBOL_NATURAL_NAME (sym.symbol),
+				  sym_type, filename, line);
     }
   else
     {
@@ -196,9 +195,8 @@ convert_one_symbol (struct compile_c_instance *context,
 	      /* Already handled by convert_enum.  */
 	      return;
 	    }
-	  C_CTX (context)->c_ops->build_constant
-	    (C_CTX (context),
-	     sym_type, SYMBOL_NATURAL_NAME (sym.symbol),
+	  context->c_plugin->build_constant
+	    (sym_type, SYMBOL_NATURAL_NAME (sym.symbol),
 	     SYMBOL_VALUE (sym.symbol),
 	     filename, line);
 	  return;
@@ -285,15 +283,14 @@ convert_one_symbol (struct compile_c_instance *context,
       if (context->base.scope != COMPILE_I_RAW_SCOPE
 	  || symbol_name == NULL)
 	{
-	  decl = C_CTX (context)->c_ops->build_decl
-	    (C_CTX (context),
-	     SYMBOL_NATURAL_NAME (sym.symbol),
+	  decl = context->c_plugin->build_decl
+	    (SYMBOL_NATURAL_NAME (sym.symbol),
 	     kind,
 	     sym_type,
 	     symbol_name.get (), addr,
 	     filename, line);
 
-	  C_CTX (context)->c_ops->bind (C_CTX (context), decl, is_global);
+	  context->c_plugin->bind (decl, is_global);
 	}
     }
 }
@@ -402,11 +399,10 @@ convert_symbol_bmsym (struct compile_c_instance *context,
     }
 
   sym_type = convert_type (context, type);
-  decl = C_CTX (context)->c_ops->build_decl (C_CTX (context),
-					     MSYMBOL_NATURAL_NAME (msym),
-					     kind, sym_type, NULL, addr,
-					     NULL, 0);
-  C_CTX (context)->c_ops->bind (C_CTX (context), decl, 1 /* is_global */);
+  decl = context->c_plugin->build_decl (MSYMBOL_NATURAL_NAME (msym),
+					kind, sym_type, NULL, addr,
+					NULL, 0);
+  context->c_plugin->bind (decl, 1 /* is_global */);
 }
 
 /* See compile-internal.h.  */
@@ -463,7 +459,7 @@ gcc_convert_symbol (void *datum,
 
   CATCH (e, RETURN_MASK_ALL)
     {
-      C_CTX (context)->c_ops->error (C_CTX (context), e.message);
+      context->c_plugin->error (e.message);
     }
   END_CATCH
 
@@ -525,7 +521,7 @@ gcc_symbol_address (void *datum, struct gcc_c_context *gcc_context,
 
   CATCH (e, RETURN_MASK_ERROR)
     {
-      C_CTX (context)->c_ops->error (C_CTX (context), e.message);
+      context->c_plugin->error (e.message);
     }
   END_CATCH
 
diff --git a/gdb/compile/compile-c-types.c b/gdb/compile/compile-c-types.c
index 46b9f2b3b4..927445208a 100644
--- a/gdb/compile/compile-c-types.c
+++ b/gdb/compile/compile-c-types.c
@@ -99,8 +99,7 @@ convert_pointer (struct compile_c_instance *context, struct type *type)
 {
   gcc_type target = convert_type (context, TYPE_TARGET_TYPE (type));
 
-  return C_CTX (context)->c_ops->build_pointer_type (C_CTX (context),
-						     target);
+  return context->c_plugin->build_pointer_type (target);
 }
 
 /* Convert an array type to its gcc representation.  */
@@ -114,13 +113,11 @@ convert_array (struct compile_c_instance *context, struct type *type)
   element_type = convert_type (context, TYPE_TARGET_TYPE (type));
 
   if (TYPE_LOW_BOUND_KIND (range) != PROP_CONST)
-    return C_CTX (context)->c_ops->error (C_CTX (context),
-					  _("array type with non-constant"
-					    " lower bound is not supported"));
+    return context->c_plugin->error (_("array type with non-constant"
+				       " lower bound is not supported"));
   if (TYPE_LOW_BOUND (range) != 0)
-    return C_CTX (context)->c_ops->error (C_CTX (context),
-					  _("cannot convert array type with "
-					    "non-zero lower bound to C"));
+    return context->c_plugin->error (_("cannot convert array type with "
+				       "non-zero lower bound to C"));
 
   if (TYPE_HIGH_BOUND_KIND (range) == PROP_LOCEXPR
       || TYPE_HIGH_BOUND_KIND (range) == PROP_LOCLIST)
@@ -128,15 +125,13 @@ convert_array (struct compile_c_instance *context, struct type *type)
       gcc_type result;
 
       if (TYPE_VECTOR (type))
-	return C_CTX (context)->c_ops->error (C_CTX (context),
-					      _("variably-sized vector type"
-						" is not supported"));
+	return context->c_plugin->error (_("variably-sized vector type"
+					   " is not supported"));
 
       std::string upper_bound
 	= c_get_range_decl_name (&TYPE_RANGE_DATA (range)->high);
-      result = C_CTX (context)->c_ops->build_vla_array_type (C_CTX (context),
-							     element_type,
-							     upper_bound.c_str ());
+      result = context->c_plugin->build_vla_array_type (element_type,
+							upper_bound.c_str ());
       return result;
     }
   else
@@ -152,11 +147,8 @@ convert_array (struct compile_c_instance *context, struct type *type)
 	}
 
       if (TYPE_VECTOR (type))
-	return C_CTX (context)->c_ops->build_vector_type (C_CTX (context),
-							  element_type,
-							  count);
-      return C_CTX (context)->c_ops->build_array_type (C_CTX (context),
-						       element_type, count);
+	return context->c_plugin->build_vector_type (element_type, count);
+      return context->c_plugin->build_array_type (element_type, count);
     }
 }
 
@@ -171,11 +163,11 @@ convert_struct_or_union (struct compile_c_instance *context, struct type *type)
   /* First we create the resulting type and enter it into our hash
      table.  This lets recursive types work.  */
   if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
-    result = C_CTX (context)->c_ops->build_record_type (C_CTX (context));
+    result = context->c_plugin->build_record_type ();
   else
     {
       gdb_assert (TYPE_CODE (type) == TYPE_CODE_UNION);
-      result = C_CTX (context)->c_ops->build_union_type (C_CTX (context));
+      result = context->c_plugin->build_union_type ();
     }
   insert_type (context, type, result);
 
@@ -187,15 +179,14 @@ convert_struct_or_union (struct compile_c_instance *context, struct type *type)
       field_type = convert_type (context, TYPE_FIELD_TYPE (type, i));
       if (bitsize == 0)
 	bitsize = 8 * TYPE_LENGTH (TYPE_FIELD_TYPE (type, i));
-      C_CTX (context)->c_ops->build_add_field (C_CTX (context), result,
-					       TYPE_FIELD_NAME (type, i),
-					       field_type,
-					       bitsize,
-					       TYPE_FIELD_BITPOS (type, i));
+      context->c_plugin->build_add_field (result,
+					  TYPE_FIELD_NAME (type, i),
+					  field_type,
+					  bitsize,
+					  TYPE_FIELD_BITPOS (type, i));
     }
 
-  C_CTX (context)->c_ops->finish_record_or_union (C_CTX (context), result,
-						  TYPE_LENGTH (type));
+  context->c_plugin->finish_record_or_union (result, TYPE_LENGTH (type));
   return result;
 }
 
@@ -206,22 +197,20 @@ convert_enum (struct compile_c_instance *context, struct type *type)
 {
   gcc_type int_type, result;
   int i;
-  struct gcc_c_context *ctx = C_CTX (context);
+  const gcc_c_plugin *plugin = context->c_plugin;
 
-  int_type = ctx->c_ops->int_type_v0 (ctx,
-				      TYPE_UNSIGNED (type),
-				      TYPE_LENGTH (type));
+  int_type = plugin->int_type_v0 (TYPE_UNSIGNED (type),
+					    TYPE_LENGTH (type));
 
-  result = ctx->c_ops->build_enum_type (ctx, int_type);
+  result = plugin->build_enum_type (int_type);
   for (i = 0; i < TYPE_NFIELDS (type); ++i)
     {
-      ctx->c_ops->build_add_enum_constant (ctx,
-					   result,
-					   TYPE_FIELD_NAME (type, i),
-					   TYPE_FIELD_ENUMVAL (type, i));
+      plugin->build_add_enum_constant (result,
+				       TYPE_FIELD_NAME (type, i),
+				       TYPE_FIELD_ENUMVAL (type, i));
     }
 
-  ctx->c_ops->finish_enum_type (ctx, result);
+  plugin->finish_enum_type (result);
 
   return result;
 }
@@ -261,9 +250,8 @@ convert_func (struct compile_c_instance *context, struct type *type)
   for (i = 0; i < TYPE_NFIELDS (type); ++i)
     array.elements[i] = convert_type (context, TYPE_FIELD_TYPE (type, i));
 
-  result = C_CTX (context)->c_ops->build_function_type (C_CTX (context),
-							return_type,
-							&array, is_varargs);
+  result = context->c_plugin->build_function_type (return_type,
+						   &array, is_varargs);
   xfree (array.elements);
 
   return result;
@@ -274,22 +262,20 @@ convert_func (struct compile_c_instance *context, struct type *type)
 static gcc_type
 convert_int (struct compile_c_instance *context, struct type *type)
 {
-  if (C_CTX (context)->c_ops->c_version >= GCC_C_FE_VERSION_1)
+  if (context->c_plugin->version () >= GCC_C_FE_VERSION_1)
     {
       if (TYPE_NOSIGN (type))
 	{
 	  gdb_assert (TYPE_LENGTH (type) == 1);
-	  return C_CTX (context)->c_ops->char_type (C_CTX (context));
+	  return context->c_plugin->char_type ();
 	}
-      return C_CTX (context)->c_ops->int_type (C_CTX (context),
-					       TYPE_UNSIGNED (type),
-					       TYPE_LENGTH (type),
-					       TYPE_NAME (type));
+      return context->c_plugin->int_type (TYPE_UNSIGNED (type),
+					  TYPE_LENGTH (type),
+					  TYPE_NAME (type));
     }
   else
-    return C_CTX (context)->c_ops->int_type_v0 (C_CTX (context),
-						TYPE_UNSIGNED (type),
-						TYPE_LENGTH (type));
+    return context->c_plugin->int_type_v0 (TYPE_UNSIGNED (type),
+					   TYPE_LENGTH (type));
 }
 
 /* Convert a floating-point type to its gcc representation.  */
@@ -297,13 +283,11 @@ convert_int (struct compile_c_instance *context, struct type *type)
 static gcc_type
 convert_float (struct compile_c_instance *context, struct type *type)
 {
-  if (C_CTX (context)->c_ops->c_version >= GCC_C_FE_VERSION_1)
-    return C_CTX (context)->c_ops->float_type (C_CTX (context),
-					       TYPE_LENGTH (type),
-					       TYPE_NAME (type));
+  if (context->c_plugin->version () >= GCC_C_FE_VERSION_1)
+    return context->c_plugin->float_type (TYPE_LENGTH (type),
+					  TYPE_NAME (type));
   else
-    return C_CTX (context)->c_ops->float_type_v0 (C_CTX (context),
-						  TYPE_LENGTH (type));
+    return context->c_plugin->float_type_v0 (TYPE_LENGTH (type));
 }
 
 /* Convert the 'void' type to its gcc representation.  */
@@ -311,7 +295,7 @@ convert_float (struct compile_c_instance *context, struct type *type)
 static gcc_type
 convert_void (struct compile_c_instance *context, struct type *type)
 {
-  return C_CTX (context)->c_ops->void_type (C_CTX (context));
+  return context->c_plugin->void_type ();
 }
 
 /* Convert a boolean type to its gcc representation.  */
@@ -319,7 +303,7 @@ convert_void (struct compile_c_instance *context, struct type *type)
 static gcc_type
 convert_bool (struct compile_c_instance *context, struct type *type)
 {
-  return C_CTX (context)->c_ops->bool_type (C_CTX (context));
+  return context->c_plugin->bool_type ();
 }
 
 /* Convert a qualified type to its gcc representation.  */
@@ -340,9 +324,7 @@ convert_qualified (struct compile_c_instance *context, struct type *type)
   if (TYPE_RESTRICT (type))
     quals |= GCC_QUALIFIER_RESTRICT;
 
-  return C_CTX (context)->c_ops->build_qualified_type (C_CTX (context),
-						       unqual_converted,
-						       quals);
+  return context->c_plugin->build_qualified_type (unqual_converted, quals);
 }
 
 /* Convert a complex type to its gcc representation.  */
@@ -352,7 +334,7 @@ convert_complex (struct compile_c_instance *context, struct type *type)
 {
   gcc_type base = convert_type (context, TYPE_TARGET_TYPE (type));
 
-  return C_CTX (context)->c_ops->build_complex_type (C_CTX (context), base);
+  return context->c_plugin->build_complex_type (base);
 }
 
 /* A helper function which knows how to convert most types from their
@@ -419,9 +401,7 @@ convert_type_basic (struct compile_c_instance *context, struct type *type)
       }
     }
 
-  return C_CTX (context)->c_ops->error (C_CTX (context),
-					_("cannot convert gdb type "
-					  "to gcc type"));
+  return context->c_plugin->error (_("cannot convert gdb type to gcc type"));
 }
 
 /* See compile-internal.h.  */
@@ -456,6 +436,7 @@ delete_instance (struct compile_instance *c)
   struct compile_c_instance *context = (struct compile_c_instance *) c;
 
   context->base.fe->ops->destroy (context->base.fe);
+  delete context->c_plugin;
   htab_delete (context->type_map);
   if (context->symbol_err_map != NULL)
     htab_delete (context->symbol_err_map);
@@ -481,8 +462,46 @@ new_compile_instance (struct gcc_c_context *fe)
 					eq_type_map_instance,
 					xfree, xcalloc, xfree);
 
-  fe->c_ops->set_callbacks (fe, gcc_convert_symbol,
-			    gcc_symbol_address, result);
+  result->c_plugin = new gcc_c_plugin (fe);
+
+  result->c_plugin->set_callbacks (gcc_convert_symbol, gcc_symbol_address,
+				   result);
 
   return &result->base;
 }
+
+/* C plug-in wrapper.  */
+
+#define FORWARD(OP,...) m_context->c_ops->OP(m_context, ##__VA_ARGS__)
+#define GCC_METHOD0(R, N) \
+  R gcc_c_plugin::N () const \
+  { return FORWARD (N); }
+#define GCC_METHOD1(R, N, A) \
+  R gcc_c_plugin::N (A a) const \
+  { return FORWARD (N, a); }
+#define GCC_METHOD2(R, N, A, B) \
+  R gcc_c_plugin::N (A a, B b) const \
+  { return FORWARD (N, a, b); }
+#define GCC_METHOD3(R, N, A, B, C) \
+  R gcc_c_plugin::N (A a, B b, C c)  const \
+  { return FORWARD (N, a, b, c); }
+#define GCC_METHOD4(R, N, A, B, C, D) \
+  R gcc_c_plugin::N (A a, B b, C c, D d) const \
+  { return FORWARD (N, a, b, c, d); }
+#define GCC_METHOD5(R, N, A, B, C, D, E) \
+  R gcc_c_plugin::N (A a, B b, C c, D d, E e) const \
+  { return FORWARD (N, a, b, c, d, e); }
+#define GCC_METHOD7(R, N, A, B, C, D, E, F, G) \
+  R gcc_c_plugin::N (A a, B b, C c, D d, E e, F f, G g) const \
+  { return FORWARD (N, a, b, c, d, e, f, g); }
+
+#include "gcc-c-fe.def"
+
+#undef GCC_METHOD0
+#undef GCC_METHOD1
+#undef GCC_METHOD2
+#undef GCC_METHOD3
+#undef GCC_METHOD4
+#undef GCC_METHOD5
+#undef GCC_METHOD7
+#undef FORWARD
diff --git a/gdb/compile/compile-c.h b/gdb/compile/compile-c.h
index cff2aef906..ffa58021e2 100644
--- a/gdb/compile/compile-c.h
+++ b/gdb/compile/compile-c.h
@@ -18,6 +18,7 @@
 #define GDB_COMPILE_C_H
 
 #include "common/enum-flags.h"
+#include "gcc-c-plugin.h"
 #include "hashtab.h"
 
 /* enum-flags wrapper.  */
@@ -46,12 +47,10 @@ struct compile_c_instance
 
   /* Map from gdb symbols to gcc error messages to emit.  */
   htab_t symbol_err_map;
-};
-
-/* A helper macro that takes a compile_c_instance and returns its
-   corresponding gcc_c_context.  */
 
-#define C_CTX(I) ((struct gcc_c_context *) ((I)->base.fe))
+  /* GCC C plugin.  */
+  gcc_c_plugin *c_plugin;
+};
 
 /* Emit code to compute the address for all the local variables in
    scope at PC in BLOCK.  Returns a malloc'd vector, indexed by gdb
diff --git a/gdb/compile/gcc-c-plugin.h b/gdb/compile/gcc-c-plugin.h
new file mode 100644
index 0000000000..12d422f6aa
--- /dev/null
+++ b/gdb/compile/gcc-c-plugin.h
@@ -0,0 +1,64 @@
+/* GCC C plug-in wrapper for GDB.
+
+   Copyright (C) 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* A class representing the C plug-in.  */
+
+class gcc_c_plugin
+{
+public:
+
+  explicit gcc_c_plugin (struct gcc_c_context *gcc_c)
+    : m_context (gcc_c)
+  {
+  }
+
+  /* Set the oracle callbacks to be used by the compiler plug-in.  */
+  void set_callbacks (gcc_c_oracle_function *binding_oracle,
+		      gcc_c_symbol_address_function *address_oracle,
+		      void *datum)
+  {
+    m_context->c_ops->set_callbacks (m_context, binding_oracle,
+				     address_oracle, datum);
+  }
+
+  /* Returns the interface version of the compiler plug-in.  */
+  int version () const { return m_context->c_ops->c_version; }
+
+#define GCC_METHOD0(R, N) R N () const;
+#define GCC_METHOD1(R, N, A) R N (A) const;
+#define GCC_METHOD2(R, N, A, B) R N (A, B) const;
+#define GCC_METHOD3(R, N, A, B, C) R N (A, B, C) const;
+#define GCC_METHOD4(R, N, A, B, C, D) R N (A, B, C, D) const;
+#define GCC_METHOD5(R, N, A, B, C, D, E) R N (A, B, C, D, E) const;
+#define GCC_METHOD7(R, N, A, B, C, D, E, F, G) R N (A, B, C, D, E, F, G) const;
+
+#include "gcc-c-fe.def"
+
+#undef GCC_METHOD0
+#undef GCC_METHOD1
+#undef GCC_METHOD2
+#undef GCC_METHOD3
+#undef GCC_METHOD4
+#undef GCC_METHOD5
+#undef GCC_METHOD7
+
+private:
+  /* The GCC C context.  */
+  struct gcc_c_context *m_context;
+};
-- 
2.13.6

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 7/8] Move compile_instance to compile.c
  2018-05-03 18:42 [PATCH 0/8] Convert C compile to C++ Keith Seitz
                   ` (5 preceding siblings ...)
  2018-05-03 18:50 ` [PATCH 8/8] Use policies for code generation Keith Seitz
@ 2018-05-03 18:50 ` Keith Seitz
  2018-05-03 18:51 ` [PATCH 5/8] Change compile_instance/compile_c_instance into classes Keith Seitz
  2018-06-06 14:38 ` [PATCH 0/8] Convert C compile to C++ Pedro Alves
  8 siblings, 0 replies; 21+ messages in thread
From: Keith Seitz @ 2018-05-03 18:50 UTC (permalink / raw)
  To: gdb-patches

This simple patch moves any code related to compile_instance into
compile.c, reserving compile-c-* files strictly for C language support.

gdb/ChangeLog:

	* compile/compile-c-symbols.c (compile_instance::insert_symbol_error)
	(compile_instance::error_symbol_once): Move to compile.c.
	* compile/compile-c-types.c (compile_instance::insert_type): Move
	to ...
	* compile/compile.c: Here.
---
 gdb/compile/compile-c-symbols.c | 30 ---------------------------
 gdb/compile/compile-c-types.c   | 20 ------------------
 gdb/compile/compile.c           | 45 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 50 deletions(-)

diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c
index 0b74e4b4cd..61ef5e378e 100644
--- a/gdb/compile/compile-c-symbols.c
+++ b/gdb/compile/compile-c-symbols.c
@@ -31,36 +31,6 @@
 #include "gdbtypes.h"
 #include "dwarf2loc.h"
 
-\f
-
-/* See compile-internal.h.  */
-
-void
-compile_instance::insert_symbol_error (const struct symbol *sym,
-				       std::string text)
-{
-  symbol_err_map_t::iterator pos = m_symbol_err_map.find (sym);
-
-  if (pos == m_symbol_err_map.end ())
-    m_symbol_err_map.insert (std::make_pair (sym, text));
-}
-
-/* See compile-internal.h.  */
-
-void
-compile_instance::error_symbol_once (const struct symbol *sym)
-{
-  symbol_err_map_t::iterator pos = m_symbol_err_map.find (sym);
-  if (pos == m_symbol_err_map.end () || pos->second.length () == 0)
-    return;
-
-  std::string message (pos->second);
-  pos->second.clear ();
-  ::error (_("%s"), message.c_str ());
-}
-
-\f
-
 /* Compute the name of the pointer representing a local symbol's
    address.  */
 
diff --git a/gdb/compile/compile-c-types.c b/gdb/compile/compile-c-types.c
index 1ad6c2a4da..f5f99e4062 100644
--- a/gdb/compile/compile-c-types.c
+++ b/gdb/compile/compile-c-types.c
@@ -24,26 +24,6 @@
 #include "compile-c.h"
 #include "objfiles.h"
 
-/* See compile-internal.h.  */
-
-void
-compile_instance::insert_type (struct type *type, gcc_type gcc_type)
-{
-  type_map_t::iterator pos = m_type_map.find (type);
-
-  if (pos != m_type_map.end ())
-    {
-      /* The type might have already been inserted in order to handle
-         recursive types.  */
-      if (pos->second != gcc_type)
-        error (_("Unexpected type id from GCC, check for recent "
-                 "enough GCC."));
-    }
-  else
-    m_type_map.insert (std::make_pair (type, gcc_type));
-
-}
-
 /* Convert a pointer type to its gcc representation.  */
 
 static gcc_type
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index c965f575fd..3d8a93907d 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -661,6 +661,51 @@ compile_register_name_demangle (struct gdbarch *gdbarch,
   error (_("Cannot find gdbarch register \"%s\"."), regname);
 }
 
+/* See compile-internal.h.  */
+
+void
+compile_instance::insert_type (struct type *type, gcc_type gcc_type)
+{
+  type_map_t::iterator pos = m_type_map.find (type);
+
+  if (pos != m_type_map.end ())
+    {
+      /* The type might have already been inserted in order to handle
+         recursive types.  */
+      if (pos->second != gcc_type)
+        error (_("Unexpected type id from GCC, check for recent "
+                 "enough GCC."));
+    }
+  else
+    m_type_map.insert (std::make_pair (type, gcc_type));
+}
+
+/* See compile-internal.h.  */
+
+void
+compile_instance::insert_symbol_error (const struct symbol *sym,
+				       std::string text)
+{
+  symbol_err_map_t::iterator pos = m_symbol_err_map.find (sym);
+
+  if (pos == m_symbol_err_map.end ())
+    m_symbol_err_map.insert (std::make_pair (sym, text));
+}
+
+/* See compile-internal.h.  */
+
+void
+compile_instance::error_symbol_once (const struct symbol *sym)
+{
+  symbol_err_map_t::iterator pos = m_symbol_err_map.find (sym);
+  if (pos == m_symbol_err_map.end () || pos->second.length () == 0)
+    return;
+
+  std::string message (pos->second);
+  pos->second.clear ();
+  ::error (_("%s"), message.c_str ());
+}
+
 /* Forwards to the plug-in.  */
 
 #define FORWARD(OP,...) (m_gcc_fe->ops->OP (m_gcc_fe, ##__VA_ARGS__))
-- 
2.13.6

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 8/8] Use policies for code generation
  2018-05-03 18:42 [PATCH 0/8] Convert C compile to C++ Keith Seitz
                   ` (4 preceding siblings ...)
  2018-05-03 18:49 ` [PATCH 6/8] Use std::unordered_map instead of htab_t Keith Seitz
@ 2018-05-03 18:50 ` Keith Seitz
  2018-05-03 18:50 ` [PATCH 7/8] Move compile_instance to compile.c Keith Seitz
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Keith Seitz @ 2018-05-03 18:50 UTC (permalink / raw)
  To: gdb-patches

This patch changes code generation procedures add_code_header,
add_code_footer, and several other language-specific code generation
functions into policies.

gdb/ChangeLog:

	* compile/compile-c-support.c (add_code_header, add_code_footer):
	Move into policy class.
	(c_push_user_expression, pop_user_expression_nop)
	(c_add_code_header, c_add_code_footer, c_add_input): New policy class.
	(compile_program): New host class.
	(c_compile_program): New typedef.
	(c_compute_porgram): Use c_compile_program.
---
 gdb/compile/compile-c-support.c | 413 +++++++++++++++++++++++++---------------
 1 file changed, 256 insertions(+), 157 deletions(-)

diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index d77c7d9625..e8993aa5eb 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -180,71 +180,6 @@ write_macro_definitions (const struct block *block, CORE_ADDR pc,
     }
 }
 
-/* Helper function to construct a header scope for a block of code.
-   Takes a scope argument which selects the correct header to
-   insert into BUF.  */
-
-static void
-add_code_header (enum compile_i_scope_types type, struct ui_file *buf)
-{
-  switch (type)
-    {
-    case COMPILE_I_SIMPLE_SCOPE:
-      fputs_unfiltered ("void "
-			GCC_FE_WRAPPER_FUNCTION
-			" (struct "
-			COMPILE_I_SIMPLE_REGISTER_STRUCT_TAG
-			" *"
-			COMPILE_I_SIMPLE_REGISTER_ARG_NAME
-			") {\n",
-			buf);
-      break;
-    case COMPILE_I_PRINT_ADDRESS_SCOPE:
-    case COMPILE_I_PRINT_VALUE_SCOPE:
-      /* <string.h> is needed for a memcpy call below.  */
-      fputs_unfiltered ("#include <string.h>\n"
-			"void "
-			GCC_FE_WRAPPER_FUNCTION
-			" (struct "
-			COMPILE_I_SIMPLE_REGISTER_STRUCT_TAG
-			" *"
-			COMPILE_I_SIMPLE_REGISTER_ARG_NAME
-			", "
-			COMPILE_I_PRINT_OUT_ARG_TYPE
-			" "
-			COMPILE_I_PRINT_OUT_ARG
-			") {\n",
-			buf);
-      break;
-
-    case COMPILE_I_RAW_SCOPE:
-      break;
-    default:
-      gdb_assert_not_reached (_("Unknown compiler scope reached."));
-    }
-}
-
-/* Helper function to construct a footer scope for a block of code.
-   Takes a scope argument which selects the correct footer to
-   insert into BUF.  */
-
-static void
-add_code_footer (enum compile_i_scope_types type, struct ui_file *buf)
-{
-  switch (type)
-    {
-    case COMPILE_I_SIMPLE_SCOPE:
-    case COMPILE_I_PRINT_ADDRESS_SCOPE:
-    case COMPILE_I_PRINT_VALUE_SCOPE:
-      fputs_unfiltered ("}\n", buf);
-      break;
-    case COMPILE_I_RAW_SCOPE:
-      break;
-    default:
-      gdb_assert_not_reached (_("Unknown compiler scope reached."));
-    }
-}
-
 /* Generate a structure holding all the registers used by the function
    we're generating.  */
 
@@ -325,113 +260,277 @@ generate_register_struct (struct ui_file *stream, struct gdbarch *gdbarch,
   fputs_unfiltered ("};\n\n", stream);
 }
 
-/* Take the source code provided by the user with the 'compile'
-   command, and compute the additional wrapping, macro, variable and
-   register operations needed.  INPUT is the source code derived from
-   the 'compile' command, GDBARCH is the architecture to use when
-   computing above, EXPR_BLOCK denotes the block relevant contextually
-   to the inferior when the expression was created, and EXPR_PC
-   indicates the value of $PC.  */
+/* C-language policy to emit a push user expression pragma into BUF.  */
 
-std::string
-c_compute_program (compile_instance *inst,
-		   const char *input,
-		   struct gdbarch *gdbarch,
-		   const struct block *expr_block,
-		   CORE_ADDR expr_pc)
+struct c_push_user_expression
 {
-  compile_c_instance *context
-    = static_cast<compile_c_instance *> (inst);
+  void push_user_expression (struct ui_file *buf)
+  {
+    fputs_unfiltered ("#pragma GCC user_expression\n", buf);
+  }
+};
 
-  string_file buf;
-  string_file var_stream;
+/* C-language policy to emit a pop user expression pragma into BUF.
+   For C, this is a nop.  */
 
-  write_macro_definitions (expr_block, expr_pc, &buf);
+struct pop_user_expression_nop
+{
+  void pop_user_expression (struct ui_file *buf)
+  {
+    /* Nothing to do.  */
+  }
+};
+
+/* C-language policy to construct a code header for a block of code.
+   Takes a scope TYPE argument which selects the correct header to
+   insert into BUF.  */
 
-  /* Do not generate local variable information for "raw"
-     compilations.  In this case we aren't emitting our own function
-     and the user's code may only refer to globals.  */
-  if (inst->scope () != COMPILE_I_RAW_SCOPE)
-    {
-      int i;
+struct c_add_code_header
+{
+  void add_code_header (enum compile_i_scope_types type, struct ui_file *buf)
+  {
+    switch (type)
+      {
+      case COMPILE_I_SIMPLE_SCOPE:
+	fputs_unfiltered ("void "
+			  GCC_FE_WRAPPER_FUNCTION
+			  " (struct "
+			  COMPILE_I_SIMPLE_REGISTER_STRUCT_TAG
+			  " *"
+			  COMPILE_I_SIMPLE_REGISTER_ARG_NAME
+			  ") {\n",
+			  buf);
+	break;
+
+      case COMPILE_I_PRINT_ADDRESS_SCOPE:
+      case COMPILE_I_PRINT_VALUE_SCOPE:
+	/* <string.h> is needed for a memcpy call below.  */
+	fputs_unfiltered ("#include <string.h>\n"
+			  "void "
+			  GCC_FE_WRAPPER_FUNCTION
+			  " (struct "
+			  COMPILE_I_SIMPLE_REGISTER_STRUCT_TAG
+			  " *"
+			  COMPILE_I_SIMPLE_REGISTER_ARG_NAME
+			  ", "
+			  COMPILE_I_PRINT_OUT_ARG_TYPE
+			  " "
+			  COMPILE_I_PRINT_OUT_ARG
+			  ") {\n",
+			  buf);
+	break;
+
+      case COMPILE_I_RAW_SCOPE:
+	break;
+
+      default:
+	gdb_assert_not_reached (_("Unknown compiler scope reached."));
+      }
+  }
+};
 
-      /* Generate the code to compute variable locations, but do it
-	 before generating the function header, so we can define the
-	 register struct before the function body.  This requires a
-	 temporary stream.  */
-      gdb::unique_xmalloc_ptr<unsigned char> registers_used
-	= generate_c_for_variable_locations (context, var_stream, gdbarch,
-					     expr_block, expr_pc);
-
-      buf.puts ("typedef unsigned int"
-		" __attribute__ ((__mode__(__pointer__)))"
-		" __gdb_uintptr;\n");
-      buf.puts ("typedef int"
-		" __attribute__ ((__mode__(__pointer__)))"
-		" __gdb_intptr;\n");
-
-      /* Iterate all log2 sizes in bytes supported by c_get_mode_for_size.  */
-      for (i = 0; i < 4; ++i)
-	{
-	  const char *mode = c_get_mode_for_size (1 << i);
+/* C-language policy to construct a code footer for a block of code.
+   Takes a scope TYPE which selects the correct footer to insert into BUF.  */
 
-	  gdb_assert (mode != NULL);
-	  buf.printf ("typedef int"
-		      " __attribute__ ((__mode__(__%s__)))"
-		      " __gdb_int_%s;\n",
-		      mode, mode);
-	}
+struct c_add_code_footer
+{
+  void add_code_footer (enum compile_i_scope_types type, struct ui_file *buf)
+  {
+    switch (type)
+      {
+      case COMPILE_I_SIMPLE_SCOPE:
+      case COMPILE_I_PRINT_ADDRESS_SCOPE:
+      case COMPILE_I_PRINT_VALUE_SCOPE:
+	fputs_unfiltered ("}\n", buf);
+	break;
 
-      generate_register_struct (&buf, gdbarch, registers_used.get ());
-    }
+      case COMPILE_I_RAW_SCOPE:
+	break;
 
-  add_code_header (inst->scope (), &buf);
+      default:
+	gdb_assert_not_reached (_("Unknown compiler scope reached."));
+      }
+  }
+};
 
-  if (inst->scope () == COMPILE_I_SIMPLE_SCOPE
-      || inst->scope () == COMPILE_I_PRINT_ADDRESS_SCOPE
-      || inst->scope () == COMPILE_I_PRINT_VALUE_SCOPE)
-    {
-      buf.write (var_stream.c_str (), var_stream.size ());
-      buf.puts ("#pragma GCC user_expression\n");
-    }
+/* C-language policy to emit the user code snippet INPUT into BUF based on the
+   scope TYPE.  */
 
-  /* The user expression has to be in its own scope, so that "extern"
-     works properly.  Otherwise gcc thinks that the "extern"
-     declaration is in the same scope as the declaration provided by
-     gdb.  */
-  if (inst->scope () != COMPILE_I_RAW_SCOPE)
-    buf.puts ("{\n");
+struct c_add_input
+{
+  void add_input (enum compile_i_scope_types type, const char *input,
+		  struct ui_file *buf)
+  {
+    switch (type)
+      {
+      case COMPILE_I_PRINT_ADDRESS_SCOPE:
+      case COMPILE_I_PRINT_VALUE_SCOPE:
+	fprintf_unfiltered (buf,
+			    "__auto_type " COMPILE_I_EXPR_VAL " = %s;\n"
+			    "typeof (%s) *" COMPILE_I_EXPR_PTR_TYPE ";\n"
+			    "memcpy (" COMPILE_I_PRINT_OUT_ARG ", %s"
+			    COMPILE_I_EXPR_VAL ",\n"
+			    "sizeof (*" COMPILE_I_EXPR_PTR_TYPE "));\n"
+			    , input, input,
+			    (type == COMPILE_I_PRINT_ADDRESS_SCOPE
+			     ? "&" : ""));
+	break;
+
+      default:
+	fputs_unfiltered (input, buf);
+	break;
+      }
+    fputs_unfiltered ("\n", buf);
+  }
+};
 
-  buf.puts ("#line 1 \"gdb command line\"\n");
+/* A host class representing a compile program.
 
-  switch (inst->scope ())
-    {
-    case COMPILE_I_PRINT_ADDRESS_SCOPE:
-    case COMPILE_I_PRINT_VALUE_SCOPE:
-      buf.printf (
-"__auto_type " COMPILE_I_EXPR_VAL " = %s;\n"
-"typeof (%s) *" COMPILE_I_EXPR_PTR_TYPE ";\n"
-"memcpy (" COMPILE_I_PRINT_OUT_ARG ", %s" COMPILE_I_EXPR_VAL ",\n"
-	 "sizeof (*" COMPILE_I_EXPR_PTR_TYPE "));\n"
-			  , input, input,
-			  (inst->scope () == COMPILE_I_PRINT_ADDRESS_SCOPE
-			   ? "&" : ""));
-      break;
-    default:
-      buf.puts (input);
-      break;
-    }
+   CompileInstanceType is the type of the compile_instance for the
+   language.
+
+   PushUserExpressionPolicy and PopUserExpressionPolicy are used to
+   push and pop user expression pragmas to the compile plug-in.
+
+   AddCodeHeaderPolicy and AddCodeFooterPolicy are used to add the appropriate
+   code header and footer, respectively.
 
-  buf.puts ("\n");
+   AddInputPolicy adds the actual user code.  */
 
-  /* For larger user expressions the automatic semicolons may be
-     confusing.  */
-  if (strchr (input, '\n') == NULL)
-    buf.puts (";\n");
+template <class CompileInstanceType, class PushUserExpressionPolicy,
+	  class PopUserExpressionPolicy, class AddCodeHeaderPolicy,
+	  class AddCodeFooterPolicy, class AddInputPolicy>
+class compile_program
+  : private PushUserExpressionPolicy, private PopUserExpressionPolicy,
+    private AddCodeHeaderPolicy, private AddCodeFooterPolicy,
+    private AddInputPolicy
+{
+public:
+
+  /* Construct a compile_program using the compiler instance INST
+     using the architecture given by GDBARCH.  */
+  compile_program (CompileInstanceType *inst, struct gdbarch *gdbarch)
+    : m_instance (inst), m_arch (gdbarch)
+  {
+  }
+
+  /* Take the source code provided by the user with the 'compile'
+     command and compute the additional wrapping, macro, variable and
+     register operations needed.  INPUT is the source code derived from
+     the 'compile' command, EXPR_BLOCK denotes the block relevant contextually
+     to the inferior when the expression was created, and EXPR_PC
+     indicates the value of $PC.
+
+     Returns the text of the program to compile.  */
+  std::string compute (const char *input, const struct block *expr_block,
+		       CORE_ADDR expr_pc)
+  {
+    string_file var_stream;
+    string_file buf;
+
+    /* Do not generate local variable information for "raw"
+       compilations.  In this case we aren't emitting our own function
+       and the user's code may only refer to globals.  */
+    if (m_instance->scope () != COMPILE_I_RAW_SCOPE)
+      {
+	/* Generate the code to compute variable locations, but do it
+	   before generating the function header, so we can define the
+	   register struct before the function body.  This requires a
+	   temporary stream.  */
+	gdb::unique_xmalloc_ptr<unsigned char> registers_used
+	  = generate_c_for_variable_locations (m_instance, var_stream, m_arch,
+					       expr_block, expr_pc);
+
+	buf.puts ("typedef unsigned int"
+		  " __attribute__ ((__mode__(__pointer__)))"
+		  " __gdb_uintptr;\n");
+	buf.puts ("typedef int"
+		  " __attribute__ ((__mode__(__pointer__)))"
+		  " __gdb_intptr;\n");
+
+	/* Iterate all log2 sizes in bytes supported by c_get_mode_for_size.  */
+	for (int i = 0; i < 4; ++i)
+	  {
+	    const char *mode = c_get_mode_for_size (1 << i);
+
+	    gdb_assert (mode != NULL);
+	    buf.printf ("typedef int"
+			" __attribute__ ((__mode__(__%s__)))"
+			" __gdb_int_%s;\n",
+			mode, mode);
+	  }
+
+	generate_register_struct (&buf, m_arch, registers_used.get ());
+      }
+
+    AddCodeHeaderPolicy::add_code_header (m_instance->scope (), &buf);
+
+    if (m_instance->scope () == COMPILE_I_SIMPLE_SCOPE
+	|| m_instance->scope () == COMPILE_I_PRINT_ADDRESS_SCOPE
+	|| m_instance->scope () == COMPILE_I_PRINT_VALUE_SCOPE)
+      {
+	buf.write (var_stream.c_str (), var_stream.size ());
+	PushUserExpressionPolicy::push_user_expression (&buf);
+      }
 
-  if (inst->scope () != COMPILE_I_RAW_SCOPE)
-    buf.puts ("}\n");
+    write_macro_definitions (expr_block, expr_pc, &buf);
+
+    /* The user expression has to be in its own scope, so that "extern"
+       works properly.  Otherwise gcc thinks that the "extern"
+       declaration is in the same scope as the declaration provided by
+       gdb.  */
+    if (m_instance->scope () != COMPILE_I_RAW_SCOPE)
+      buf.puts ("{\n");
+
+    buf.puts ("#line 1 \"gdb command line\"\n");
+
+    AddInputPolicy::add_input (m_instance->scope (), input, &buf);
+
+    /* For larger user expressions the automatic semicolons may be
+       confusing.  */
+    if (strchr (input, '\n') == NULL)
+      buf.puts (";\n");
+
+    if (m_instance->scope () != COMPILE_I_RAW_SCOPE)
+      buf.puts ("}\n");
+
+    if (m_instance->scope () == COMPILE_I_SIMPLE_SCOPE
+	|| m_instance->scope () == COMPILE_I_PRINT_ADDRESS_SCOPE
+	|| m_instance->scope () == COMPILE_I_PRINT_VALUE_SCOPE)
+      PopUserExpressionPolicy::pop_user_expression (&buf);
+
+    AddCodeFooterPolicy::add_code_footer (m_instance->scope (), &buf);
+    return buf.string ();
+  }
+
+private:
+
+  /* The compile instance to be used for compilation and
+     type-conversion.  */
+  CompileInstanceType *m_instance;
+
+  /* The architecture to be used.  */
+  struct gdbarch *m_arch;
+};
+
+/* Type used for C program computations.  */
+
+typedef compile_program<compile_c_instance,
+			c_push_user_expression, pop_user_expression_nop,
+			c_add_code_header, c_add_code_footer,
+			c_add_input> c_compile_program;
+
+/* The la_compute_program method for C.  */
+
+std::string
+c_compute_program (compile_instance *inst,
+		   const char *input,
+		   struct gdbarch *gdbarch,
+		   const struct block *expr_block,
+		   CORE_ADDR expr_pc)
+{
+  compile_c_instance *c_inst = static_cast<compile_c_instance *> (inst);
+  c_compile_program program (c_inst, gdbarch);
 
-  add_code_footer (inst->scope (), &buf);
-  return std::move (buf.string ());
+  return program.compute (input, expr_block, expr_pc);
 }
-- 
2.13.6

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 5/8] Change compile_instance/compile_c_instance into classes
  2018-05-03 18:42 [PATCH 0/8] Convert C compile to C++ Keith Seitz
                   ` (6 preceding siblings ...)
  2018-05-03 18:50 ` [PATCH 7/8] Move compile_instance to compile.c Keith Seitz
@ 2018-05-03 18:51 ` Keith Seitz
  2018-06-06 14:38 ` [PATCH 0/8] Convert C compile to C++ Pedro Alves
  8 siblings, 0 replies; 21+ messages in thread
From: Keith Seitz @ 2018-05-03 18:51 UTC (permalink / raw)
  To: gdb-patches

This patch changes structs compile_instance and compile_c_instance into
classes.

Because of the nature of the change, there are a number of unavoidably
mechanical changes buried in here, such as turning variable access of the
POD struct into method calls, removing the struct keyword, and changing
access of the plugin from "c_plugin->operation()" to
"plugin ()->operation ()".

There is one "non-trivial" change associated with this patch, though.
The type cache and symbol error maps have been moved into the base class,
believing these facilities would be used other language implementations.
[They are indeed re-used by C++.]

gdb/ChangeLog:

	* compile/compile-c-support.c (c_get_compile_context): Use `new'
	instead of `new_compile_instance'.
	* compile/compile-c-symbols.c (compile_instance::insert_symbol_error):
	Update description.
	If the symbol error map is not initialized, create it.
	(generate_c_for_for_one_symbol): Do not check/initialize
	the symbol error map.
	* compile/compile-c-types.c (compile_c_instance): Make a class.
	Update all callers.
	(compile_instance::compile_instance): Initialize the type cache.
	(insert_type): Update description.
	(compile_c_instance::m_default_cflags): Define.
	(convert_type): Update description.
	(delete_instance): Moved to destructor.
	(new_compile_instance): Moved to constructor.
	* compile/compile-c.h (compile_c_instance): Make class inheriting
	from compile_instance.
	<base>: Remove field.
	<type_map, symbol_err_map>: Move to base class.
	<c_plugin>: Rename to `m_plugin' and remove pointer type.
	* compile/compile-internal.h (compile_instance): Make class.
	<type_map_t, symbol_err_map_t>: Define.
	<fe>: Rename to `m_gcc_fe'.
	<scope, block, gcc_target_options>: Add `m_' prefix.
	<m_type_map, m_symbol_err_map>: New fields, moved from
	compile_c_instance.
	<destroy>: Remove.
	(convert_type, new_compile_instance): Remove.
	* compile/compile.c (cleanup_compile_instance): Remove.
	(compile_to_object): Use unique_ptr to eliminate cleanups.
	(compile_instance::set_print_callback, compile_instance::version)
	(compile_instance::set_verbose)
	(compile_instance::set_driver_filename)
	(compile_instance::set_triplet_regexp)
	(compile_instance::set_arguments)
	(compile_instance::set_source_file)
	(compile_instance::compile): Define.
---
 gdb/c-lang.h                    |   4 +-
 gdb/compile/compile-c-support.c |  29 +++---
 gdb/compile/compile-c-symbols.c |  84 +++++++++--------
 gdb/compile/compile-c-types.c   | 197 +++++++++++++++++-----------------------
 gdb/compile/compile-c.h         |  37 +++++---
 gdb/compile/compile-internal.h  | 122 ++++++++++++++++++++-----
 gdb/compile/compile.c           | 153 ++++++++++++++++++++-----------
 gdb/language.h                  |   6 +-
 8 files changed, 375 insertions(+), 257 deletions(-)

diff --git a/gdb/c-lang.h b/gdb/c-lang.h
index 18dedcc1fa..ebe2124652 100644
--- a/gdb/c-lang.h
+++ b/gdb/c-lang.h
@@ -153,7 +153,7 @@ extern int c_textual_element_type (struct type *, char);
    exception on failure.  This is suitable for use as the
    la_get_compile_instance language method.  */
 
-extern struct compile_instance *c_get_compile_context (void);
+extern compile_instance *c_get_compile_context (void);
 
 /* This takes the user-supplied text and returns a new bit of code to
    compile.
@@ -161,7 +161,7 @@ extern struct compile_instance *c_get_compile_context (void);
    This is used as the la_compute_program language method; see that
    for a description of the arguments.  */
 
-extern std::string c_compute_program (struct compile_instance *inst,
+extern std::string c_compute_program (compile_instance *inst,
 				      const char *input,
 				      struct gdbarch *gdbarch,
 				      const struct block *expr_block,
diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index 41fead9ad1..d77c7d9625 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -96,7 +96,7 @@ load_libcc (void)
    This function calls the symbol returned from the load_libcc
    function.  This will provide the gcc_c_context.  */
 
-struct compile_instance *
+compile_instance *
 c_get_compile_context (void)
 {
   static gcc_c_fe_context_function *func;
@@ -114,7 +114,7 @@ c_get_compile_context (void)
     error (_("The loaded version of GCC does not support the required version "
 	     "of the API."));
 
-  return new_compile_instance (context);
+  return new compile_c_instance (context);
 }
 
 \f
@@ -334,13 +334,14 @@ generate_register_struct (struct ui_file *stream, struct gdbarch *gdbarch,
    indicates the value of $PC.  */
 
 std::string
-c_compute_program (struct compile_instance *inst,
+c_compute_program (compile_instance *inst,
 		   const char *input,
 		   struct gdbarch *gdbarch,
 		   const struct block *expr_block,
 		   CORE_ADDR expr_pc)
 {
-  struct compile_c_instance *context = (struct compile_c_instance *) inst;
+  compile_c_instance *context
+    = static_cast<compile_c_instance *> (inst);
 
   string_file buf;
   string_file var_stream;
@@ -350,7 +351,7 @@ c_compute_program (struct compile_instance *inst,
   /* Do not generate local variable information for "raw"
      compilations.  In this case we aren't emitting our own function
      and the user's code may only refer to globals.  */
-  if (inst->scope != COMPILE_I_RAW_SCOPE)
+  if (inst->scope () != COMPILE_I_RAW_SCOPE)
     {
       int i;
 
@@ -384,11 +385,11 @@ c_compute_program (struct compile_instance *inst,
       generate_register_struct (&buf, gdbarch, registers_used.get ());
     }
 
-  add_code_header (inst->scope, &buf);
+  add_code_header (inst->scope (), &buf);
 
-  if (inst->scope == COMPILE_I_SIMPLE_SCOPE
-      || inst->scope == COMPILE_I_PRINT_ADDRESS_SCOPE
-      || inst->scope == COMPILE_I_PRINT_VALUE_SCOPE)
+  if (inst->scope () == COMPILE_I_SIMPLE_SCOPE
+      || inst->scope () == COMPILE_I_PRINT_ADDRESS_SCOPE
+      || inst->scope () == COMPILE_I_PRINT_VALUE_SCOPE)
     {
       buf.write (var_stream.c_str (), var_stream.size ());
       buf.puts ("#pragma GCC user_expression\n");
@@ -398,12 +399,12 @@ c_compute_program (struct compile_instance *inst,
      works properly.  Otherwise gcc thinks that the "extern"
      declaration is in the same scope as the declaration provided by
      gdb.  */
-  if (inst->scope != COMPILE_I_RAW_SCOPE)
+  if (inst->scope () != COMPILE_I_RAW_SCOPE)
     buf.puts ("{\n");
 
   buf.puts ("#line 1 \"gdb command line\"\n");
 
-  switch (inst->scope)
+  switch (inst->scope ())
     {
     case COMPILE_I_PRINT_ADDRESS_SCOPE:
     case COMPILE_I_PRINT_VALUE_SCOPE:
@@ -413,7 +414,7 @@ c_compute_program (struct compile_instance *inst,
 "memcpy (" COMPILE_I_PRINT_OUT_ARG ", %s" COMPILE_I_EXPR_VAL ",\n"
 	 "sizeof (*" COMPILE_I_EXPR_PTR_TYPE "));\n"
 			  , input, input,
-			  (inst->scope == COMPILE_I_PRINT_ADDRESS_SCOPE
+			  (inst->scope () == COMPILE_I_PRINT_ADDRESS_SCOPE
 			   ? "&" : ""));
       break;
     default:
@@ -428,9 +429,9 @@ c_compute_program (struct compile_instance *inst,
   if (strchr (input, '\n') == NULL)
     buf.puts (";\n");
 
-  if (inst->scope != COMPILE_I_RAW_SCOPE)
+  if (inst->scope () != COMPILE_I_RAW_SCOPE)
     buf.puts ("}\n");
 
-  add_code_footer (inst->scope, &buf);
+  add_code_footer (inst->scope (), &buf);
   return std::move (buf.string ());
 }
diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c
index abd829838b..7bc30a5b3d 100644
--- a/gdb/compile/compile-c-symbols.c
+++ b/gdb/compile/compile-c-symbols.c
@@ -79,16 +79,27 @@ del_symbol_error (void *a)
   xfree (se);
 }
 
-/* Associate SYMBOL with some error text.  */
+/* See compile-internal.h.  */
 
-static void
-insert_symbol_error (htab_t hash, const struct symbol *sym, const char *text)
+void
+compile_instance::insert_symbol_error (const struct symbol *sym,
+				       const char *text)
 {
   struct symbol_error e;
   void **slot;
 
+  if (m_symbol_err_map == NULL)
+    {
+      m_symbol_err_map = htab_create_alloc (10,
+					    hash_symbol_error,
+					    eq_symbol_error,
+					    del_symbol_error,
+					    xcalloc,
+					    xfree);
+    }
+
   e.sym = sym;
-  slot = htab_find_slot (hash, &e, INSERT);
+  slot = htab_find_slot (m_symbol_err_map, &e, INSERT);
   if (*slot == NULL)
     {
       struct symbol_error *e = XNEW (struct symbol_error);
@@ -99,21 +110,19 @@ insert_symbol_error (htab_t hash, const struct symbol *sym, const char *text)
     }
 }
 
-/* Emit the error message corresponding to SYM, if one exists, and
-   arrange for it not to be emitted again.  */
+/* See compile-internal.h.  */
 
-static void
-error_symbol_once (struct compile_c_instance *context,
-		   const struct symbol *sym)
+void
+compile_instance::error_symbol_once (const struct symbol *sym)
 {
   struct symbol_error search;
   struct symbol_error *err;
 
-  if (context->symbol_err_map == NULL)
+  if (m_symbol_err_map == NULL)
     return;
 
   search.sym = sym;
-  err = (struct symbol_error *) htab_find (context->symbol_err_map, &search);
+  err = (struct symbol_error *) htab_find (m_symbol_err_map, &search);
   if (err == NULL || err->message == NULL)
     return;
 
@@ -142,7 +151,7 @@ c_symbol_substitution_name (struct symbol *sym)
    scope.)  */
 
 static void
-convert_one_symbol (struct compile_c_instance *context,
+convert_one_symbol (compile_c_instance *context,
 		    struct block_symbol sym,
 		    int is_global,
 		    int is_local)
@@ -151,17 +160,17 @@ convert_one_symbol (struct compile_c_instance *context,
   const char *filename = symbol_symtab (sym.symbol)->filename;
   unsigned short line = SYMBOL_LINE (sym.symbol);
 
-  error_symbol_once (context, sym.symbol);
+  context->error_symbol_once (sym.symbol);
 
   if (SYMBOL_CLASS (sym.symbol) == LOC_LABEL)
     sym_type = 0;
   else
-    sym_type = convert_type (context, SYMBOL_TYPE (sym.symbol));
+    sym_type = context->convert_type (SYMBOL_TYPE (sym.symbol));
 
   if (SYMBOL_DOMAIN (sym.symbol) == STRUCT_DOMAIN)
     {
       /* Binding a tag, so we don't need to build a decl.  */
-      context->c_plugin->tagbind (SYMBOL_NATURAL_NAME (sym.symbol),
+      context->plugin ()->tagbind (SYMBOL_NATURAL_NAME (sym.symbol),
 				  sym_type, filename, line);
     }
   else
@@ -195,7 +204,7 @@ convert_one_symbol (struct compile_c_instance *context,
 	      /* Already handled by convert_enum.  */
 	      return;
 	    }
-	  context->c_plugin->build_constant
+	  context->plugin ()->build_constant
 	    (sym_type, SYMBOL_NATURAL_NAME (sym.symbol),
 	     SYMBOL_VALUE (sym.symbol),
 	     filename, line);
@@ -280,17 +289,17 @@ convert_one_symbol (struct compile_c_instance *context,
 	}
 
       /* Don't emit local variable decls for a raw expression.  */
-      if (context->base.scope != COMPILE_I_RAW_SCOPE
+      if (context->scope () != COMPILE_I_RAW_SCOPE
 	  || symbol_name == NULL)
 	{
-	  decl = context->c_plugin->build_decl
+	  decl = context->plugin ()->build_decl
 	    (SYMBOL_NATURAL_NAME (sym.symbol),
 	     kind,
 	     sym_type,
 	     symbol_name.get (), addr,
 	     filename, line);
 
-	  context->c_plugin->bind (decl, is_global);
+	  context->plugin ()->bind (decl, is_global);
 	}
     }
 }
@@ -300,7 +309,7 @@ convert_one_symbol (struct compile_c_instance *context,
    itself, and DOMAIN is the domain which was searched.  */
 
 static void
-convert_symbol_sym (struct compile_c_instance *context, const char *identifier,
+convert_symbol_sym (compile_c_instance *context, const char *identifier,
 		    struct block_symbol sym, domain_enum domain)
 {
   const struct block *static_block;
@@ -350,7 +359,7 @@ convert_symbol_sym (struct compile_c_instance *context, const char *identifier,
    to use and BMSYM is the minimal symbol to convert.  */
 
 static void
-convert_symbol_bmsym (struct compile_c_instance *context,
+convert_symbol_bmsym (compile_c_instance *context,
 		      struct bound_minimal_symbol bmsym)
 {
   struct minimal_symbol *msym = bmsym.minsym;
@@ -398,11 +407,11 @@ convert_symbol_bmsym (struct compile_c_instance *context,
       break;
     }
 
-  sym_type = convert_type (context, type);
-  decl = context->c_plugin->build_decl (MSYMBOL_NATURAL_NAME (msym),
+  sym_type = context->convert_type (type);
+  decl = context->plugin ()->build_decl (MSYMBOL_NATURAL_NAME (msym),
 					kind, sym_type, NULL, addr,
 					NULL, 0);
-  context->c_plugin->bind (decl, 1 /* is_global */);
+  context->plugin ()->bind (decl, 1 /* is_global */);
 }
 
 /* See compile-internal.h.  */
@@ -413,7 +422,8 @@ gcc_convert_symbol (void *datum,
 		    enum gcc_c_oracle_request request,
 		    const char *identifier)
 {
-  struct compile_c_instance *context = (struct compile_c_instance *) datum;
+  compile_c_instance *context
+    = static_cast<compile_c_instance *> (datum);
   domain_enum domain;
   int found = 0;
 
@@ -438,7 +448,7 @@ gcc_convert_symbol (void *datum,
     {
       struct block_symbol sym;
 
-      sym = lookup_symbol (identifier, context->base.block, domain, NULL);
+      sym = lookup_symbol (identifier, context->block (), domain, NULL);
       if (sym.symbol != NULL)
 	{
 	  convert_symbol_sym (context, identifier, sym, domain);
@@ -459,7 +469,7 @@ gcc_convert_symbol (void *datum,
 
   CATCH (e, RETURN_MASK_ALL)
     {
-      context->c_plugin->error (e.message);
+      context->plugin ()->error (e.message);
     }
   END_CATCH
 
@@ -476,7 +486,8 @@ gcc_address
 gcc_symbol_address (void *datum, struct gcc_c_context *gcc_context,
 		    const char *identifier)
 {
-  struct compile_c_instance *context = (struct compile_c_instance *) datum;
+  compile_c_instance *context
+    = static_cast<compile_c_instance *> (datum);
   gcc_address result = 0;
   int found = 0;
 
@@ -521,7 +532,7 @@ gcc_symbol_address (void *datum, struct gcc_c_context *gcc_context,
 
   CATCH (e, RETURN_MASK_ERROR)
     {
-      context->c_plugin->error (e.message);
+      context->plugin ()->error (e.message);
     }
   END_CATCH
 
@@ -575,7 +586,7 @@ symbol_seen (htab_t hashtab, struct symbol *sym)
 /* Generate C code to compute the length of a VLA.  */
 
 static void
-generate_vla_size (struct compile_c_instance *compiler,
+generate_vla_size (compile_c_instance *compiler,
 		   string_file &stream,
 		   struct gdbarch *gdbarch,
 		   unsigned char *registers_used,
@@ -629,7 +640,7 @@ generate_vla_size (struct compile_c_instance *compiler,
 /* Generate C code to compute the address of SYM.  */
 
 static void
-generate_c_for_for_one_variable (struct compile_c_instance *compiler,
+generate_c_for_for_one_variable (compile_c_instance *compiler,
 				 string_file &stream,
 				 struct gdbarch *gdbarch,
 				 unsigned char *registers_used,
@@ -691,14 +702,7 @@ generate_c_for_for_one_variable (struct compile_c_instance *compiler,
 
   CATCH (e, RETURN_MASK_ERROR)
     {
-      if (compiler->symbol_err_map == NULL)
-	compiler->symbol_err_map = htab_create_alloc (10,
-						      hash_symbol_error,
-						      eq_symbol_error,
-						      del_symbol_error,
-						      xcalloc,
-						      xfree);
-      insert_symbol_error (compiler->symbol_err_map, sym, e.message);
+      compiler->insert_symbol_error (sym, e.message);
     }
   END_CATCH
 }
@@ -706,7 +710,7 @@ generate_c_for_for_one_variable (struct compile_c_instance *compiler,
 /* See compile-c.h.  */
 
 gdb::unique_xmalloc_ptr<unsigned char>
-generate_c_for_variable_locations (struct compile_c_instance *compiler,
+generate_c_for_variable_locations (compile_c_instance *compiler,
 				   string_file &stream,
 				   struct gdbarch *gdbarch,
 				   const struct block *block,
diff --git a/gdb/compile/compile-c-types.c b/gdb/compile/compile-c-types.c
index 927445208a..44b8317739 100644
--- a/gdb/compile/compile-c-types.c
+++ b/gdb/compile/compile-c-types.c
@@ -58,25 +58,31 @@ eq_type_map_instance (const void *a, const void *b)
   return insta->type == instb->type;
 }
 
+/* Constructor for compile_instance.  */
+
+compile_instance::compile_instance (struct gcc_base_context *gcc_fe,
+				    const char *options)
+  : m_gcc_fe (gcc_fe), m_gcc_target_options (options),
+    m_symbol_err_map (NULL)
+{
+  m_type_map = htab_create_alloc (10, hash_type_map_instance,
+				  eq_type_map_instance,
+				  xfree, xcalloc, xfree);
+}
+
 \f
 
-/* Insert an entry into the type map associated with CONTEXT that maps
-   from the gdb type TYPE to the gcc type GCC_TYPE.  It is ok for a
-   given type to be inserted more than once, provided that the exact
-   same association is made each time.  This simplifies how type
-   caching works elsewhere in this file -- see how struct type caching
-   is handled.  */
+/* See compile-internal.h.  */
 
-static void
-insert_type (struct compile_c_instance *context, struct type *type,
-	     gcc_type gcc_type)
+void
+compile_instance::insert_type (struct type *type, gcc_type gcc_type)
 {
   struct type_map_instance inst, *add;
   void **slot;
 
   inst.type = type;
   inst.gcc_type_handle = gcc_type;
-  slot = htab_find_slot (context->type_map, &inst, INSERT);
+  slot = htab_find_slot (m_type_map, &inst, INSERT);
 
   add = (struct type_map_instance *) *slot;
   /* The type might have already been inserted in order to handle
@@ -95,28 +101,28 @@ insert_type (struct compile_c_instance *context, struct type *type,
 /* Convert a pointer type to its gcc representation.  */
 
 static gcc_type
-convert_pointer (struct compile_c_instance *context, struct type *type)
+convert_pointer (compile_c_instance *context, struct type *type)
 {
-  gcc_type target = convert_type (context, TYPE_TARGET_TYPE (type));
+  gcc_type target = context->convert_type (TYPE_TARGET_TYPE (type));
 
-  return context->c_plugin->build_pointer_type (target);
+  return context->plugin ()->build_pointer_type (target);
 }
 
 /* Convert an array type to its gcc representation.  */
 
 static gcc_type
-convert_array (struct compile_c_instance *context, struct type *type)
+convert_array (compile_c_instance *context, struct type *type)
 {
   gcc_type element_type;
   struct type *range = TYPE_INDEX_TYPE (type);
 
-  element_type = convert_type (context, TYPE_TARGET_TYPE (type));
+  element_type = context->convert_type (TYPE_TARGET_TYPE (type));
 
   if (TYPE_LOW_BOUND_KIND (range) != PROP_CONST)
-    return context->c_plugin->error (_("array type with non-constant"
+    return context->plugin ()->error (_("array type with non-constant"
 				       " lower bound is not supported"));
   if (TYPE_LOW_BOUND (range) != 0)
-    return context->c_plugin->error (_("cannot convert array type with "
+    return context->plugin ()->error (_("cannot convert array type with "
 				       "non-zero lower bound to C"));
 
   if (TYPE_HIGH_BOUND_KIND (range) == PROP_LOCEXPR
@@ -125,12 +131,12 @@ convert_array (struct compile_c_instance *context, struct type *type)
       gcc_type result;
 
       if (TYPE_VECTOR (type))
-	return context->c_plugin->error (_("variably-sized vector type"
+	return context->plugin ()->error (_("variably-sized vector type"
 					   " is not supported"));
 
       std::string upper_bound
 	= c_get_range_decl_name (&TYPE_RANGE_DATA (range)->high);
-      result = context->c_plugin->build_vla_array_type (element_type,
+      result = context->plugin ()->build_vla_array_type (element_type,
 							upper_bound.c_str ());
       return result;
     }
@@ -147,15 +153,15 @@ convert_array (struct compile_c_instance *context, struct type *type)
 	}
 
       if (TYPE_VECTOR (type))
-	return context->c_plugin->build_vector_type (element_type, count);
-      return context->c_plugin->build_array_type (element_type, count);
+	return context->plugin ()->build_vector_type (element_type, count);
+      return context->plugin ()->build_array_type (element_type, count);
     }
 }
 
 /* Convert a struct or union type to its gcc representation.  */
 
 static gcc_type
-convert_struct_or_union (struct compile_c_instance *context, struct type *type)
+convert_struct_or_union (compile_c_instance *context, struct type *type)
 {
   int i;
   gcc_type result;
@@ -163,54 +169,52 @@ convert_struct_or_union (struct compile_c_instance *context, struct type *type)
   /* First we create the resulting type and enter it into our hash
      table.  This lets recursive types work.  */
   if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
-    result = context->c_plugin->build_record_type ();
+    result = context->plugin ()->build_record_type ();
   else
     {
       gdb_assert (TYPE_CODE (type) == TYPE_CODE_UNION);
-      result = context->c_plugin->build_union_type ();
+      result = context->plugin ()->build_union_type ();
     }
-  insert_type (context, type, result);
+  context->insert_type (type, result);
 
   for (i = 0; i < TYPE_NFIELDS (type); ++i)
     {
       gcc_type field_type;
       unsigned long bitsize = TYPE_FIELD_BITSIZE (type, i);
 
-      field_type = convert_type (context, TYPE_FIELD_TYPE (type, i));
+      field_type = context->convert_type (TYPE_FIELD_TYPE (type, i));
       if (bitsize == 0)
 	bitsize = 8 * TYPE_LENGTH (TYPE_FIELD_TYPE (type, i));
-      context->c_plugin->build_add_field (result,
+      context->plugin ()->build_add_field (result,
 					  TYPE_FIELD_NAME (type, i),
 					  field_type,
 					  bitsize,
 					  TYPE_FIELD_BITPOS (type, i));
     }
 
-  context->c_plugin->finish_record_or_union (result, TYPE_LENGTH (type));
+  context->plugin ()->finish_record_or_union (result, TYPE_LENGTH (type));
   return result;
 }
 
 /* Convert an enum type to its gcc representation.  */
 
 static gcc_type
-convert_enum (struct compile_c_instance *context, struct type *type)
+convert_enum (compile_c_instance *context, struct type *type)
 {
   gcc_type int_type, result;
   int i;
-  const gcc_c_plugin *plugin = context->c_plugin;
 
-  int_type = plugin->int_type_v0 (TYPE_UNSIGNED (type),
-					    TYPE_LENGTH (type));
+  int_type = context->plugin ()->int_type_v0 (TYPE_UNSIGNED (type),
+					      TYPE_LENGTH (type));
 
-  result = plugin->build_enum_type (int_type);
+  result = context->plugin ()->build_enum_type (int_type);
   for (i = 0; i < TYPE_NFIELDS (type); ++i)
     {
-      plugin->build_add_enum_constant (result,
-				       TYPE_FIELD_NAME (type, i),
-				       TYPE_FIELD_ENUMVAL (type, i));
+      context->plugin ()->build_add_enum_constant
+	(result, TYPE_FIELD_NAME (type, i), TYPE_FIELD_ENUMVAL (type, i));
     }
 
-  plugin->finish_enum_type (result);
+  context->plugin ()->finish_enum_type (result);
 
   return result;
 }
@@ -218,7 +222,7 @@ convert_enum (struct compile_c_instance *context, struct type *type)
 /* Convert a function type to its gcc representation.  */
 
 static gcc_type
-convert_func (struct compile_c_instance *context, struct type *type)
+convert_func (compile_c_instance *context, struct type *type)
 {
   int i;
   gcc_type result, return_type;
@@ -243,14 +247,14 @@ convert_func (struct compile_c_instance *context, struct type *type)
 
   /* This approach means we can't make self-referential function
      types.  Those are impossible in C, though.  */
-  return_type = convert_type (context, target_type);
+  return_type = context->convert_type (target_type);
 
   array.n_elements = TYPE_NFIELDS (type);
   array.elements = XNEWVEC (gcc_type, TYPE_NFIELDS (type));
   for (i = 0; i < TYPE_NFIELDS (type); ++i)
-    array.elements[i] = convert_type (context, TYPE_FIELD_TYPE (type, i));
+    array.elements[i] = context->convert_type (TYPE_FIELD_TYPE (type, i));
 
-  result = context->c_plugin->build_function_type (return_type,
+  result = context->plugin ()->build_function_type (return_type,
 						   &array, is_varargs);
   xfree (array.elements);
 
@@ -260,62 +264,62 @@ convert_func (struct compile_c_instance *context, struct type *type)
 /* Convert an integer type to its gcc representation.  */
 
 static gcc_type
-convert_int (struct compile_c_instance *context, struct type *type)
+convert_int (compile_c_instance *context, struct type *type)
 {
-  if (context->c_plugin->version () >= GCC_C_FE_VERSION_1)
+  if (context->plugin ()->version () >= GCC_C_FE_VERSION_1)
     {
       if (TYPE_NOSIGN (type))
 	{
 	  gdb_assert (TYPE_LENGTH (type) == 1);
-	  return context->c_plugin->char_type ();
+	  return context->plugin ()->char_type ();
 	}
-      return context->c_plugin->int_type (TYPE_UNSIGNED (type),
-					  TYPE_LENGTH (type),
-					  TYPE_NAME (type));
+      return context->plugin ()->int_type (TYPE_UNSIGNED (type),
+					   TYPE_LENGTH (type),
+					   TYPE_NAME (type));
     }
   else
-    return context->c_plugin->int_type_v0 (TYPE_UNSIGNED (type),
-					   TYPE_LENGTH (type));
+    return context->plugin ()->int_type_v0 (TYPE_UNSIGNED (type),
+					    TYPE_LENGTH (type));
 }
 
 /* Convert a floating-point type to its gcc representation.  */
 
 static gcc_type
-convert_float (struct compile_c_instance *context, struct type *type)
+convert_float (compile_c_instance *context, struct type *type)
 {
-  if (context->c_plugin->version () >= GCC_C_FE_VERSION_1)
-    return context->c_plugin->float_type (TYPE_LENGTH (type),
-					  TYPE_NAME (type));
+  if (context->plugin ()->version () >= GCC_C_FE_VERSION_1)
+    return context->plugin ()->float_type (TYPE_LENGTH (type),
+					   TYPE_NAME (type));
   else
-    return context->c_plugin->float_type_v0 (TYPE_LENGTH (type));
+    return context->plugin ()->float_type_v0 (TYPE_LENGTH (type));
 }
 
 /* Convert the 'void' type to its gcc representation.  */
 
 static gcc_type
-convert_void (struct compile_c_instance *context, struct type *type)
+convert_void (compile_c_instance *context, struct type *type)
 {
-  return context->c_plugin->void_type ();
+  return context->plugin ()->void_type ();
 }
 
 /* Convert a boolean type to its gcc representation.  */
 
 static gcc_type
-convert_bool (struct compile_c_instance *context, struct type *type)
+convert_bool (compile_c_instance *context, struct type *type)
 {
-  return context->c_plugin->bool_type ();
+  return context->plugin ()->bool_type ();
 }
 
 /* Convert a qualified type to its gcc representation.  */
 
 static gcc_type
-convert_qualified (struct compile_c_instance *context, struct type *type)
+convert_qualified (compile_c_instance *context, struct type *type)
 {
   struct type *unqual = make_unqualified_type (type);
   gcc_type unqual_converted;
   gcc_qualifiers_flags quals = 0;
 
-  unqual_converted = convert_type (context, unqual);
+  unqual_converted = context->convert_type (unqual);
 
   if (TYPE_CONST (type))
     quals |= GCC_QUALIFIER_CONST;
@@ -324,17 +328,17 @@ convert_qualified (struct compile_c_instance *context, struct type *type)
   if (TYPE_RESTRICT (type))
     quals |= GCC_QUALIFIER_RESTRICT;
 
-  return context->c_plugin->build_qualified_type (unqual_converted, quals);
+  return context->plugin ()->build_qualified_type (unqual_converted, quals);
 }
 
 /* Convert a complex type to its gcc representation.  */
 
 static gcc_type
-convert_complex (struct compile_c_instance *context, struct type *type)
+convert_complex (compile_c_instance *context, struct type *type)
 {
-  gcc_type base = convert_type (context, TYPE_TARGET_TYPE (type));
+  gcc_type base = context->convert_type (TYPE_TARGET_TYPE (type));
 
-  return context->c_plugin->build_complex_type (base);
+  return context->plugin ()->build_complex_type (base);
 }
 
 /* A helper function which knows how to convert most types from their
@@ -343,7 +347,7 @@ convert_complex (struct compile_c_instance *context, struct type *type)
    returns the gcc type.  */
 
 static gcc_type
-convert_type_basic (struct compile_c_instance *context, struct type *type)
+convert_type_basic (compile_c_instance *context, struct type *type)
 {
   /* If we are converting a qualified type, first convert the
      unqualified type and then apply the qualifiers.  */
@@ -401,13 +405,22 @@ convert_type_basic (struct compile_c_instance *context, struct type *type)
       }
     }
 
-  return context->c_plugin->error (_("cannot convert gdb type to gcc type"));
+  return context->plugin ()->error (_("cannot convert gdb type to gcc type"));
 }
 
-/* See compile-internal.h.  */
+/* Default compile flags for C.  */
+
+const char *compile_c_instance::m_default_cflags = "-std=gnu11"
+  /* Otherwise the .o file may need
+     "_Unwind_Resume" and
+     "__gcc_personality_v0".  */
+  " -fno-exceptions"
+  " -Wno-implicit-function-declaration";
+
+/* See compile-c.h.  */
 
 gcc_type
-convert_type (struct compile_c_instance *context, struct type *type)
+compile_c_instance::convert_type (struct type *type)
 {
   struct type_map_instance inst, *found;
   gcc_type result;
@@ -417,59 +430,17 @@ convert_type (struct compile_c_instance *context, struct type *type)
   type = check_typedef (type);
 
   inst.type = type;
-  found = (struct type_map_instance *) htab_find (context->type_map, &inst);
+  found = (struct type_map_instance *) htab_find (m_type_map, &inst);
   if (found != NULL)
     return found->gcc_type_handle;
 
-  result = convert_type_basic (context, type);
-  insert_type (context, type, result);
+  result = convert_type_basic (this, type);
+  insert_type (type, result);
   return result;
 }
 
 \f
 
-/* Delete the compiler instance C.  */
-
-static void
-delete_instance (struct compile_instance *c)
-{
-  struct compile_c_instance *context = (struct compile_c_instance *) c;
-
-  context->base.fe->ops->destroy (context->base.fe);
-  delete context->c_plugin;
-  htab_delete (context->type_map);
-  if (context->symbol_err_map != NULL)
-    htab_delete (context->symbol_err_map);
-  xfree (context);
-}
-
-/* See compile-internal.h.  */
-
-struct compile_instance *
-new_compile_instance (struct gcc_c_context *fe)
-{
-  struct compile_c_instance *result = XCNEW (struct compile_c_instance);
-
-  result->base.fe = &fe->base;
-  result->base.destroy = delete_instance;
-  result->base.gcc_target_options = ("-std=gnu11"
-				     /* Otherwise the .o file may need
-					"_Unwind_Resume" and
-					"__gcc_personality_v0".  */
-				     " -fno-exceptions");
-
-  result->type_map = htab_create_alloc (10, hash_type_map_instance,
-					eq_type_map_instance,
-					xfree, xcalloc, xfree);
-
-  result->c_plugin = new gcc_c_plugin (fe);
-
-  result->c_plugin->set_callbacks (gcc_convert_symbol, gcc_symbol_address,
-				   result);
-
-  return &result->base;
-}
-
 /* C plug-in wrapper.  */
 
 #define FORWARD(OP,...) m_context->c_ops->OP(m_context, ##__VA_ARGS__)
diff --git a/gdb/compile/compile-c.h b/gdb/compile/compile-c.h
index ffa58021e2..2b80755d4e 100644
--- a/gdb/compile/compile-c.h
+++ b/gdb/compile/compile-c.h
@@ -36,20 +36,35 @@ extern gcc_c_symbol_address_function gcc_symbol_address;
 /* A subclass of compile_instance that is specific to the C front
    end.  */
 
-struct compile_c_instance
+class compile_c_instance : public compile_instance
 {
-  /* Base class.  Note that the base class vtable actually points to a
-     gcc_c_fe_vtable.  */
-  struct compile_instance base;
+public:
+  explicit compile_c_instance (struct gcc_c_context *gcc_c)
+    : compile_instance (&gcc_c->base, m_default_cflags),
+      m_plugin (gcc_c)
+  {
+    m_plugin.set_callbacks (gcc_convert_symbol, gcc_symbol_address, this);
+  }
 
-  /* Map from gdb types to gcc types.  */
-  htab_t type_map;
+  ~compile_c_instance ()
+  {
+    m_gcc_fe->ops->destroy (m_gcc_fe);
+  }
 
-  /* Map from gdb symbols to gcc error messages to emit.  */
-  htab_t symbol_err_map;
+  /* Convert a gdb type, TYPE, to a GCC type.
 
-  /* GCC C plugin.  */
-  gcc_c_plugin *c_plugin;
+     The new GCC type is returned.  */
+  gcc_type convert_type (struct type *type);
+
+  /* Return a handle for the GCC plug-in.  */
+  gcc_c_plugin *plugin () { return &m_plugin; }
+
+private:
+  /* Default compiler flags for C.  */
+  static const char *m_default_cflags;
+
+  /* The GCC plug-in.  */
+  gcc_c_plugin m_plugin;
 };
 
 /* Emit code to compute the address for all the local variables in
@@ -59,7 +74,7 @@ struct compile_c_instance
 
 extern gdb::unique_xmalloc_ptr<unsigned char>
   generate_c_for_variable_locations
-     (struct compile_c_instance *compiler,
+     (compile_c_instance *compiler,
       string_file &stream,
       struct gdbarch *gdbarch,
       const struct block *block,
diff --git a/gdb/compile/compile-internal.h b/gdb/compile/compile-internal.h
index afe20e5141..8cfb214c02 100644
--- a/gdb/compile/compile-internal.h
+++ b/gdb/compile/compile-internal.h
@@ -28,28 +28,116 @@ struct block;
 /* An object of this type holds state associated with a given
    compilation job.  */
 
-struct compile_instance
+class compile_instance
 {
-  /* The GCC front end.  */
+public:
+  compile_instance (struct gcc_base_context *gcc_fe, const char *options);
+
+  virtual ~compile_instance ()
+  {
+    htab_delete (m_type_map);
+    if (m_symbol_err_map != NULL)
+      htab_delete (m_symbol_err_map);
+  }
+
+  /* Returns the GCC options to be passed during compilation.  */
+  const std::string &gcc_target_options () const
+  {
+    return m_gcc_target_options;
+  }
+
+  /* Insert GCC_TYPE into the type cache for TYPE.
+
+     It is ok for a given type to be inserted more than once, provided that
+     the exact same association is made each time.  */
+  void insert_type (struct type *type, gcc_type gcc_type);
+
+  /* Associate SYMBOL with some error text.  */
+  void insert_symbol_error (const struct symbol *sym, const char *text);
+
+  /* Emit the error message corresponding to SYM, if one exists, and
+     arrange for it not to be emitted again.  */
+  void error_symbol_once (const struct symbol *sym);
+
+  /* These currently just forward to the underlying ops
+     vtable.  */
+
+  /* Set the plug-in print callback.  */
+  void set_print_callback (void (*print_function) (void *, const char *),
+			   void *datum);
+
+  /* Return the plug-in's front-end version.  */
+  unsigned int version () const;
+
+  /* Set the plug-in's verbosity level.  Nop for GCC_FE_VERSION_0.  */
+  void set_verbose (int level);
+
+  /* Set the plug-in driver program.  Nop for GCC_FE_VERSION_0.  */
+  void set_driver_filename (const char *filename);
+
+  /* Set the regular expression used to match the configury triplet
+     prefix to the compiler.  Nop for GCC_FE_VERSION_0.  */
+  void set_triplet_regexp (const char *regexp);
+
+  /* Set compilation arguments.  REGEXP is only used for protocol
+     version GCC_FE_VERSION_0.  */
+  char *set_arguments (int argc, char **argv, const char *regexp = NULL);
+
+  /* Set the filename of the program to compile.  Nop for GCC_FE_VERSION_0.  */
+  void set_source_file (const char *filename);
+
+  /* Compile the previously specified source file to FILENAME.
+     VERBOSE_LEVEL is only used for protocol version GCC_FE_VERSION_0.  */
+  bool compile (const char *filename, int verbose_level = -1);
+
+  /* Set the scope type for this compile.  */
+  void set_scope (enum compile_i_scope_types scope)
+  {
+    m_scope = scope;
+  }
+
+  /* Return the scope type.  */
+  enum compile_i_scope_types scope () const
+  {
+    return m_scope;
+  }
+
+  /* Set the block to be used for symbol searches.  */
+  void set_block (const struct block *block)
+  {
+    m_block = block;
+  }
+
+  /* Return the search block.  */
+  const struct block *block () const
+  {
+    return m_block;
+  }
+
+protected:
+  /* Map types used by the compile instance for caching type conversions.
+     and error tracking.  */
+  typedef htab_t type_map_t;
+  typedef htab_t symbol_err_map_t;
 
-  struct gcc_base_context *fe;
+  /* The GCC front end.  */
+  struct gcc_base_context *m_gcc_fe;
 
   /* The "scope" of this compilation.  */
-
-  enum compile_i_scope_types scope;
+  enum compile_i_scope_types m_scope;
 
   /* The block in which an expression is being parsed.  */
-
-  const struct block *block;
+  const struct block *m_block;
 
   /* Specify "-std=gnu11", "-std=gnu++11" or similar.  These options are put
      after CU's DW_AT_producer compilation options to override them.  */
+  std::string m_gcc_target_options;
 
-  const char *gcc_target_options;
+  /* Map from gdb types to gcc types.  */
+  type_map_t m_type_map;
 
-  /* How to destroy this object.  */
-
-  void (*destroy) (struct compile_instance *);
+  /* Map from gdb symbols to gcc error messages to emit.  */
+  symbol_err_map_t m_symbol_err_map;
 };
 
 /* Define header and footers for different scopes.  */
@@ -79,18 +167,6 @@ extern std::string compile_register_name_mangled (struct gdbarch *gdbarch,
 extern int compile_register_name_demangle (struct gdbarch *gdbarch,
 					   const char *reg_name);
 
-/* Convert a gdb type, TYPE, to a GCC type.  CONTEXT is used to do the
-   actual conversion.  The new GCC type is returned.  */
-
-struct type;
-extern gcc_type convert_type (struct compile_c_instance *context,
-			      struct type *type);
-
-/* Instantiate a GDB object holding state for the GCC context FE.  The
-   new object is returned.  */
-
-extern struct compile_instance *new_compile_instance (struct gcc_c_context *fe);
-
 /* Type used to hold and pass around the source and object file names
    to use for compilation.  */
 class compile_file_names
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 7f35272872..c965f575fd 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -415,7 +415,7 @@ filter_args (int *argcp, char **argv)
    generated above.  */
 
 static void
-get_args (const struct compile_instance *compiler, struct gdbarch *gdbarch,
+get_args (const compile_instance *compiler, struct gdbarch *gdbarch,
 	  int *argcp, char ***argvp)
 {
   const char *cs_producer_options;
@@ -437,7 +437,7 @@ get_args (const struct compile_instance *compiler, struct gdbarch *gdbarch,
       freeargv (argv_producer);
     }
 
-  build_argc_argv (compiler->gcc_target_options,
+  build_argc_argv (compiler->gcc_target_options ().c_str (),
 		   &argc_compiler, &argv_compiler);
   append_args (argcp, argvp, argc_compiler, argv_compiler);
   freeargv (argv_compiler);
@@ -445,16 +445,6 @@ get_args (const struct compile_instance *compiler, struct gdbarch *gdbarch,
   append_args (argcp, argvp, compile_args_argc, compile_args_argv);
 }
 
-/* A cleanup function to destroy a gdb_gcc_instance.  */
-
-static void
-cleanup_compile_instance (void *arg)
-{
-  struct compile_instance *inst = (struct compile_instance *) arg;
-
-  inst->destroy (inst);
-}
-
 /* A helper function suitable for use as the "print_callback" in the
    compiler object.  */
 
@@ -472,8 +462,6 @@ static compile_file_names
 compile_to_object (struct command_line *cmd, const char *cmd_string,
 		   enum compile_i_scope_types scope)
 {
-  struct compile_instance *compiler;
-  struct cleanup *cleanup;
   const struct block *expr_block;
   CORE_ADDR trash_pc, expr_pc;
   int argc;
@@ -481,7 +469,6 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
   int ok;
   struct gdbarch *gdbarch = get_current_arch ();
   std::string triplet_rx;
-  char *error_message;
 
   if (!target_has_execution)
     error (_("The program must be running for the compile command to "\
@@ -494,13 +481,13 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
   if (current_language->la_get_compile_instance == NULL)
     error (_("No compiler support for language %s."),
 	   current_language->la_name);
-  compiler = current_language->la_get_compile_instance ();
-  cleanup = make_cleanup (cleanup_compile_instance, compiler);
 
-  compiler->fe->ops->set_print_callback (compiler->fe, print_callback, NULL);
-
-  compiler->scope = scope;
-  compiler->block = expr_block;
+  compile_instance *compiler_instance
+    = current_language->la_get_compile_instance ();
+  std::unique_ptr<compile_instance> compiler (compiler_instance);
+  compiler->set_print_callback (print_callback, NULL);
+  compiler->set_scope (scope);
+  compiler->set_block (expr_block);
 
   /* From the provided expression, build a scope to pass to the
      compiler.  */
@@ -526,21 +513,20 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
     error (_("Neither a simple expression, or a multi-line specified."));
 
   std::string code
-    = current_language->la_compute_program (compiler, input, gdbarch,
+    = current_language->la_compute_program (compiler.get (), input, gdbarch,
 					    expr_block, expr_pc);
   if (compile_debug)
     fprintf_unfiltered (gdb_stdlog, "debug output:\n\n%s", code.c_str ());
 
-  if (compiler->fe->ops->version >= GCC_FE_VERSION_1)
-    compiler->fe->ops->set_verbose (compiler->fe, compile_debug);
+  compiler->set_verbose (compile_debug);
 
   if (compile_gcc[0] != 0)
     {
-      if (compiler->fe->ops->version < GCC_FE_VERSION_1)
+      if (compiler->version () < GCC_FE_VERSION_1)
 	error (_("Command 'set compile-gcc' requires GCC version 6 or higher "
 		 "(libcc1 interface version 1 or higher)"));
 
-      compiler->fe->ops->set_driver_filename (compiler->fe, compile_gcc);
+      compiler->set_driver_filename (compile_gcc);
     }
   else
     {
@@ -549,27 +535,19 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
 
       /* Allow triplets with or without vendor set.  */
       triplet_rx = std::string (arch_rx) + "(-[^-]*)?-" + os_rx;
-
-      if (compiler->fe->ops->version >= GCC_FE_VERSION_1)
-	compiler->fe->ops->set_triplet_regexp (compiler->fe,
-					       triplet_rx.c_str ());
+      compiler->set_triplet_regexp (triplet_rx.c_str ());
     }
 
   /* Set compiler command-line arguments.  */
-  get_args (compiler, gdbarch, &argc, &argv);
+  get_args (compiler.get (), gdbarch, &argc, &argv);
   gdb_argv argv_holder (argv);
 
-  if (compiler->fe->ops->version >= GCC_FE_VERSION_1)
-    error_message = compiler->fe->ops->set_arguments (compiler->fe, argc, argv);
-  else
-    error_message = compiler->fe->ops->set_arguments_v0 (compiler->fe,
-							 triplet_rx.c_str (),
-							 argc, argv);
+  gdb::unique_xmalloc_ptr<char> error_message;
+  error_message.reset (compiler->set_arguments (argc, argv,
+						triplet_rx.c_str ()));
+
   if (error_message != NULL)
-    {
-      make_cleanup (xfree, error_message);
-      error ("%s", error_message);
-    }
+    error ("%s", error_message.get ());
 
   if (compile_debug)
     {
@@ -601,13 +579,8 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
 			fnames.source_file ());
 
   /* Call the compiler and start the compilation process.  */
-  compiler->fe->ops->set_source_file (compiler->fe, fnames.source_file ());
-
-  if (compiler->fe->ops->version >= GCC_FE_VERSION_1)
-    ok = compiler->fe->ops->compile (compiler->fe, fnames.object_file ());
-  else
-    ok = compiler->fe->ops->compile_v0 (compiler->fe, fnames.object_file (),
-					compile_debug);
+  compiler->set_source_file (fnames.source_file ());
+  ok = compiler->compile (fnames.object_file (), compile_debug);
   if (!ok)
     error (_("Compilation failed."));
 
@@ -617,9 +590,6 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
 
   /* Keep the source file.  */
   source_remover->keep ();
-
-  do_cleanups (cleanup);
-
   return fnames;
 }
 
@@ -691,6 +661,87 @@ compile_register_name_demangle (struct gdbarch *gdbarch,
   error (_("Cannot find gdbarch register \"%s\"."), regname);
 }
 
+/* Forwards to the plug-in.  */
+
+#define FORWARD(OP,...) (m_gcc_fe->ops->OP (m_gcc_fe, ##__VA_ARGS__))
+
+/* See compile-internal.h.  */
+
+void
+compile_instance::set_print_callback
+  (void (*print_function) (void *, const char *), void *datum)
+{
+  FORWARD (set_print_callback, print_function, datum);
+}
+
+/* See compile-internal.h.  */
+
+unsigned int
+compile_instance::version () const
+{
+  return m_gcc_fe->ops->version;
+}
+
+/* See compile-internal.h.  */
+
+void
+compile_instance::set_verbose (int level)
+{
+  if (version () >= GCC_FE_VERSION_1)
+    FORWARD (set_verbose, level);
+}
+
+/* See compile-internal.h.  */
+
+void
+compile_instance::set_driver_filename (const char *filename)
+{
+  if (version () >= GCC_FE_VERSION_1)
+    FORWARD (set_driver_filename, filename);
+}
+
+/* See compile-internal.h.  */
+
+void
+compile_instance::set_triplet_regexp (const char *regexp)
+{
+  if (version () >= GCC_FE_VERSION_1)
+    FORWARD (set_triplet_regexp, regexp);
+}
+
+/* See compile-internal.h.  */
+
+char *
+compile_instance::set_arguments (int argc, char **argv, const char *regexp)
+{
+  if (version () >= GCC_FE_VERSION_1)
+    return FORWARD (set_arguments, argc, argv);
+  else
+    return FORWARD (set_arguments_v0, regexp, argc, argv);
+}
+
+/* See compile-internal.h.  */
+
+void
+compile_instance::set_source_file (const char *filename)
+{
+  FORWARD (set_source_file, filename);
+}
+
+/* See compile-internal.h.  */
+
+bool
+compile_instance::compile (const char *filename, int verbose_level)
+{
+  if (version () >= GCC_FE_VERSION_1)
+    return FORWARD (compile, filename);
+  else
+    return FORWARD (compile_v0, filename, verbose_level);
+}
+
+#undef FORWARD
+
+
 void
 _initialize_compile (void)
 {
diff --git a/gdb/language.h b/gdb/language.h
index 029de4a7ab..662519bafd 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -36,7 +36,7 @@ struct value_print_options;
 struct type_print_options;
 struct lang_varobj_ops;
 struct parser_state;
-struct compile_instance;
+class compile_instance;
 struct completion_match_for_lcd;
 
 #define MAX_FORTRAN_DIMS  7	/* Maximum number of F77 array dims.  */
@@ -426,7 +426,7 @@ struct language_defn
        instance is owned by its caller and must be deallocated by
        calling its 'destroy' method.  */
 
-    struct compile_instance *(*la_get_compile_instance) (void);
+    compile_instance *(*la_get_compile_instance) (void);
 
     /* This method must be defined if 'la_get_gcc_context' is defined.
        If 'la_get_gcc_context' is not defined, then this method is
@@ -442,7 +442,7 @@ struct language_defn
        parsed.
        EXPR_PC is the PC at which the expression is being parsed.  */
 
-    std::string (*la_compute_program) (struct compile_instance *inst,
+    std::string (*la_compute_program) (compile_instance *inst,
 				       const char *input,
 				       struct gdbarch *gdbarch,
 				       const struct block *expr_block,
-- 
2.13.6

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/8] Move C-related declarations to compile-c.h
  2018-05-03 18:42 ` [PATCH 3/8] Move C-related declarations to compile-c.h Keith Seitz
@ 2018-06-06 14:34   ` Pedro Alves
  0 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2018-06-06 14:34 UTC (permalink / raw)
  To: Keith Seitz, gdb-patches

On 05/03/2018 07:41 PM, Keith Seitz wrote:
> @@ -1453,7 +1451,11 @@ HFILES_NO_SRCDIR = \
> +	compile/compile-c.h \
>  	compile/compile.h \

Sort the other way around.  See comment at top of file.

> +	compile/compile-internal.h \
> +	compile/compile-object-load.h \
> +	compile/compile-object-run.h \

These ones are correctly placed.

>  	config/nm-linux.h \
>  	config/nm-nto.h \
>  	config/djgpp/langinfo.h \
Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/8] Add a C++ wrapper for GCC C plug-in
  2018-05-03 18:49 ` [PATCH 4/8] Add a C++ wrapper for GCC C plug-in Keith Seitz
@ 2018-06-06 14:34   ` Pedro Alves
  2018-07-10 16:58     ` Keith Seitz
  0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2018-06-06 14:34 UTC (permalink / raw)
  To: Keith Seitz, gdb-patches

On 05/03/2018 07:41 PM, Keith Seitz wrote:
> This patch introduces a new class which wraps the GCC C compile plug-in.
> It is a little unfortunate that this all happened in between the time that
> GCC moved to C++ and GDB moved to C++, leaving us with an ABI promise to
> support a C-like interface.  

It's easier to keep an ABI promise in C anyway, so it's not uncommon
for C++ projects to expose a C plugin interface and then provide wrappers
for multiple languages.  That's how the GCC JIT interface is exported
for example.

> The hope is to isolate GDB from some of this
> should it change in the future.

Not so sure it would be a good idea to change it, IMHO.

> 
> Broadly, what this does is replace calls like:
> 
>   C_CTX (context)->c_ops->operation (C_CTX (context), ...);
> 
> with calls that now look like:
> 
>   context->c_plugin->operation (...);

This LGTM.

I do wonder whether c_plugin needs to be a pointer though?

Couldn't it be an object directly?  Like:

  context->c_plugin.operation (...);

I see that the next patch switches this to a method named
plugin(), and then I wonder if that method can return a reference
instead of a pointer (because IIUC, you can never have a
NULL plugin).

TBC, even if you agree, I don't think it's worth it to go
over the pain of to redoing the series because of this.
I'd be totally fine with changing it on top of the series.

> @@ -206,22 +197,20 @@ convert_enum (struct compile_c_instance *context, struct type *type)
>  {
>    gcc_type int_type, result;
>    int i;
> -  struct gcc_c_context *ctx = C_CTX (context);
> +  const gcc_c_plugin *plugin = context->c_plugin;
>  
> -  int_type = ctx->c_ops->int_type_v0 (ctx,
> -				      TYPE_UNSIGNED (type),
> -				      TYPE_LENGTH (type));
> +  int_type = plugin->int_type_v0 (TYPE_UNSIGNED (type),
> +					    TYPE_LENGTH (type));

Alignment looks odd here, but maybe it's just in the email client.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] Use std::unordered_map instead of htab_t.
  2018-05-03 18:49 ` [PATCH 6/8] Use std::unordered_map instead of htab_t Keith Seitz
@ 2018-06-06 14:35   ` Pedro Alves
  2018-07-10 17:05     ` Keith Seitz
  0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2018-06-06 14:35 UTC (permalink / raw)
  To: Keith Seitz, gdb-patches

On 05/03/2018 07:41 PM, Keith Seitz wrote:
> This patch simply removes the use of libiberty's hashtab facility in favor
> of C++'s unordered_map.

Do you know whether these hash tables end up with just a few entries, or
many entries?  I hope the open vs closed hashing, different hashing, and
increased memory usage aren't a real concern here.

> -
>  /* See compile-internal.h.  */
>  
>  void
>  compile_instance::insert_symbol_error (const struct symbol *sym,
> -				       const char *text)
> +				       std::string text)

Is this passing by value/copy on purpose?

>  {
> -  struct symbol_error e;
> -  void **slot;
> +  symbol_err_map_t::iterator pos = m_symbol_err_map.find (sym);
>  
> -  if (m_symbol_err_map == NULL)
> -    {
> -      m_symbol_err_map = htab_create_alloc (10,
> -					    hash_symbol_error,
> -					    eq_symbol_error,
> -					    del_symbol_error,
> -					    xcalloc,
> -					    xfree);
> -    }
> -
> -  e.sym = sym;
> -  slot = htab_find_slot (m_symbol_err_map, &e, INSERT);
> -  if (*slot == NULL)
> -    {
> -      struct symbol_error *e = XNEW (struct symbol_error);
> -
> -      e->sym = sym;
> -      e->message = xstrdup (text);
> -      *slot = e;
> -    }
> +  if (pos == m_symbol_err_map.end ())
> +    m_symbol_err_map.insert (std::make_pair (sym, text));

Move 'text' ?

>  }
>  
>  /* See compile-internal.h.  */
> @@ -115,20 +50,13 @@ compile_instance::insert_symbol_error (const struct symbol *sym,
>  void
>  compile_instance::error_symbol_once (const struct symbol *sym)
>  {
> -  struct symbol_error search;
> -  struct symbol_error *err;
> -
> -  if (m_symbol_err_map == NULL)
> -    return;
> -
> -  search.sym = sym;
> -  err = (struct symbol_error *) htab_find (m_symbol_err_map, &search);
> -  if (err == NULL || err->message == NULL)
> +  symbol_err_map_t::iterator pos = m_symbol_err_map.find (sym);
> +  if (pos == m_symbol_err_map.end () || pos->second.length () == 0)
>      return;
>  
> -  gdb::unique_xmalloc_ptr<char> message (err->message);
> -  err->message = NULL;
> -  error (_("%s"), message.get ());
> +  std::string message (pos->second);
> +  pos->second.clear ();

Move pos->second instead of copying?


> +  ::error (_("%s"), message.c_str ());
>  }
>  

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/8] Convert C compile to C++
  2018-05-03 18:42 [PATCH 0/8] Convert C compile to C++ Keith Seitz
                   ` (7 preceding siblings ...)
  2018-05-03 18:51 ` [PATCH 5/8] Change compile_instance/compile_c_instance into classes Keith Seitz
@ 2018-06-06 14:38 ` Pedro Alves
  2018-07-10 17:07   ` Keith Seitz
  8 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2018-06-06 14:38 UTC (permalink / raw)
  To: Keith Seitz, gdb-patches

On 05/03/2018 07:41 PM, Keith Seitz wrote:
> This patch series begins the move to C++ for the compile feature,
> specifically targeting C type conversion.  This work is the basis for ongoing
> C++ compile work, which I hope to submit in the not-too-distant future.

Looking forward to that.

> 
> The main drive of the patches is to convert compile_instance and
> compile_c_instance into classes, but there are several pre-cursor patches
> which attempt to reorganize the code a little bit.  Generic compile
> functionality is put into compile-internal.h/compile.c, and all other
> C-specific code is moved into compile-c-*.c/h files.

The series LGTM.  I sent a few comments, but really nothing serious.

I was a bit surprised by use policy templates in patch #8,
but if it works for you, then it's certainly fine with me.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/8] Add a C++ wrapper for GCC C plug-in
  2018-06-06 14:34   ` Pedro Alves
@ 2018-07-10 16:58     ` Keith Seitz
  0 siblings, 0 replies; 21+ messages in thread
From: Keith Seitz @ 2018-07-10 16:58 UTC (permalink / raw)
  To: gdb-patches

On 06/06/2018 07:34 AM, Pedro Alves wrote:
> 
> This LGTM.
> 
> I do wonder whether c_plugin needs to be a pointer though?
> 
> Couldn't it be an object directly?  Like:
> 
>   context->c_plugin.operation (...);
> 
> I see that the next patch switches this to a method named
> plugin(), and then I wonder if that method can return a reference
> instead of a pointer (because IIUC, you can never have a
> NULL plugin).

Yes, I agree. I've changed the plugin method to return a reference instead of a pointer.

> TBC, even if you agree, I don't think it's worth it to go
> over the pain of to redoing the series because of this.
> I'd be totally fine with changing it on top of the series.

It's a trivial change (-> to .), so I've just made the change.

Thanks,
Keith

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] Use std::unordered_map instead of htab_t.
  2018-06-06 14:35   ` Pedro Alves
@ 2018-07-10 17:05     ` Keith Seitz
  2018-07-11 11:21       ` Pedro Alves
  0 siblings, 1 reply; 21+ messages in thread
From: Keith Seitz @ 2018-07-10 17:05 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

On 06/06/2018 07:35 AM, Pedro Alves wrote:
> On 05/03/2018 07:41 PM, Keith Seitz wrote:
> Do you know whether these hash tables end up with just a few entries, or
> many entries?  I hope the open vs closed hashing, different hashing, and
> increased memory usage aren't a real concern here.

insert_symbol_error is an exceptional case. It is only ever used when there's been an exception "converting" a local variable. In fact, AFAICT, we have /no/ test suite coverage for this yet.

The type map is different, though. It is used as a cache (which is currently tossed every time a compile command runs). So I guess it is possible for the type conversion map to grow large. While I'm not particularly concerned with it at this time, if you'd like me to revert that to using an htab_up, just let me know. It's almost trivial.

> 
>> -
>>  /* See compile-internal.h.  */
>>  
>>  void
>>  compile_instance::insert_symbol_error (const struct symbol *sym,
>> -				       const char *text)
>> +				       std::string text)
> 
> Is this passing by value/copy on purpose?

Nope, it was an oversight/mistake. Since this method is only called from one place, I've changed this to accept a const char*.

>>  }
>>  
>>  /* See compile-internal.h.  */
>> @@ -115,20 +50,13 @@ compile_instance::insert_symbol_error (const struct symbol *sym,
>>  void
>>  compile_instance::error_symbol_once (const struct symbol *sym)
>>  {
>> -  struct symbol_error search;
>> -  struct symbol_error *err;
>> -
>> -  if (m_symbol_err_map == NULL)
>> -    return;
>> -
>> -  search.sym = sym;
>> -  err = (struct symbol_error *) htab_find (m_symbol_err_map, &search);
>> -  if (err == NULL || err->message == NULL)
>> +  symbol_err_map_t::iterator pos = m_symbol_err_map.find (sym);
>> +  if (pos == m_symbol_err_map.end () || pos->second.length () == 0)
>>      return;
>>  
>> -  gdb::unique_xmalloc_ptr<char> message (err->message);
>> -  err->message = NULL;
>> -  error (_("%s"), message.get ());
>> +  std::string message (pos->second);
>> +  pos->second.clear ();
> 
> Move pos->second instead of copying?

Yes, indeed. Fixed.

Thank you,
Keith

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/8] Convert C compile to C++
  2018-06-06 14:38 ` [PATCH 0/8] Convert C compile to C++ Pedro Alves
@ 2018-07-10 17:07   ` Keith Seitz
  2018-07-11 11:31     ` Pedro Alves
  0 siblings, 1 reply; 21+ messages in thread
From: Keith Seitz @ 2018-07-10 17:07 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

On 06/06/2018 07:38 AM, Pedro Alves wrote:
> On 05/03/2018 07:41 PM, Keith Seitz wrote:
>> This patch series begins the move to C++ for the compile feature,
>> specifically targeting C type conversion.  This work is the basis for ongoing
>> C++ compile work, which I hope to submit in the not-too-distant future.
> 
> Looking forward to that.

Me, too. I've hoarded this for far too long!

> The series LGTM.  I sent a few comments, but really nothing serious.
> 
> I was a bit surprised by use policy templates in patch #8,
> but if it works for you, then it's certainly fine with me.

If you have a better/more maintainable/easier to read/understand alternative, I'm all ears.

Keith

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] Use std::unordered_map instead of htab_t.
  2018-07-10 17:05     ` Keith Seitz
@ 2018-07-11 11:21       ` Pedro Alves
  2018-08-07 14:57         ` Keith Seitz
  0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2018-07-11 11:21 UTC (permalink / raw)
  To: Keith Seitz, gdb-patches

On 07/10/2018 06:05 PM, Keith Seitz wrote:
> On 06/06/2018 07:35 AM, Pedro Alves wrote:
>> On 05/03/2018 07:41 PM, Keith Seitz wrote:
>> Do you know whether these hash tables end up with just a few entries, or
>> many entries?  I hope the open vs closed hashing, different hashing, and
>> increased memory usage aren't a real concern here.
> 
> insert_symbol_error is an exceptional case. It is only ever used when there's been an exception "converting" a local variable. In fact, AFAICT, we have /no/ test suite coverage for this yet.
> 
> The type map is different, though. It is used as a cache (which is currently tossed every time a compile command runs). So I guess it is possible for the type conversion map to grow large. While I'm not particularly concerned with it at this time, if you'd like me to revert that to using an htab_up, just let me know. It's almost trivial.

Yeah, it's not whether or not I _want_ you to revert.  Instead, data rules.
I really have no awareness of whether that map ends up with a hundred entries
or a hundred million entries, and thus whether the difference ends up
significant.  So if after considering that you're not concerned, then
neither am I.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/8] Convert C compile to C++
  2018-07-10 17:07   ` Keith Seitz
@ 2018-07-11 11:31     ` Pedro Alves
  0 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2018-07-11 11:31 UTC (permalink / raw)
  To: Keith Seitz, gdb-patches

On 07/10/2018 06:07 PM, Keith Seitz wrote:
> On 06/06/2018 07:38 AM, Pedro Alves wrote:
>> On 05/03/2018 07:41 PM, Keith Seitz wrote:
>>> This patch series begins the move to C++ for the compile feature,
>>> specifically targeting C type conversion.  This work is the basis for ongoing
>>> C++ compile work, which I hope to submit in the not-too-distant future.
>>
>> Looking forward to that.
> 
> Me, too. I've hoarded this for far too long!
> 
>> The series LGTM.  I sent a few comments, but really nothing serious.
>>
>> I was a bit surprised by use policy templates in patch #8,
>> but if it works for you, then it's certainly fine with me.
> 
> If you have a better/more maintainable/easier to read/understand alternative, I'm all ears.

Nah, it's not really possible to suggest something else before
seeing the corresponding C++ bits.  I guess templates makes sense
assuming the C++ bits add new plugin types unrelated to the C
plugin, etc.  I trust you on that, I was just surprised to see it.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] Use std::unordered_map instead of htab_t.
  2018-07-11 11:21       ` Pedro Alves
@ 2018-08-07 14:57         ` Keith Seitz
  2018-08-08 12:29           ` Pedro Alves
  0 siblings, 1 reply; 21+ messages in thread
From: Keith Seitz @ 2018-08-07 14:57 UTC (permalink / raw)
  To: gdb-patches

On 07/11/2018 04:21 AM, Pedro Alves wrote:
> Yeah, it's not whether or not I _want_ you to revert.  Instead, data rules.

Ha ha. Indeed, it (data) does rule. I apologize for being so vague...

> I really have no awareness of whether that map ends up with a hundred entries
> or a hundred million entries, and thus whether the difference ends up
> significant.  So if after considering that you're not concerned, then
> neither am I.

I've revisited this topic a bit on the my C++ compile branch, and
there is sufficient evidence there to suggest that the type cache
could grow quite large -- especially for non-trivial compilations such
as compiling a file (or perhaps in the future if/when we support code
replacement or other advanced uses of this feature.

IIRC, I originally wrote this before the move to C++11 had been
proposed. Consequently unique_ptr was not available. Of course, it is
now, and it makes sense to me to move to using htab_up instead.

[The underlying storage for the error map doesn't really matter. That
map seldom has any entries in it.]

WDYT of the below patch?

Keith

Subject: [PATCH] Use unique_ptr for htabs

This patch updates the type-conversion caching in C compile to use
unique pointers.  This patch also removes the on-demand allocation of the
symbol error map in favor of initialization, simplifying the code.


gdb/ChangeLog
YYYY-MM-DD  Keith Seitz  <keiths@redhat.com>

	* compile/compile-internal.h (compile_instance::~compile_instance):
	Remove calls to htab_delete.
	<m_type_map, m_symbol_err_map>: Switch type to htab_up.
	* compile.c (compile_instance::compile_instance): Initialize
	htab unique pointers.
	(compile_instance::get_cached_type, compile_instance::insert_type)
	(compile_instance::error_symbol_once): Update for unique_ptr.
---
 gdb/compile/compile-internal.h |  7 ++-----
 gdb/compile/compile.c          | 28 ++++++++++------------------
 2 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/gdb/compile/compile-internal.h b/gdb/compile/compile-internal.h
index 3916f84f6f..c8d2d2f427 100644
--- a/gdb/compile/compile-internal.h
+++ b/gdb/compile/compile-internal.h
@@ -49,9 +49,6 @@ public:
   virtual ~compile_instance ()
   {
     m_gcc_fe->ops->destroy (m_gcc_fe);
-    htab_delete (m_type_map);
-    if (m_symbol_err_map != NULL)
-      htab_delete (m_symbol_err_map);
   }
 
   /* Returns the GCC options to be passed during compilation.  */
@@ -148,10 +145,10 @@ protected:
   std::string m_gcc_target_options;
 
   /* Map from gdb types to gcc types.  */
-  htab_t m_type_map;
+  htab_up m_type_map;
 
   /* Map from gdb symbols to gcc error messages to emit.  */
-  htab_t m_symbol_err_map;
+  htab_up m_symbol_err_map;
 };
 
 /* Define header and footers for different scopes.  */
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 308c82ee54..31ae5970b9 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -129,11 +129,13 @@ del_symbol_error (void *a)
 compile_instance::compile_instance (struct gcc_base_context *gcc_fe,
 				    const char *options)
   : m_gcc_fe (gcc_fe), m_gcc_target_options (options),
-    m_symbol_err_map (NULL)
+    m_type_map (htab_create_alloc (10, hash_type_map_instance,
+				   eq_type_map_instance,
+				   xfree, xcalloc, xfree)),
+    m_symbol_err_map (htab_create_alloc (10, hash_symbol_error,
+					 eq_symbol_error, del_symbol_error,
+					 xcalloc, xfree))
 {
-  m_type_map = htab_create_alloc (10, hash_type_map_instance,
-				  eq_type_map_instance,
-				  xfree, xcalloc, xfree);
 }
 
 /* See compile-internal.h.  */
@@ -144,7 +146,7 @@ compile_instance::get_cached_type (struct type *type, gcc_type &ret) const
   struct type_map_instance inst, *found;
 
   inst.type = type;
-  found = (struct type_map_instance *) htab_find (m_type_map, &inst);
+  found = (struct type_map_instance *) htab_find (m_type_map.get (), &inst);
   if (found != NULL)
     {
       ret = found->gcc_type_handle;
@@ -164,7 +166,7 @@ compile_instance::insert_type (struct type *type, gcc_type gcc_type)
 
   inst.type = type;
   inst.gcc_type_handle = gcc_type;
-  slot = htab_find_slot (m_type_map, &inst, INSERT);
+  slot = htab_find_slot (m_type_map.get (), &inst, INSERT);
 
   add = (struct type_map_instance *) *slot;
   /* The type might have already been inserted in order to handle
@@ -189,18 +191,8 @@ compile_instance::insert_symbol_error (const struct symbol *sym,
   struct symbol_error e;
   void **slot;
 
-  if (m_symbol_err_map == NULL)
-    {
-      m_symbol_err_map = htab_create_alloc (10,
-					    hash_symbol_error,
-					    eq_symbol_error,
-					    del_symbol_error,
-					    xcalloc,
-					    xfree);
-    }
-
   e.sym = sym;
-  slot = htab_find_slot (m_symbol_err_map, &e, INSERT);
+  slot = htab_find_slot (m_symbol_err_map.get (), &e, INSERT);
   if (*slot == NULL)
     {
       struct symbol_error *e = XNEW (struct symbol_error);
@@ -223,7 +215,7 @@ compile_instance::error_symbol_once (const struct symbol *sym)
     return;
 
   search.sym = sym;
-  err = (struct symbol_error *) htab_find (m_symbol_err_map, &search);
+  err = (struct symbol_error *) htab_find (m_symbol_err_map.get (), &search);
   if (err == NULL || err->message == NULL)
     return;
 
-- 
2.13.6

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] Use std::unordered_map instead of htab_t.
  2018-08-07 14:57         ` Keith Seitz
@ 2018-08-08 12:29           ` Pedro Alves
  2018-08-10 18:18             ` Keith Seitz
  0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2018-08-08 12:29 UTC (permalink / raw)
  To: Keith Seitz, gdb-patches

On 08/07/2018 03:57 PM, Keith Seitz wrote:

> WDYT of the below patch?
> 
LGTM.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] Use std::unordered_map instead of htab_t.
  2018-08-08 12:29           ` Pedro Alves
@ 2018-08-10 18:18             ` Keith Seitz
  0 siblings, 0 replies; 21+ messages in thread
From: Keith Seitz @ 2018-08-10 18:18 UTC (permalink / raw)
  To: gdb-patches

On 08/08/2018 05:28 AM, Pedro Alves wrote:
> On 08/07/2018 03:57 PM, Keith Seitz wrote:
> 
>> WDYT of the below patch?
>>
> LGTM.

And with that, I've pushed the entire series.

Thank you for reviewing and your advice!

Keith

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2018-08-10 18:18 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-03 18:42 [PATCH 0/8] Convert C compile to C++ Keith Seitz
2018-05-03 18:41 ` [PATCH 2/8] Rename symbol_substitution_name Keith Seitz
2018-05-03 18:42 ` [PATCH 1/8] Return unique_xmalloc_ptr for generate_c_for_variable_locations Keith Seitz
2018-05-03 18:42 ` [PATCH 3/8] Move C-related declarations to compile-c.h Keith Seitz
2018-06-06 14:34   ` Pedro Alves
2018-05-03 18:49 ` [PATCH 4/8] Add a C++ wrapper for GCC C plug-in Keith Seitz
2018-06-06 14:34   ` Pedro Alves
2018-07-10 16:58     ` Keith Seitz
2018-05-03 18:49 ` [PATCH 6/8] Use std::unordered_map instead of htab_t Keith Seitz
2018-06-06 14:35   ` Pedro Alves
2018-07-10 17:05     ` Keith Seitz
2018-07-11 11:21       ` Pedro Alves
2018-08-07 14:57         ` Keith Seitz
2018-08-08 12:29           ` Pedro Alves
2018-08-10 18:18             ` Keith Seitz
2018-05-03 18:50 ` [PATCH 8/8] Use policies for code generation Keith Seitz
2018-05-03 18:50 ` [PATCH 7/8] Move compile_instance to compile.c Keith Seitz
2018-05-03 18:51 ` [PATCH 5/8] Change compile_instance/compile_c_instance into classes Keith Seitz
2018-06-06 14:38 ` [PATCH 0/8] Convert C compile to C++ Pedro Alves
2018-07-10 17:07   ` Keith Seitz
2018-07-11 11:31     ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).