public inbox for gcc-rust@gcc.gnu.org
 help / color / mirror / Atom feed
From: Iain Sandoe <idsandoe@googlemail.com>
To: Arthur Cohen <arthur.cohen@embecosm.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>, gcc-rust@gcc.gnu.org
Subject: Re: [COMMITTED] rust_debug: Cast size_t values to unsigned long before printing.
Date: Thu, 18 Jan 2024 11:02:53 +0000	[thread overview]
Message-ID: <282A2062-61F0-46B5-96C3-23FBBE5D3130@googlemail.com> (raw)
In-Reply-To: <7b53ee9e-1830-4878-9e12-b3e748a2e4fd@embecosm.com>

Hi Arthur,

> On 18 Jan 2024, at 10:30, Arthur Cohen <arthur.cohen@embecosm.com> wrote:

> On 1/18/24 10:13, Rainer Orth wrote:
>> Arthur Cohen <arthur.cohen@embecosm.com> 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.
> 
> 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.
> 
> 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.

For the sake of completeness, the issue does not just affect 32b hosts;  If a 64b host chooses (as Darwin does, so that 32b and 64b targets have the same representation) to make uint64_t “unsigned long long int”, then %lu breaks there too.
thanks
Iain


  parent reply	other threads:[~2024-01-18 11:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-18  9:00 Arthur Cohen
2024-01-18  9:13 ` Rainer Orth
2024-01-18 10:30   ` Arthur Cohen
2024-01-18  9:34     ` Rainer Orth
2024-01-18 20:19       ` Arthur Cohen
2024-01-18 11:02     ` Iain Sandoe [this message]
2024-01-18 20:21       ` Arthur Cohen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=282A2062-61F0-46B5-96C3-23FBBE5D3130@googlemail.com \
    --to=idsandoe@googlemail.com \
    --cc=arthur.cohen@embecosm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=gcc-rust@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).