Error braces around scalar initializer

Description

Will Wray

2018-12-21 18:10:33 UTC

Note: This is a C++ specific issue (not C). All versions since C++11.

    int a[1]{{0}}; // GCC error - should be warning

GCC:   error: braces around scalar initializer for type 'int'
Clang: warning: braces around scalar initializer [-Wbraced-scalar-init]
Intel: warning #1119: extra braces are nonstandard
MSVC:  no warning (with Wall)

    "I'm frankly stunned that GCC rejects."
        Quote from a Clang dev (cfe-dev thread linked below).

Same error on initializing a scalar member of an aggregate:

    struct S { int i; };
    S s{{0}}; // GCC error - should be warning

Or, on initializing an array-of-scalar aggregate member:

    struct SA { int n[1]; };
    SA sa{{{0}}}; // GCC error - should be warning

A different but related bug (non-SFINAE-error) was just fixed on Clang trunk.
The discussion thread is here:
http://clang-developers.42468.n3.nabble.com/braces-around-scalar-initializer-warning-should-be-error-td4063311.html
The Clang fix commit is here, with test code applicable to this GCC bug:
https://github.com/llvm-mirror/clang/commit/3410781dffff51e89751a2494bd8dfe7e7d83541

Here's a compiler explorer link with the test code:
https://gcc.godbolt.org/z/8HCba4

test code listing
=================

  using valid = int&;
  using invalid = float&;

  template<typename T> valid braces1(decltype(T{0})*);
  template<typename T> invalid braces1(...);

  template<typename T> valid braces2(decltype(T{{0}})*);
  template<typename T> invalid braces2(...);

  template<typename T> valid braces3(decltype(T{{{0}}})*);
  template<typename T> invalid braces3(...);

  valid   scalar_1_brace = braces1<int>(0);
  invalid scalar_2_brace = braces2<int>(0);
  invalid scalar_3_brace = braces3<int>(0);

//int a[1]{{0}}; // GCC error - should be warning

  valid   array_scalar_1_brace = braces1<int[1]>(0);
  valid   array_scalar_2_brace = braces2<int[1]>(0); // GCC error
  invalid array_scalar_3_brace = braces3<int[1]>(0);

  struct SS { int n; };
//SS ss{{0}}; // GCC error - should be warning

  valid   struct_scalar_1_brace = braces1<SS>(0);
  valid   struct_scalar_2_brace = braces2<SS>(0);  // GCC error
  invalid struct_scalar_3_brace = braces3<SS>(0);

  struct SAS { int n[1]; };
//SAS sas{{{0}}}; // GCC error - should be warning

  valid struct_array_scalar_1_brace = braces1<SAS>(0);
  valid struct_array_scalar_2_brace = braces2<SAS>(0);
  valid struct_array_scalar_3_brace = braces3<SAS>(0); // GCC error

Comment 1

Will Wray

2018-12-28 22:33:37 UTC

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.

Comment 3

Jonathan Wakely

2019-01-07 15:19:01 UTC

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.

Comment 4

Jonathan Wakely

2019-01-07 15:42:37 UTC

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<int>(0);
      |                            ~~~~~~~~~~~~^~~

Which means that int{{0}} is now accepted, so my patch isn't right.

Comment 5

Will Wray

2019-01-07 15:50:18 UTC

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).

Comment 6

Jonathan Wakely

2019-01-07 16:02:21 UTC

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.

Comment 7

Will Wray

2019-01-07 16:47:16 UTC

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.

Comment 8

Jonathan Wakely

2019-01-07 16:52:39 UTC

(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.

Comment 9

Will Wray

2019-01-07 22:15:43 UTC

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);
 	}

Comment 10

Will Wray

2019-01-07 22:24:47 UTC

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.

Comment 11

Jonathan Wakely

2019-01-07 23:57:01 UTC

(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.

Comment 12

Will Wray

2019-01-08 19:51:47 UTC

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.

Comment 13

Will Wray

2019-01-09 16:07:56 UTC

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.

Comment 14

Will Wray

2019-02-04 22:57:17 UTC

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.

Comment 15

Will Wray

2019-02-12 22:36:57 UTC

Created attachment 45683 [details]
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.

Comment 16

Will Wray

2019-02-20 17:18:34 UTC

Created attachment 45778 [details]
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.

Comment 17

Jason Merrill

2019-02-20 18:51:03 UTC

Author: jason
Date: Wed Feb 20 18:50:32 2019
New Revision: 269045

URL: https://gcc.gnu.org/viewcvs?rev=269045&root=gcc&view=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

Comment 18

Jason Merrill

2019-02-21 18:17:24 UTC

Fixed in GCC 9.

Comment 19

Marek Polacek

2020-06-10 13:34:57 UTC

*** Bug 95559 has been marked as a duplicate of this bug. ***