Re: [PATCH] libgccjit: add some reflection functions in the jit C api

[Antoni Boucher <bouanto@zoho.com>]

Le vendredi 11 juin 2021 à 14:00 -0400, David Malcolm a écrit :
> On Fri, 2021-06-11 at 08:15 -0400, Antoni Boucher wrote:
> > Thank you for your answer.
> > I attached the updated patch.
> 
> BTW you (or possibly me) dropped the mailing lists; was that
> deliberate?

Oh, my bad.

> > 
> > See my answers below.
> > 
> > Le jeudi 10 juin 2021 à 18:41 -0400, David Malcolm a écrit :
> > > On Thu, 2021-05-27 at 21:51 -0400, Antoni Boucher wrote:
> > > > I chose option A, so everything is a size_t, now.
> > > > I also renamed the dyncast functions.
> > > > Here's the new patch.
> > > 
> > > Thanks, sorry again about the delays in reviewing your work.
> > > 
> > > You didn't specify how you tested the patch; are you running the
> > > full
> > > jit regression test suite?
> > 
> > Yes, I run the full jit test suite.
> 
> Great.
> 
> [...snip...]
> 
> > > 
> > > I can't remember, sorry, do you have push rights to the gcc git
> > > repository?
> > 
> > I don't have push rights; it's actually my first contribution to
> > gcc.
> 
> Congratulations!  I'm excited about having Rust support for GCC (two
> different ways, in fact!)

Yup. Exciting times!

> 
> > I have signed the FSF copyright attribution.
> 
> I can push changes on your behalf, but I'd prefer it if you did it,
> especially given that you have various other patches you want to get
> in.
> 
> Instructions on how to get push rights to the git repo are here:
>   https://gcc.gnu.org/gitwrite.html
> 
> I can sponsor you.

Thanks.
I did sign up to get push rights.
Have you accepted my request to get those?

> 
> Note that your patches would still need review/approval ("Write After
> Approval").
> 
> > > Do you have a preference as to which patch you want me to look at
> > > next?
> > > Otherwise I'll go through them in the order in
> > > https://github.com/antoyo/rustc_codegen_gcc/tree/master/gcc-patches
> > 
> > You can indeed look in this directory in the order they were added.
> > There was one patch before the reflection one (0001-This-patch-
> > handles-
> > truncation-and-extension-for-cast.patch), but since it's only a
> > bugfix,
> > it doesn't matter if it's merged after.
> 
> That one looks good to me now too (I've replied on that patch).
> 
> > 
> > This one is ready for review, but I believe the other one needs
> > correction on my end since the last review you made. I'll make sure
> > to
> > fix them soon.
> 
> Now I'm confused as to which patches you're referring to, sorry. 
> I'll
> hold off for now on further reviews; let me know when you want me to
> look at them, and which ones.
> 
> Thanks
> Dave
>