On Tue, 2020-09-01 at 21:01 -0400, Antoni Boucher via Jit wrote: > Hello. > This WIP patch implements new reflection functions in the C API as > mentioned in bug 96889. > I'm looking forward for feedbacks on this patch. > It's WIP because I'll probably add a few more reflection functions. > Thanks.
Sorry about the belated review, looks like I missed this one. At a high level, it seems reasonable. Do you have a copyright assignment in place for GCC contributions? See https://gcc.gnu.org/contribute.html [...] > diff --git a/gcc/jit/docs/topics/compatibility.rst > b/gcc/jit/docs/topics/compatibility.rst > index bb3387fa583..7e786194ded 100644 > --- a/gcc/jit/docs/topics/compatibility.rst > +++ b/gcc/jit/docs/topics/compatibility.rst > @@ -219,3 +219,14 @@ entrypoints: > * :func:`gcc_jit_version_minor` > > * :func:`gcc_jit_version_patchlevel` > + > +.. _LIBGCCJIT_ABI_14: > + > +``LIBGCCJIT_ABI_14`` > +-------------------- > +``LIBGCCJIT_ABI_14`` covers the addition of reflection functions via API > +entrypoints: > + > + * :func:`gcc_jit_function_get_return_type` > + > + * :func:`gcc_jit_function_get_param_count` This will now need bumping to 15; 14 covers the addition of gcc_jit_global_set_initializer. [...] > +/* Public entrypoint. See description in libgccjit.h. > + > + After error-checking, the real work is done by the > + gcc::jit::recording::function::get_return_type method, in > + jit-recording.h. */ > + > +gcc_jit_type * > +gcc_jit_function_get_return_type (gcc_jit_function *func) > +{ This one is missing a: RETURN_NULL_IF_FAIL (func, NULL, NULL, "NULL function"); > + return (gcc_jit_type *)func->get_return_type (); > +} [...] > diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h > index 1c5a12e9c01..6999ce25ca2 100644 > --- a/gcc/jit/libgccjit.h > +++ b/gcc/jit/libgccjit.h [...] > @@ -1503,6 +1511,22 @@ gcc_jit_version_minor (void); > extern int > gcc_jit_version_patchlevel (void); > > +#define LIBGCCJIT_HAVE_gcc_jit_function_reflection > + > +/* Reflection functions to get the number of parameters and return types of > + a function from the C API. "return type", is better grammar, I think, given that "function" is singular. > + > + "vec_type" should be a vector type, created using gcc_jit_type_get_vector. This line about "vec_type" seems to be a leftover from a copy&paste. > + This API entrypoint was added in LIBGCCJIT_ABI_14; you can test for its > + presence using > + #ifdef LIBGCCJIT_HAVE_gcc_jit_function_reflection Version number will need bumping, as mentioned above. [...] > diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map > index 6137dd4b4b0..b28f81a7a32 100644 > --- a/gcc/jit/libgccjit.map > +++ b/gcc/jit/libgccjit.map > @@ -186,4 +186,10 @@ LIBGCCJIT_ABI_13 { > gcc_jit_version_major; > gcc_jit_version_minor; > gcc_jit_version_patchlevel; > -} LIBGCCJIT_ABI_12; > \ No newline at end of file > +} LIBGCCJIT_ABI_12; > + > +LIBGCCJIT_ABI_14 { > + global: > + gcc_jit_function_get_return_type; > + gcc_jit_function_get_param_count; > +} LIBGCCJIT_ABI_13; Likewise. [...] Otherwise looks good to me. Bonus points for adding C++ bindings (and docs for them), but I don't know of anyone using the C++ bindings. Dave
