Hi Jakub,
On 21/05/2025 14:46, Christopher Bazley wrote:
Hi Jakub,
Thanks for your review.
On 21/05/2025 13:51, Jakub Jelinek wrote:
On Wed, May 21, 2025 at 01:42:01PM +0100, Christopher Bazley wrote:
On 21/05/2025 12:26, Christopher Bazley wrote:
Hi Joseph,
Thanks for reviewing my patch.
On 20/05/2025 18:02, Joseph Myers wrote:
On Tue, 20 May 2025, Christopher Bazley wrote:
+ if (!cleared)
+ {
+ if (complete_p.padded_non_union
+ && warn_zero_init_padding_bits >=
ZERO_INIT_PADDING_BITS_ALL)
+ {
+ warning (OPT_Wzero_init_padding_bits_,
+ "Padding bits might not be initialized to zero; "
+ "consider using
%<-fzero-init-padding-bits=all%>");
+ }
+ else if (complete_p.padded_union
+ && warn_zero_init_padding_bits
+ >= ZERO_INIT_PADDING_BITS_UNIONS)
+ {
+ warning (OPT_Wzero_init_padding_bits_,
+ "Padding bits might not be initialized to zero; "
+ "consider using %<-fzero-init-padding-bits=unions%> "
+ "or %<-fzero-init-padding-bits=all%>");
Diagnostics should start with a lowercase letter.
I'll change it to "padding might not be initialized to zero..." OK?
("padding" seems to be more common than "padding bits" in existing
messages.)
Is there anything I need to do for initialization?
Sorry, I meant "internationalisation" (i.e. to support translation),
not
"initialization"!
For translations I think it is fine as is.
Please avoid {}s around single statements (e.g. the warning call above).
Originally, it was more than one statement to produce information on
how to initialise padding bits. That information was language (and
dialect) specific, which wasn't acceptable in the Gimplifier. I'll
remove the redundant braces.
Fixed in patch v4. OK?
And, I think the warnings should include text like
"if code relies on it being zero, consider"
because telling people to use those options mindlessly is undesirable.
Nobody is going to see these warnings unless they explicitly enable
them, in which case I hope they would have read the documentation first.
The current text is:
+ warning (OPT_Wzero_init_padding_bits_,
+ "padding might not be initialized to zero; "
+ "consider using
%<-fzero-init-padding-bits=unions%> "
+ "or %<-fzero-init-padding-bits=all%>");
I can add "if code relies on it being zero," although I don't think
"consider" is a very strong prescription and I highly doubt whether
the people reading these warnings will know whether the code they are
compiling relies on padding being zeroed or not. My presumption was
that programmers who know their own code intimately won't enable the
warnings in the first place.
Changed in patch v4. OK?
Thanks,
--
Christopher Bazley
Staff Software Engineer, GNU Tools Team.
Arm Ltd, 110 Fulbourn Road, Cambridge, CB1 9NJ, UK.
http://www.arm.com/
