RE: [PATCH] rust: Fix up aarch64-linux bootstrap [PR106072]

[Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>]

> -----Original Message-----
> From: Gcc-patches <gcc-patches-
> bounces+kyrylo.tkachov=arm.com@gcc.gnu.org> On Behalf Of Arthur Cohen
> Sent: Wednesday, December 14, 2022 10:05 AM
> To: Jakub Jelinek <jakub@redhat.com>; gcc-rust@gcc.gnu.org; gcc-
> patches@gcc.gnu.org
> Subject: Re: [PATCH] rust: Fix up aarch64-linux bootstrap [PR106072]
> 
> Hi Jakub,
> 
> On 12/14/22 10:14, Jakub Jelinek via Gcc-rust wrote:
> > Hi!
> >
> > Bootstrap fails on aarch64-linux and some other arches with:
> > .../gcc/rust/parse/rust-parse-impl.h: In member function
> ‘Rust::AST::ClosureParam
> Rust::Parser<ManagedTokenSource>::parse_closure_param() [with
> ManagedTokenSource = Rust::Lexer]’:
> > .../gcc/rust/parse/rust-parse-impl.h:8916:49: error: ‘this’ pointer is null [-
> Werror=nonnull]
> > The problem is that while say on x86_64-linux the side-effects in the
> > arguments are evaluated from last argument to first, on aarch64-linux
> > it is the other way around, from first to last.  The C++ I believe even
> > in C++17 makes the evaluation of those side-effects unordered
> > (indeterminately sequenced with no interleaving), so that is fine.
> > But, when the call in return statement is evaluated from first to
> > last, std::move (pattern) happens before pattern->get_locus () and
> > the former will make pattern (std::unique_ptr) a wrapper object around
> > nullptr, so dereferencing it later to call get_locus () on it is invalid.
> >
> > The following patch fixes that, ok for trunk?

FWIW, with this patch my aarch64 bootstrap progressed past the previous point of failure (it's currently in stage 3).
Thanks,
Kyrill

> >
> > 2022-12-14  Jakub Jelinek  <jakub@redhat.com>
> >
> > 	PR rust/106072
> > 	* parse/rust-parse-impl.h (parse_closure_param): Store
> > 	pattern->get_locus () in a temporary before std::move (pattern) is
> > 	invoked.
> >
> > --- gcc/rust/parse/rust-parse-impl.h.jj	2022-12-13
> 16:50:12.708093521 +0100
> > +++ gcc/rust/parse/rust-parse-impl.h	2022-12-14 09:50:31.731111932
> +0100
> > @@ -8912,8 +8912,9 @@ Parser<ManagedTokenSource>::parse_closur
> >   	}
> >       }
> >
> > -  return AST::ClosureParam (std::move (pattern), pattern->get_locus (),
> > -			    std::move (type), std::move (outer_attrs));
> > +  Location loc = pattern->get_locus ();
> > +  return AST::ClosureParam (std::move (pattern), loc, std::move (type),
> > +			    std::move (outer_attrs));
> >   }
> >
> >   // Parses a grouped or tuple expression (disambiguates).
> >
> > 	Jakub
> >
> 
> Thanks :) this looks good to me. We already have that issue fixed in our
> upstream dev branch, by this PR:
> 
> https://github.com/Rust-GCC/gccrs/pull/1619
> 
> but we have yet to update GCC's master with our upstream dev branch, so
> in the meantime feel free to apply your patch. When I'll get to updating
> master, I'm expecting these kinds of tiny conflicts and we'll deal with
> them.
> 
> Thanks a lot for working on this and sorry that my tardiness in updating
> has caused a duplication of efforts.
> 
> All the best,
> 
> --
> Arthur Cohen <arthur.cohen@embecosm.com>
> 
> Toolchain Engineer
> 
> Embecosm GmbH
> 
> Geschäftsführer: Jeremy Bennett
> Niederlassung: Nürnberg
> Handelsregister: HR-B 36368
> www.embecosm.de
> 
> Fürther Str. 27
> 90429 Nürnberg
> 
> 
> Tel.: 091 - 128 707 040
> Fax: 091 - 128 707 077