This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] don't trim empty string initializers for pointers (PR 90947)

On 8/1/19 4:22 PM, Martin Sebor wrote:
On 6/27/19 3:25 PM, Jason Merrill wrote:
On 6/23/19 5:51 PM, Martin Sebor wrote:
On 6/22/19 9:37 PM, Jason Merrill wrote:
On 6/21/19 8:05 PM, Martin Sebor wrote:
The solution we implemented in GCC 9 to get the mangling of
non-type template arguments of class types containing array
members consistent regardless of the form of their
initialization introduced a couple of bugs.  One of these
is the subject of this patch.  The bug results in stripping
trailing initializers for array elements that involve the empty
string such as in here:

   void f (void)
     const char* a[1][1] = { "" };
     if (!a[0][0])
       __builtin_abort ();

The problem is caused by relying on initializer_zerop() that
returns true for the empty string regardless of whether it's
used to initialize an array or a pointer (the function doesn't
know what the initializer is being used for).

Why doesn't the existing POINTER_TYPE_P check handle this?  It's clearly intended to.

It does but only only for initializers of "leaf" elements/members.
The function processes initializers of enclosing elements or members
recursively.  When initializer_zerop returns true for one of those,
it doesn't differentiate between an empty string that's being used
to initialize a leaf array or a leaf pointer.

For the example above, the first time through, elt_type is char*
and elt_init is the empty string, or the array char[1], and so
the initializer is retained.

The second time through (after the leaf recursive call returns),
elt_init is { "" } (i.e., an array of one empty string), elt_type
is also an array, and initializer_zerop(elt_init) returns true, so
the initializer is dropped.

I'm actually surprised this initializer_zerop ambiguity between
arrays and strings doesn't come up elsewhere.  That's also why
I added the new function to tree.c.

Makes sense.

Btw., it belatedly occurred to me that the new function doesn't
correctly handled unnamed bit-fields so I've corrected it to
handle those as well.  Attached is the revised patch with this
change and a test to exercise it.

+         || (DECL_BIT_FIELD (fld) && !!DECL_NAME (fld)))

Hmm, this looks like it would skip named bit-fields rather than unnamed.

It was a typo that I fixed by hand separately in my tree and in
the patch but missed Emacs' question when saving the patch and
wound up sending the unchanged version.

PS I found DECL_UNNAMED_BIT_FIELD in c-family/c-common.h.
I used (DECL_BIT_FIELD (fld) && !DECL_NAME (fld)) because
the former macro isn't available in tree.c.  Does that mean
that that would make the new function not completely correct
in some other languages?

I don't know if other languages treat unnamed bit-fields as initializable.

+/* Analogous to initializer_zerop but examines the type for which
+   the initializer is being used but unlike it, considers empty
+   strings to be zero initializers for arrays and non-zero for
+   pointers.  */

This comment could use more editing.

I've broken up the long sentence into two so it reads a little
better.  If you were objecting to something else please let me

Other than that and the typo the patch is the same.

OK, thanks.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]