Hi!
Kees, any progress on this?
On Mon, Jan 06, 2025 at 04:34:27PM -0500, Marek Polacek wrote:
> > - pedwarn_init (init_loc, 0,
> > - ("initializer-string for array of %qT "
> > - "is too long"), typ1);
> > - else if (warn_unterminated_string_initialization
> > - && compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
> > - warning_at (init_loc, OPT_Wunterminated_string_initialization,
> > - ("initializer-string for array of %qT "
> > - "is too long"), typ1);
> > +
> > if (compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
> > {
> > - unsigned HOST_WIDE_INT size
> > - = tree_to_uhwi (TYPE_SIZE_UNIT (type));
> > - const char *p = TREE_STRING_POINTER (inside_init);
> > -
> > - inside_init = build_string (size, p);
> > + unsigned HOST_WIDE_INT avail
> > + = tree_to_uhwi (TYPE_SIZE_UNIT (type));
> > + unsigned unit = TYPE_PRECISION (typ1) / BITS_PER_UNIT;
> > + const char *p = TREE_STRING_POINTER (inside_init);
> > +
> > + /* Construct truncated string. */
> > + inside_init = build_string (avail, p);
> > +
> > + /* Subtract the size of a single (possibly wide) character
> > + because it may be ok to ignore the terminating NUL char
> > + that is counted in the length of the constant. */
> > + if (warn_cxx_compat || len - unit > avail)
> > + {
> > + pedwarn_init (init_loc, 0,
> > + ("initializer-string for array of %qT "
> > + "is too long (%lu chars into %lu "
> > + "available)"), typ1, len, avail);
> > + }
>
> No need for the { } here.
The indentation is wrong in the whole block.
The { after compare_tree_int is indented by 2 tabs (so 16 columns), but
unsigned HOST_WIDE_INT avail already by 2 tabs and 3 spaces rather than
2 tabs and 2 spaces. So it is one column more right than it should be.
Anyway, more importantly, the "warn_cxx_compat || " part looks wrong.
For -Wc++-compat we should (perhaps with slightly different wording)
preserve the GCC 14 behavior, which was
/* Subtract the size of a single (possibly wide) character
because it's ok to ignore the terminating null char
that is counted in the length of the constant. */
if (compare_tree_int (TYPE_SIZE_UNIT (type), len - unit) < 0)
pedwarn_init (init_loc, 0,
("initializer-string for array of %qT "
"is too long"), typ1);
else if (warn_cxx_compat
&& compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
warning_at (init_loc, OPT_Wc___compat,
("initializer-string for array of %qT "
"is too long for C++"), typ1);
So, if (len - unit > avail) do the pedwarn_init (also note that %lu is
wrong for unsigned HOST_WIDE_INT, one should use %wu instead; or for the
avail case one could just print %E and pass it TYPE_SIZE_UNIT (type);
unsigned HOST_WIDE_INT doesn't have to be unsigned long, it can be also
unsigned long long). And if warn_cxx_compat, do warning_at, IMNSHO
with OPT_Wc___compat, with the "is too long for C++" substring in it
and perhaps some more details (len and avail too).
Though, perhaps we should also change
C ObjC Var(warn_unterminated_string_initialization) Warning LangEnabledBy(C
ObjC,Wextra || Wc++-compat)
to
C ObjC Var(warn_unterminated_string_initialization) Warning LangEnabledBy(C
ObjC,Wextra)
and adjust documentation, because say
-Wno-unterminated-string-initialization will no longer turn off that
warning, essentially -Wc++-compat and -Wunterminated-string-initialization
are then independent warnings with -Wc++-compat having priority over the
latter.
Or we could guard it with
warn_cxx_compat && warn_unterminated_string_initialization
but then the question is what to pass to second warning_at option,
because one needs both.
So, I think it is better to keep -Wc++-compat working as before (perhaps
with the extra descriptions of the two lengths) and then have this new
-Wunterminated-string-initialization warning implied by -Wextra which
does nothing in the rare case when -Wc++-compat is on, and otherwise
does the new stuff of warning except when initializing nonstring
objects.
Jakub
