Hi. I added all the functions I need in this new patch. Please tell me if that looks good and I'll add documentation for those functions. Thanks. On Fri, Oct 02, 2020 at 04:24:26PM -0400, David Malcolm wrote: >On Fri, 2020-10-02 at 16:17 -0400, David Malcolm wrote: >> 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. > > >Also, please add "PR jit/96889" to the ChangeLog entries, and [PR96889] >to the subject line. > >Dave >