Hi!
Thanks for the review. Having proper grammar in user facing text is important
so
I in no way feel it is overzealous. I'll push the patch once I fixed the points
mentioned
in the review.
Regards, Petter
Från: David Malcolm <dmalc...@redhat.com>
Skickat: den 13 december 2021 20:22
Till: Antoni Boucher; Petter Tomner; gcc-patches@gcc.gnu.org; j...@gcc.gnu.org
Ämne: Re: SV: [PATCH v2] jit: Add support for global rvalue initialization and
ctors
On Sun, 2021-12-12 at 20:39 -0500, Antoni Boucher wrote:
> Yes, this patch works for rustc_codegen_gcc perfectly.
> It even fixes one issue that was in my patch, so that's nice!
Excellent - thanks Antoni.
>
> Le samedi 11 décembre 2021 à 15:35 +0000, Petter Tomner a écrit :
> > Hi!
> >
> > > s/an union/a union/
> > > s/a rvalue/an rvalue/
> >
> > Heh no way ... and I though I knew English grammar :)
I apologize for the non-regularity of English :) [and it's somewhat
unfair for me to nit-pick, given that it's my first language - sorry;
we may as well get this stuff correct]
> >
> > Had to look that up to see what the deal was but it makes sense.
> >
> > yunion, arevalue.
Exactly. English spelling is a mess.
> >
> > > s/wrong-field-name/wrong-field-obj/
> > >
> > > to match the struct example (given that the issue being tested
> > > for
> > > is
> > > that it's the wrong object, rather than the wrong name).
> >
> > Initially, before submitting to the list, I made the code such that
> > the field
> > objects did not have to be the ones that were used when creating
> > the
> > struct or union, and forgot changing the test names.
> >
> > I figured it required too much string compares for the field names
> > and
> > pointer compares for the field object were more appropriate. To
> > create
> > dummy field objects were also kinda heavy.
> >
> > I'll address the points.
Petter, you can go ahead and commit this, once you address the various
points from my earlier review, and from Antoni's observation that
"flags" should be "m_flags" when it's a member. Antoni has committed
at least one ABI version bump since you posted this, so you'll need to
rebase your work on top of his and adjust accordingly. Please re-test
the patch before pushing it.
Thanks for all your great work on this.
Dave
