[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572 Marek Polacek changed: What|Removed |Added CC||haoxintu at gmail dot com --- Comment #19 from Marek Polacek --- *** Bug 95559 has been marked as a duplicate of this bug. ***
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572 Jason Merrill changed: What|Removed |Added Status|NEW |RESOLVED CC||jason at gcc dot gnu.org Resolution|--- |FIXED Target Milestone|--- |9.0 --- Comment #18 from Jason Merrill --- Fixed in GCC 9.
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572 --- Comment #17 from Jason Merrill --- Author: jason Date: Wed Feb 20 18:50:32 2019 New Revision: 269045 URL: https://gcc.gnu.org/viewcvs?rev=269045=gcc=rev Log: PR c++/88572 - wrong handling of braces on scalar init. * decl.c (reshape_init_r): Allow braces around scalar initializer within aggregate init. Reject double braced-init of scalar variable. Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/decl.c trunk/gcc/testsuite/g++.dg/cpp0x/initlist69.C trunk/gcc/testsuite/g++.dg/cpp1z/direct-enum-init1.C trunk/gcc/testsuite/g++.dg/init/brace1.C trunk/gcc/testsuite/g++.dg/init/brace2.C trunk/gcc/testsuite/g++.dg/init/union2.C trunk/gcc/testsuite/g++.dg/warn/Wbraces2.C
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572 Will Wray changed: What|Removed |Added Attachment #45683|0 |1 is obsolete|| --- Comment #16 from Will Wray --- Created attachment 45778 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45778=edit Updated patch addressing review comments (Updated Change Log entry for decl.c) Log: PR c++/88572 * decl.c (reshape_init_r): Remove condition that was incorrectly rejecting braces around scalar initializer within aggregate init. Produce no warning. Add a conditional block rejecting braced-init of scalar variable, including empty-brace value-initialization that was being incorrectly accepted.
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572 Will Wray changed: What|Removed |Added CC||wjwray at gmail dot com --- Comment #15 from Will Wray --- Created attachment 45683 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45683=edit Proposed patch including updated tests Accept braces around scalar initializer within aggregate init, previously rejected; this patch accepts without a warning. Reject braced empty-brace value-initialization of scalar variable previously accepted. Maintain C++98 rejection of scalar braced-init. See Bug 88572 for details, standards links and extra test code. Log: PR c++/88572 * decl.c (reshape_init_r): Remove condition that was incorrectly rejecting braces around scalar initializer within aggregate init. Produce no warning. Replace with condition rejecting braced-init of scalar variable, including empty-brace value-initialization that was being incorrectly accepted. Produce same error for both. PR c++/88572 * g++.dg/cpp0x/initlist69.C: Update 2 tests; changed error, OK. * g++.dg/cpp1z/direct-enum-init1.C: Update 3 tests; changed error. * g++.dg/init/brace1.C: Update 1 test; error only on c++98. Add 1 test with one more set of braces; error on too many braces. * g++.dg/init/brace2.C: Add 1 test; empty-brace scalar init fail. * g++.dg/init/union2.C: Update 1 test; error only on c++98. Add 1 test with one more set of braces; error on too many braces. * g++.dg/warn/Wbraces2.C: Update 3 test; 1 error only on c++98. 2 now give invalid conversion instead of braces error on >98.
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572 --- Comment #14 from Will Wray --- I intend to submit a patch, or two patches, for these scalar braced init issues: Case 1: GCC rejects braced-init of scalars in aggregates. It should accept. Case 2: GCC accepts empty braced-init of scalars (comment 13). It should reject. Fixing case 2 has the potential to break code. Such broken code is already rejected by Clang and MSVC. It seems unlikely that there will be any examples of this in the wild. Both case 1 and case 2 are fixed by the patch in comment 9. The patch needs to be updated to give a warning only when appropriate. I plan to submit a single patch to fix both issues. A more cautious approach would be to submit 2 separate patches. Please speak now if you'd prefer to see 2 patches.
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572
--- Comment #13 from Will Wray ---
Re-reviewing, I notice that the patch I posted in comment #9
now rejects nested empty-brace scalar init:
int i{{}};
which was previously accepted. So we'll need a decision on this too.
Clang rejects with -pedantic-errors or warns otherwise:
pedantic error / warning: too many braces around scalar initializer
MSVC rejects:
error: 'initializing': cannot convert from 'initializer list' to 'int'
note: Too many braces around initializer for 'int'
I reckon that Clang is right to reject under -pedantic, else accept and warn
This Quora post comes to a similar conclusion:
https://www.quora.com/Is-double-braced-scalar-initialization-allowed-by-the-C-standard-int-x
>accepting {{}} for int seems like a harmless language extension.
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572
--- Comment #12 from Will Wray ---
On further investigation the logic of using first_initializer_p looks correct.
The comment on reshape_init suggests that it wasn't intended for scalar init:
/* Undo the brace-elision allowed by [dcl.init.aggr] in a
brace-enclosed aggregate initializer.
INIT is the CONSTRUCTOR containing the list of initializers describing
a brace-enclosed initializer for an entity of the indicated aggregate TYPE.
In fact, reshape_init is used more broadly to error-check braced-init-list -
- the first substantive case is direct enum init, a scalar init special-case.
Perhaps the general scalar case should be dealt with here at the top level.
Instead the recursive call to reshape_init_r is done next
setting first_initializer_p to true - the only place it is set true
(the true value will be forwarded to reshape_init_class for class type).
So, for a scalar init, the tree type passed to reshape_init is SCALAR_TYPE_P.
This is passed down to reshape_init_r with first_initializer_p set true.
Entering reshape_init_r we have
if (first_initializer_p && !CP_AGGREGATE_TYPE_P (type)
&& has_designator_problem (d, complain))
return error_mark_node;
Here, first_initializer_p && !CP_AGGREGATE_TYPE_P (type) covers scalar init
(and other non-aggregate init).
Arriving at our block, we also enter a context requiring a single-initializer
/* A non-aggregate type is always initialized with a single
initializer. */
if (!CP_AGGREGATE_TYPE_P (type))
and then further filter down to CONSTRUCTOR with BRACE_ENCLOSED_INITIALIZER_P
and, specifically of SCALAR_TYPE_P
if (TREE_CODE (stripped_init) == CONSTRUCTOR
/* Don't complain about a capture-init. */
&& !CONSTRUCTOR_IS_DIRECT_INIT (stripped_init)
&& BRACE_ENCLOSED_INITIALIZER_P (stripped_init)) /* p7626.C */
{
if (SCALAR_TYPE_P (type))
> I understood it to mean something like {{1}, 2} is the first,
> ^^^
Me too, but taking "outermost CONSTRUCTOR node" to mean the actual outer type
here (first_initializer_p==true) and with only a single scalar initializer:
like scalar_type{{1}}not { {1} }
^^^ scalar_type^^^
So, I've convinced myself that my patch follows the existing logic.
It'd be good to get review from someone not suffering from confirmation bias.
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572
--- Comment #11 from Jonathan Wakely ---
(In reply to Will Wray from comment #9)
> The patch below seems to work as far as I've tested - please review.
>
> It looks like the bool first_initializer_p argument to reshape_init_r
> gives the context that is needed, according to the function comment;
>
> /* ...
>FIRST_INITIALIZER_P is true if this is the first initializer of the
>outermost CONSTRUCTOR node. */
>
> If I read this right then this bool arg is true for scalar init and
> false when initializing a scalar member of an aggregate (class or array).
I understood it to mean something like {{1}, 2} is the first,
^^^
but apparently not. The patch works for me, but I can't review it properly.
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572 --- Comment #10 from Will Wray --- Re: warnings; I certainly prefer to have this accepted with no warning (i.e. remove the 'else if' warning in the patch above). Saves having to disable the warning in GCC, as I have to do in Clang.
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572
--- Comment #9 from Will Wray ---
The patch below seems to work as far as I've tested - please review.
It looks like the bool first_initializer_p argument to reshape_init_r
gives the context that is needed, according to the function comment;
/* ...
FIRST_INITIALIZER_P is true if this is the first initializer of the
outermost CONSTRUCTOR node. */
If I read this right then this bool arg is true for scalar init and
false when initializing a scalar member of an aggregate (class or array).
The patch below rejects
int ii{{0}};
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -6054,16 +6054,19 @@ reshape_init_r (tree type, reshape_iter
{
if (SCALAR_TYPE_P (type))
{
- if (cxx_dialect < cxx11
- /* Isn't value-initialization. */
- || CONSTRUCTOR_NELTS (stripped_init) > 0)
+ if (cxx_dialect < cxx11 || first_initializer_p)
{
if (complain & tf_error)
error ("braces around scalar initializer for type %qT",
type);
init = error_mark_node;
}
- }
+ else if (CONSTRUCTOR_NELTS (stripped_init) > 0)
+ {
+ warning (0, "braces around scalar initializer for type %qT",
+ type);
+ }
+}
else
maybe_warn_cpp0x (CPP0X_INITIALIZER_LISTS);
}
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572 --- Comment #8 from Jonathan Wakely --- (In reply to Will Wray from comment #7) > So, as to whether it should be a warning; I think not, but... > for practical portability reasons, it probably should be left as a warning > at least until some epoch like C++2a where both GCC & Clang could remove > the warning under the -std=c++2a flag. Make sense? No, I don't think so. I don't see why the warning should be given for C++11 but not C++2a when the code is valid for both. I think it should be accepted without a warning. I'd be more inclined to say it should be accepted in C++98 with a -pedantic warning, rather than give a warning for C++11. > I wonder if the extra braces should have additional benefits like not > allowing > narrowing conversions. My limited experiments (on Clang) show no difference. The outermost braces already mean it's a context that doesn't allow narrowing, so the additional inner braces make no difference.
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572
--- Comment #7 from Will Wray ---
I guess that the bug persisted so long *because* of the status quo;
portable code had to delete extra braces to silence warnings or to compile
i.e. the warning on Clang served the purpose of promoting portability to GCC.
So, as to whether it should be a warning; I think not, but...
for practical portability reasons, it probably should be left as a warning
at least until some epoch like C++2a where both GCC & Clang could remove
the warning under the -std=c++2a flag. Make sense?
FYI My use case is counting aggregate members, similar to fields_count
in boost/pfr 'magic_get', but extended to support array members:
https://github.com/apolukhin/magic_get/blob/develop/include/boost/pfr/detail/fields_count.hpp
That code initializes an aggregate T from an index_sequence I... as
T{ ubiq{I}... } (where ubiq is a 'convert to anything' type)
To support 'skipping' arrays, you want to initialize the aggregate as
T{ {ubiq{I}}...}
but, for scalar members, this is what currently fails on GCC / warns on Clang.
Another reason that the bug persisted is the general confusion around braces
and uniform initialization - this fix will give a generic, uniform, way to
initialize the leading members of an aggregate.
I wonder if the extra braces should have additional benefits like not allowing
narrowing conversions. My limited experiments (on Clang) show no difference.
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572 --- Comment #6 from Jonathan Wakely --- I don't think reshape_init_r has the context of the type that contains the object being initialized, so might need an extra parameter passed to it.
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572 --- Comment #5 from Will Wray --- Right; the patch should only apply within aggregate initialization - arrays and aggregate structures - as the initialization of actual scalars was already correct. I'll take a look now (as I meaning to fix my enum patch this week too).
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572
--- Comment #4 from Jonathan Wakely ---
Changing it to a warning, like so:
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6054,15 +6054,19 @@ reshape_init_r (tree type, reshape_iter *d, bool
first_initializer_p,
{
if (SCALAR_TYPE_P (type))
{
- if (cxx_dialect < cxx11
- /* Isn't value-initialization. */
- || CONSTRUCTOR_NELTS (stripped_init) > 0)
+ if (cxx_dialect < cxx11)
{
if (complain & tf_error)
error ("braces around scalar initializer for type %qT",
type);
init = error_mark_node;
}
+ /* No warning for value-initialization. */
+ else if (CONSTRUCTOR_NELTS (stripped_init) > 0)
+ {
+ warning (0, "braces around scalar initializer for type %qT",
+ type);
+ }
}
else
maybe_warn_cpp0x (CPP0X_INITIALIZER_LISTS);
Causes the test code in comment 0 to issue lots of warnings (as expected) but
also a new error:
88572.cc:14:40: error: cannot bind non-const lvalue reference of type 'invalid'
{aka 'float&'} to an rvalue of type 'float'
14 | invalid scalar_2_brace = braces2(0);
|^~~
Which means that int{{0}} is now accepted, so my patch isn't right.
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572
--- Comment #3 from Jonathan Wakely ---
The relevant code is:
/* It is invalid to initialize a non-aggregate type with a
brace-enclosed initializer before C++0x.
We need to check for BRACE_ENCLOSED_INITIALIZER_P here because
of g++.old-deja/g++.mike/p7626.C: a pointer-to-member constant is
a CONSTRUCTOR (with a record type). */
if (TREE_CODE (stripped_init) == CONSTRUCTOR
/* Don't complain about a capture-init. */
&& !CONSTRUCTOR_IS_DIRECT_INIT (stripped_init)
&& BRACE_ENCLOSED_INITIALIZER_P (stripped_init)) /* p7626.C */
{
if (SCALAR_TYPE_P (type))
{
if (cxx_dialect < cxx11
/* Isn't value-initialization. */
|| CONSTRUCTOR_NELTS (stripped_init) > 0)
{
if (complain & tf_error)
error ("braces around scalar initializer for type %qT",
type);
init = error_mark_node;
}
}
The condition means we give an error for any braced-init-list in C++98, and for
a non-empty braced-init-list in later dialects . But the latter condition is
wrong, it should allow a single element (and presumably more than one element
will be rejected as an invalid initializer anyway).
I'm not even sure we should warn about this.
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572 Jonathan Wakely changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2019-01-07 Ever confirmed|0 |1 --- Comment #2 from Jonathan Wakely --- (In reply to Will Wray from comment #0) > The discussion thread is here: > http://clang-developers.42468.n3.nabble.com/braces-around-scalar-initializer- > warning-should-be-error-td4063311.html A proper link, because nabble is abysmal: http://lists.llvm.org/pipermail/cfe-dev/2018-December/060598.html
[Bug c++/88572] error: braces around scalar initializer - should be a warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88572
--- Comment #1 from Will Wray ---
This bug is straightforward to confirm.
Compile this snippet (-std=c++11 / 14 / 17 / 2a):
struct S { int i; };
S s{{0}};
Gives error: braces around scalar initializer for type 'int'
Should be a warning
(Or, follow the provided compiler explorer link to the test code
which includes two more failing cases - array and array member.)
The standard is clear that scalar brace init should be accepted:
C++14 [dcl.init.aggr]/2 says:
"Each member is copy-initialized from the corresponding initializer-clause.
[...] [ Note: If an initializer-clause is itself an initializer list, the
member is list-initialized, which will result in a recursive application of the
rules in this section if the member is an aggregate. — end note ]"
C++14 [dcl.init.list]/3.5:
"Otherwise, if the initializer list has a single element of type E and
either T is not a reference type or its referenced type is reference-related to
E, the object or reference is initialized from that element;"
Some historical links leading up to the wording fixes:
DR 155. Brace initializer for scalar
Explains it was a C/C++ incompatibility pre-11 and points to:
DR 632. Brace-enclosed initializer for scalar member of aggregate
http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#632
"The initializer-list proposal will resolve this issue..."
As stated, it was resolved in C++11 by Initializer Lists:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2672.htm
1501. Nested braces in list-initialization
http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#1501
DR1467 List-initialization of aggregate from same-type object
http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1467
Submitter: Jason Merrill Date: 2012-02-06
[Moved to DR at the November, 2014 meeting.]
Please CONFIRM this bug.
