small C++ API additions

small C++ API additions

[Joseph Jevnik]

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

Hello all,

I have been playing around with the C++ wrapper for gccjit and have noticed a
few things missing from the C API. I have written up four small patches to add
the missing features.

The patches include the following changes:

new-function-ptr-type.patch: Adds a method on 'context' to create a new function
pointer type.

new-union-type.patch: Adds a method on 'context' for creating a new union
type. This also adds a 'union_' class like 'struct_' which subclasses 'type'.

set_fields.patch: Adds a method to 'struct_' for setting the fields of an opaque
struct type. The C++ wrapper already allowed creation of opaque structs but
didn't have a way to populate them for creating recusive structures like linked
lists.

operator-call.patch: Adds a variadic dispatch for 'function::operator()' when
compiling with C++11 support. This extends 'function(a, b, c, ...) for arbitrary
amounts of arguments without needing to create a vector to pass the arguments
in. Right now this just dispatches through the vector case but could be changed
to use a 'std::array<rvalue, sizeof...(Args)>' in the future. This is mostly
just to simplify the API for fixed arity calls which have a lot of arguments.

Currently all of these patches are missing tests. I found the C tests but didn't
see anything for the C++ API. I would love to add some cases for these but I
just don't know where to look.

This is my first time posting to any of the GCC mailing lists or working on GCC
at all so please let me know if this is not the correct place to post this or if
I have done something incorrectly.

Thank you for your time!

[-- Attachment #2: new-function-ptr-type.patch --]
[-- Type: text/x-patch, Size: 1654 bytes --]

diff --git a/gcc/jit/libgccjit++.h b/gcc/jit/libgccjit++.h
index 8dc2112367b..a6d531dc697 100644
--- a/gcc/jit/libgccjit++.h
+++ b/gcc/jit/libgccjit++.h
@@ -169,6 +169,11 @@ namespace gccjit
 			   int is_variadic,
 			   location loc = location ());
 
+   type new_function_ptr_type (type return_type,
+                               std::vector<type> &params,
+                               int is_variadic,
+                               location loc = location ());
+
     function get_builtin_function (const std::string &name);
 
     lvalue new_global (enum gcc_jit_global_kind kind,
@@ -810,6 +815,29 @@ context::new_function (enum gcc_jit_function_kind kind,
 						 is_variadic));
 }
 
+inline type
+context::new_function_ptr_type (type return_type,
+                                std::vector<type> &params,
+                                int is_variadic,
+                                location loc)
+{
+  /* Treat std::vector as an array, relying on it not being resized: */
+  type *as_array_of_wrappers = params.data();
+
+  /* Treat the array as being of the underlying pointers, relying on
+     the wrapper type being such a pointer internally.	*/
+  gcc_jit_type **as_array_of_ptrs =
+    reinterpret_cast<gcc_jit_type **> (as_array_of_wrappers);
+
+  return type (gcc_jit_context_new_function_ptr_type (
+                   m_inner_ctxt,
+                   loc.get_inner_location (),
+                   return_type.get_inner_type (),
+                   params.size (),
+                   as_array_of_ptrs,
+                   is_variadic));
+}
+
 inline function
 context::get_builtin_function (const std::string &name)
 {

[-- Attachment #3: new-union-type.patch --]
[-- Type: text/x-patch, Size: 2423 bytes --]

diff --git a/gcc/jit/libgccjit++.h b/gcc/jit/libgccjit++.h
index 8dc2112367b..5a7bf8e2c43 100644
--- a/gcc/jit/libgccjit++.h
+++ b/gcc/jit/libgccjit++.h
@@ -40,6 +40,7 @@ namespace gccjit
     class field;
     class type;
       class struct_;
+      class union_;
     class function;
     class block;
     class rvalue;
@@ -158,6 +159,10 @@ namespace gccjit
     struct_ new_opaque_struct_type (const std::string &name,
 				    location loc = location ());
 
+    union_ new_union_type (const std::string &name,
+                           std::vector<field> &fields,
+                           location loc = location ());
+
     param new_param (type type_,
 		     const std::string &name,
 		     location loc = location ());
@@ -346,6 +351,16 @@ namespace gccjit
     gcc_jit_struct *get_inner_struct () const;
   };
 
+  class union_ : public type
+  {
+  public:
+    union_ ();
+    union_ (gcc_jit_type *inner);
+
+    gcc_jit_type *get_inner_union () const;
+  };
+
+
   class function : public object
   {
   public:
@@ -773,6 +788,26 @@ context::new_opaque_struct_type (const std::string &name,
 		    name.c_str ()));
 }
 
+inline union_
+context::new_union_type (const std::string &name,
+			  std::vector<field> &fields,
+			  location loc)
+{
+  /* Treat std::vector as an array, relying on it not being resized: */
+  field *as_array_of_wrappers = &fields[0];
+
+  /* Treat the array as being of the underlying pointers, relying on
+     the wrapper type being such a pointer internally.	*/
+  gcc_jit_field **as_array_of_ptrs =
+    reinterpret_cast<gcc_jit_field **> (as_array_of_wrappers);
+
+  return union_ (gcc_jit_context_new_union_type (m_inner_ctxt,
+                                                 loc.get_inner_location (),
+                                                 name.c_str (),
+                                                 fields.size (),
+                                                 as_array_of_ptrs));
+}
+
 inline param
 context::new_param (type type_,
 		    const std::string &name,
@@ -1317,6 +1352,13 @@ struct_::get_inner_struct () const
   return reinterpret_cast<gcc_jit_struct *> (get_inner_object ());
 }
 
+// class union_
+inline union_::union_ () : type (NULL) {}
+inline union_::union_ (gcc_jit_type *inner) :
+  type (inner)
+{
+}
+
 // class function
 inline function::function () : object () {}
 inline function::function (gcc_jit_function *inner)

[-- Attachment #4: operator-call.patch --]
[-- Type: text/x-patch, Size: 1783 bytes --]

diff --git a/gcc/jit/libgccjit++.h b/gcc/jit/libgccjit++.h
index 8dc2112367b..27031368890 100644
--- a/gcc/jit/libgccjit++.h
+++ b/gcc/jit/libgccjit++.h
@@ -296,6 +296,11 @@ namespace gccjit
 		     rvalue arg3, rvalue arg4, rvalue arg5,
 		     location loc = location ());
 
+#if __cplusplus >= 201100L
+    template<typename... Args>
+    rvalue new_call(function func, Args... args, location loc = location ());
+#endif
+
     rvalue new_cast (rvalue expr,
 		     type type_,
 		     location loc = location ());
@@ -376,6 +381,11 @@ namespace gccjit
 		       location loc = location ());
     rvalue operator() (rvalue arg0, rvalue arg1, rvalue arg2,
 		       location loc = location ());
+
+#if __cplusplus >= 201100L
+    template<typename... Args>
+    rvalue operator() (Args..., location loc = location ());
+#endif
   };
 
   class block : public object
@@ -1175,6 +1185,18 @@ context::new_call (function func,
   return new_call (func, args, loc);
 }
 
+#if __cplusplus >= 201100L
+template<typename... Args>
+inline rvalue
+context::new_call (function func,
+                   Args... args,
+                   location loc)
+{
+  std::vector<rvalue> vector_args = {args...};
+  return new_call (func, vector_args, loc);
+}
+#endif
+
 inline rvalue
 context::new_cast (rvalue expr,
 		   type type_,
@@ -1552,6 +1574,17 @@ function::operator() (rvalue arg0, rvalue arg1, rvalue arg2,
 				  loc);
 }
 
+#if __cplusplus >= 201100L
+template<typename... Args>
+inline rvalue
+function::operator() (Args... args, location loc)
+{
+  return get_context ().new_call (*this,
+                                  args...,
+                                  loc);
+}
+#endif
+
 // class block
 inline block::block () : object () {}
 inline block::block (gcc_jit_block *inner)

[-- Attachment #5: set_fields.patch --]
[-- Type: text/x-patch, Size: 1115 bytes --]

diff --git a/gcc/jit/libgccjit++.h b/gcc/jit/libgccjit++.h
index 8dc2112367b..14d685c4fa7 100644
--- a/gcc/jit/libgccjit++.h
+++ b/gcc/jit/libgccjit++.h
@@ -344,6 +344,8 @@ namespace gccjit
     struct_ (gcc_jit_struct *inner);
 
     gcc_jit_struct *get_inner_struct () const;
+
+    void set_fields(std::vector<field> &fields, location loc = location ());
   };
 
   class function : public object
@@ -1317,6 +1319,18 @@ struct_::get_inner_struct () const
   return reinterpret_cast<gcc_jit_struct *> (get_inner_object ());
 }
 
+inline void
+struct_::set_fields (std::vector<field> &fields, location loc)
+{
+  field *as_array_of_wrappers = fields.data();
+  gcc_jit_field **as_array_of_ptrs =
+      reinterpret_cast<gcc_jit_field**> (as_array_of_wrappers);
+  return gcc_jit_struct_set_fields (get_inner_struct (),
+                                    loc.get_inner_location (),
+                                    fields.size (),
+                                    as_array_of_ptrs);
+}
+
 // class function
 inline function::function () : object () {}
 inline function::function (gcc_jit_function *inner)

Re: small C++ API additions

[Joseph Jevnik]

Thanks for the quick response! I'll work on getting the tests added and change
the C++ version macro and post the updated patches this weekend.

I have already signed the contributor agreement. I work across the street from
the FSF so I walked over and did it in person.

Re: small C++ API additions

[David Malcolm]

On Wed, 2017-03-29 at 17:10 -0400, Joseph Jevnik wrote:
> Hello all,
> 
> I have been playing around with the C++ wrapper for gccjit and have
> noticed a
> few things missing from the C API. I have written up four small
> patches to add
> the missing features.
> new-function-ptr-type.patch: Adds a method on 'context' to create a
> new function
> pointer type.
> 
> new-union-type.patch: Adds a method on 'context' for creating a new
> union
> type. This also adds a 'union_' class like 'struct_' which subclasses
> 'type'.
> 
> set_fields.patch: Adds a method to 'struct_' for setting the fields
> of an opaque
> struct type. The C++ wrapper already allowed creation of opaque
> structs but
> didn't have a way to populate them for creating recusive structures
> like linked
> lists.
> 
> operator-call.patch: Adds a variadic dispatch for
> 'function::operator()' when
> compiling with C++11 support. This extends 'function(a, b, c, ...)
> for arbitrary
> amounts of arguments without needing to create a vector to pass the
> arguments
> in. Right now this just dispatches through the vector case but could
> be changed
> to use a 'std::array<rvalue, sizeof...(Args)>' in the future. This is
> mostly
> just to simplify the API for fixed arity calls which have a lot of
> arguments.

Thanks!

BTW, a nit: isn't the official value of __cplusplus for C++11 201103L,
hence:

#if __cplusplus >= 201103L

rather than:

#if __cplusplus >= 201100L

for the parameter packs?

I wonder if we need to generalize the C++ test suite so that it tests
the API for various different versions of the C++ standard.

> Currently all of these patches are missing tests. I found the C tests
> but didn't
> see anything for the C++ API. I would love to add some cases for
> these but I
> just don't know where to look.

Have a look at gcc/testsuite/jit.dg/*.cc

You can run individual test cases via (e.g.):

  make check-jit RUNTESTFLAGS="-v -v jit.exp=test-quadratic.cc"

(see https://dmalcolm.fedorapeople.org/gcc/newbies-guide/working-with-t
he-testsuite.html )

> This is my first time posting to any of the GCC mailing lists or
> working on GCC
> at all so please let me know if this is not the correct place to post
> this or if
> I have done something incorrectly.

This is the correct mailing list, and an excellent start; thanks.

Note that currently we're deep in feature freeze for gcc 7.  That said,
the C++ API to libgccjit is explicitly called out as having less
stability guarantees than the rest of the project, so we may be able to
violate that feature freeze.

Please can you add tests and documentation and post the updated patches
as separate emails (it's easier to deal with that way).

For sufficiently large contributions, you need to either file a
copyright assignment to the FSF, or to make a copyright disclaimer. 
 See the instructions here:
  https://gcc.gnu.org/contribute.html#legal
I'm not sure exactly what the threshold is for a contribution to be
called "large", but I suspect that you've reached that threshold
(especially after tests and docs are added to the patches).

> Thank you for your time!

Thanks again
Dave