On Mon, 2024-04-22 at 19:56 +0200, Guillaume Gomez wrote: > `param` is also inheriting from `lvalue`. I don't think adding this > check is a good idea > because it will not evolve nicely if more changes are done in > libgccjit.
Sorry for not responding earlier. I think I agree with Guillaume here. Looking at the checklist at: https://gcc.gnu.org/onlinedocs/jit/internals/index.html#submitting-patches the patch is missing: - a feature macro in libgccjit.h such as #define LIBGCCJIT_HAVE_gcc_jit_lvalue_get_name - documentation for the new C entrypoint - documentation for the new ABI tag (see topics/compatibility.rst). Other than that, the patch looks reasonable to me. Dave > > Le lun. 22 avr. 2024 à 17:19, Antoni Boucher <boua...@zoho.com> a > écrit : > > > > For your new API endpoint, please add a check like: > > > > RETURN_IF_FAIL (lvalue->is_global () || lvalue->is_local (), > > NULL, > > NULL, > > "lvalue should be a variable"); > > > > > > Le 2024-04-22 à 09 h 16, Guillaume Gomez a écrit : > > > Good point! > > > > > > New patch attached. > > > > > > Le lun. 22 avr. 2024 à 15:13, Antoni Boucher <boua...@zoho.com> a > > > écrit : > > > > > > > > Please move the function to be on lvalue since there are no > > > > rvalue types > > > > that are not lvalues that have a name. > > > > > > > > Le 2024-04-22 à 09 h 04, Guillaume Gomez a écrit : > > > > > Hey Arthur :) > > > > > > > > > > > Is there any reason for that getter to return a mutable > > > > > > pointer to the > > > > > > name? Would something like this work instead if you're just > > > > > > looking at > > > > > > getting the name? > > > > > > > > > > > > + virtual string * get_name () const { return NULL; } > > > > > > > > > > > > With of course adequate modifications to the inheriting > > > > > > classes. > > > > > > > > > > Good catch, thanks! > > > > > > > > > > Updated the patch and attached the new version to this email. > > > > > > > > > > Cordially. > > > > > > > > > > Le lun. 22 avr. 2024 à 11:51, Arthur Cohen > > > > > <arthur.co...@embecosm.com> a écrit : > > > > > > > > > > > > Hey Guillaume :) > > > > > > > > > > > > On 4/20/24 01:05, Guillaume Gomez wrote: > > > > > > > Hi, > > > > > > > > > > > > > > I just encountered the need to retrieve the name of an > > > > > > > `rvalue` (if > > > > > > > there is one) while working on the Rust GCC backend. > > > > > > > > > > > > > > This patch adds a getter to retrieve the information. > > > > > > > > > > > > > > Cordially. > > > > > > > > > > > > > virtual bool get_wide_int (wide_int *) const { > > > > > > > return false; } > > > > > > > > > > > > > > + virtual string * get_name () { return NULL; } > > > > > > > + > > > > > > > private: > > > > > > > virtual enum precedence get_precedence () const = 0; > > > > > > > > > > > > Is there any reason for that getter to return a mutable > > > > > > pointer to the > > > > > > name? Would something like this work instead if you're just > > > > > > looking at > > > > > > getting the name? > > > > > > > > > > > > + virtual string * get_name () const { return NULL; } > > > > > > > > > > > > With of course adequate modifications to the inheriting > > > > > > classes. > > > > > > > > > > > > Best, > > > > > > > > > > > > Arthur >
