Re: [COMMITTED] rust_debug: Cast size_t values to unsigned long before printing.

[Rainer Orth]

Arthur Cohen  writes:

> Using %lu to format size_t values breaks 32 bit targets, and %zu is not
> supported by one of the hosts GCC aims to support - HPUX

But we do have uses of %zu in gcc/rust already!

> diff --git a/gcc/rust/expand/rust-proc-macro.cc 
> b/gcc/rust/expand/rust-proc-macro.cc
> index e8618485b71..09680733e98 100644
> --- a/gcc/rust/expand/rust-proc-macro.cc
> +++ b/gcc/rust/expand/rust-proc-macro.cc
> @@ -171,7 +171,7 @@ load_macros (std::string path)
>if (array == nullptr)
>  return {};
>  
> -  rust_debug ("Found %lu procedural macros", array->length);
> +  rust_debug ("Found %lu procedural macros", (unsigned long) array->length);

Not the best way either: array->length is std::uint64_t, so the format
should use

... %" PRIu64 " procedural...

instead.

I've attached my patch to PR rust/113461.

Rainer

-- 
-----
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [COMMITTED] rust_debug: Cast size_t values to unsigned long before printing.

[Rainer Orth]

Hi Arthur,

> Yes, I was talking about this on IRC the other day - if we do run in a
> situation where we have more than UINT32_MAX procedural macros in memory 
> we have big issues. These debug prints will probably end up getting removed
> soon as they clutter the output a lot for little information.

makes sense, especially if they break the build once in a while ;-)

> I don't mind doing it the right way for our regular prints, but we have not
> been using PRIu64 in our codebase so far, so I'd rather change all those
> incriminating format specifiers at once later down the line - this patch
> was pushed so that 32bit targets could bootstrap the Rust frontend for now.

Makes sense: using different styles throughout the codebase only creates
confusion.

On a related issue: didn't you have some 32-bit host in your CI?  I
remember having similar issues in the past which could easily be avoided
in advance this way.

Thanks.
Rainer

-- 
---------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] build: Check for cargo when building rust language

[Rainer Orth]

Andrew Pinski  writes:

> On Mon, Apr 8, 2024 at 9:39 AM  wrote:
>>
>> From: Pierre-Emmanuel Patry 
>>
>> Hello,
>>
>> The rust frontend requires cargo to build some of it's components,
>> it's presence was not checked during configuration.
>
> WHY did this go in right before the release of GCC 14?
> I don't get why this is considered temporary and it goes in right
> before a release.
> That seems broken to me.

two more questions about this:

Right now, the new cargo configure test disable rust on all of my
targets (Solaris, Linux, Darwin) which didn't have it installed.  Before
(as recent as last Friday), I could successfully build and test
crab1/rust on all of them without cargo in sight.  So I wonder if the
patch isn't premature.

Besides, while there are packaged versions of cargo for Solaris 11.4 and
Linux, Darwin hasn't anything (not checked Homebrew or similar yet).
What's worse, rustup only supports macOS 10.12 and up, while I'm still
regularly testing 10.7 and 10.11.  I don't really feel like building
rust from source here (if it works at all).  This hasn't been an issue
for any other languages that require additional tools for bootstrapping
(like Ada or D): there are versions of GNAT around that still support
those old Darwin releases, and I could use the C++ version of GDC in GCC
11.

At the very least, the Rust situation needs to be documented clearly.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] build: Check for cargo when building rust language

[Rainer Orth]

Hi Arthur,

> On 4/17/24 10:13, Rainer Orth wrote:
>> Andrew Pinski  writes:
>> 
>>> On Mon, Apr 8, 2024 at 9:39 AM  wrote:
>>>>
>>>> From: Pierre-Emmanuel Patry 
>>>>
>>>> Hello,
>>>>
>>>> The rust frontend requires cargo to build some of it's components,
>>>> it's presence was not checked during configuration.
>>>
>>> WHY did this go in right before the release of GCC 14?
>>> I don't get why this is considered temporary and it goes in right
>>> before a release.
>>> That seems broken to me.
>> two more questions about this:
>> Right now, the new cargo configure test disable rust on all of my
>> targets (Solaris, Linux, Darwin) which didn't have it installed.  Before
>> (as recent as last Friday), I could successfully build and test
>> crab1/rust on all of them without cargo in sight.  So I wonder if the
>> patch isn't premature.
>
> We already have components depending on Rust libraries in our development
> repository, so this patch is important to ensure errors are emitted early
> during the configure phase rather than later at build time. I don't think
> this is premature, considering that your targets would fail to build the
> Rust frontend next time we upstream commits, which should happen this week
> or early next week.

it would have been very helpful to state this up front: introducing a
dependency that's never used outside of a configure test right night is
still damn confusing.  An alternative might have been to commit this
patch shortly before it's actually used.

>> Besides, while there are packaged versions of cargo for Solaris 11.4 and
>> Linux, Darwin hasn't anything (not checked Homebrew or similar yet).
>> What's worse, rustup only supports macOS 10.12 and up, while I'm still
>> regularly testing 10.7 and 10.11.  I don't really feel like building
>> rust from source here (if it works at all).  This hasn't been an issue
>> for any other languages that require additional tools for bootstrapping
>> (like Ada or D): there are versions of GNAT around that still support
>> those old Darwin releases, and I could use the C++ version of GDC in GCC
>> 11.
>
> Sorry, I'm not too familiar with the Rust situation on macOS. I am reading
> that starting from Rust version 1.74, the minimum macOS version required is
> indeed 10.12, released in 2016 I believe?
>
> We currently depend on Rust version 1.72, so you should be able to install
> it on macOS 10.11. Maybe with rustup? You can try something like the
> following:
>
> curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y
> --default-toolchain=1.72.0;

I tried this with mixed success: while the part of the installation that
goes into $RUSTUP_HOME at least can run cargo --version on both 10.7
and 10.11, the other one (for $CARGO_HOME) dies with SIGILL on 10.7
trying the same.

However, when I ignore most of what's installed by rustup and point
$PATH at .../toolchains/1.72.0-x86_64-apple-darwin/bin, I can run the
cargo in there with --version even on 10.7.  For the moment, that's good
enough to get a trunk build/test with rust including working again, but
of course this doesn't prove that this will remain so once cargo is
actually used.

> which is the default installation method for Rustup, with version 1.72 of
> the language specified. I'm not able to test this, sorry, but I'm very
> interested in knowing if it works. I think you can also install Rust using
> Homebrew, but again I am not able to test this and apologize.

I'll go down this route (or try installing rust from source) only if
need be.

> The goal is to reduce that Rust version further soon anyway - we are going
> to target Rust version 1.49, released 3 years ago, as that is the version
> that gccrs aims to compile. This will bring us closer to compiling our
> dependencies with our own frontend.

Good.  At least knowing this it's easier to check what macOS versions
are supported by e.g. 1.49.

>> At the very least, the Rust situation needs to be documented clearly.
>
> I'd love to work on this - what sort of documentation do you have in mind?
> Do you mean something like the online GCC documentation or an in-tree file?
> Let me know what you'd like me to add and I'll be happy to do so.

I think this should go into gcc/doc/install.texi, as for all other
languages and targets.  This way you have all the necessary information
in one place, while some in-tree file is almost guaranteed to be
overlooked.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University