Sorry, I know, coding style thread... But it's Friday and this is 
somewhat related to the previous thread.

Bug 525063 added a lot of lines like:

     explicit TTextAttr(bool aGetRootValue)
       : mGetRootValue(aGetRootValue)
       , mIsDefined{ false }
       , mIsRootDefined{ false }
     {
     }

I think I find them hard to read and ugly.

Those changes I assume were generated with clang-format / 
clang-format-diff using the "Mozilla" coding style, so I'd rather ask 
people to agree in whether we prefer that style or other in order to 
change that if needed.

Would people agree to use:

  , mIsRootDefined { false }

Instead of:

  , mIsRootDefined{ false }

And:

  , mFoo { }

Instead of:

  , mFoo{}

?

I assume the same would be for variables, I find:

   AutoRestore<bool> restore { mFoo };

easier to read than:

   AutoRestore<bool> restore{ mFoo };

What's people's opinion on that? Would people be fine with a more 
general "spaces around braces" rule? I can't think of a case right now 
where I personally wouldn't prefer it.

Also, we should probably state that consistency is preferred (I assume 
we generally agree on that), so in this case braces probably weren't 
even needed, or everything should've switched to them.

Finally, while I'm here, regarding default member initialization, what's 
preferred?

   uint32_t* mFoo = nullptr;

Or:

   uint32_t* mFoo { nullptr };

I'm ambivalent, but brace syntax should cover more cases IIUC (that is, 
there are things that you can write with braces that you couldn't with 
equals I _think_).

Should we state a preference? Or just say that both are allowed but 
consistency is better?

  -- Emilio

On 4/13/18 9:37 AM, Emilio Cobos Álvarez wrote:
> Would people agree to use:
> 
>   , mIsRootDefined { false }
> 
> Instead of:
> 
>   , mIsRootDefined{ false }

So my take is that we should not use braced initializer syntax in 
constructor initializer lists.  The reason for that is that it makes it 
much harder to scan for where the constructor body starts.  Doubly so 
when ifdefs in the initializer list are involved.  Triply so when 
someone writes it as:

   explicit TTextAttr()
     , mIsRootDefined
   {
     false
   }
#ifdef SOMETHING
#endif
   {
   }

which is what clang-format did in some of the cases in the patch for bug 
525063.

In particular, I think this should have just used:

   , mIsRootDefined(false)

I don't have a strong opinion about the one space when we do use the 
braced initializer syntax.  But we should make sure we don't end up with 
the above monstrosity where it looks like the ctor body.

-Boris

On 4/13/18 4:22 PM, Boris Zbarsky wrote:
> So my take is that we should not use braced initializer syntax in 
> constructor initializer lists.  The reason for that is that it makes it 
> much harder to scan for where the constructor body starts.

I don't think that's true in the general case where the braces remain to 
the right of the identifier in the same line, fwiw, but...

> Doubly so when ifdefs in the initializer list are involved. > Triply so when someone writes it as:
> 
>    explicit TTextAttr()
>      , mIsRootDefined
>    {
>      false
>    }
> #ifdef SOMETHING
> #endif
>    {
>    }
> 
> which is what clang-format did in some of the cases in the patch for bug 
> 525063.

.... this is definitely terrible I agree :(

> In particular, I think this should have just used:
> 
>    , mIsRootDefined(false)

Agreed in this case (specially given there were previous members 
initialized with parenthesis, and the braces are not really necessary).

> I don't have a strong opinion about the one space when we do use the 
> braced initializer syntax.  But we should make sure we don't end up with 
> the above monstrosity where it looks like the ctor body.

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1453973 for that.

> -Boris
> _______________________________________________
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform

I would like to resurrect this thread since it would help us a lot for bug =
1453795 to come up to a consensus on when to use bracelets and when to use =
parenthesis. Also I must point out a thing here, that was also mentioned he=
re earlier, that there are situations where we cannot use parenthesis. This=
 is when we want to initialize a structure that doesn't have a ctor, like:
[1]
struct str {
  int a;
  int b;
};

class Str {
  str s;
  int a;
public:
  Str() : s{0}, a(0) {}
};

Also it would help a lot if we would establish how many, spaces should be b=
etween the parenthesis or the bracelets, like how do we prefer [1] or [2]

[2]
class Str {
  str s;
  int a;
public:
  Str() : s{ 0 }, a( 0 ) {}
};

I don't have a personal preference here, but right now there are several pl=
aces in our code that combine spaces between parenthesis/bracelets with no =
spaces.

--Apple-Mail=_A0C1480A-D155-4D05-BA04-006F88B7C93E
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8



> On 5 Jun 2018, at 12:54 pm, bpostelnicu@mozilla.com wrote:
>=20
> I would like to resurrect this thread since it would help us a lot for =
bug 1453795 to come up to a consensus on when to use bracelets and when =
to use parenthesis. Also I must point out a thing here, that was also =
mentioned here earlier, that there are situations where we cannot use =
parenthesis. This is when we want to initialize a structure that doesn't =
have a ctor, like:
> [1]
> struct str {
>  int a;
>  int b;
> };
>=20
> class Str {
>  str s;
>  int a;
> public:
>  Str() : s{0}, a(0) {}
> };
>=20
> Also it would help a lot if we would establish how many, spaces should =
be between the parenthesis or the bracelets, like how do we prefer [1] =
or [2]
>=20
> [2]
> class Str {
>  str s;
>  int a;
> public:
>  Str() : s{ 0 }, a( 0 ) {}
> };
>=20
> I don't have a personal preference here, but right now there are =
several places in our code that combine spaces between =
parenthesis/bracelets with no spaces.

The current coding style: =
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_St=
yle states to not use space.

There=E2=80=99s no case where a parenthesis should be followed by a =
space.

Many things wrong here:
First the bracket { should be on a new line :

class/struct str
{
=E2=80=A6
}

Initialization are to be on multiple-lines.

clang-format would have made it:
  class Str
  {
    str s;
    int a;

  public:
    Str()
      : s{ 0 }
      , a(0)
    {
    }
  };

IMHO, should be going for C++11 initializer, it=E2=80=99s much clearer, =
and avoid duplicated code when you need multiple constructors.
What is str? I assume not a plain object, so it should have its own =
initializer.

so it all becomes:
  class Str
  {
    str s;
    int a =3D 0;

  public:
    Str() {}
  };

or:
  class Str
  {
    str s;
    int a =3D 0;

  public:
    Str() =3D default;
  };

(and I prefer constructors to be defined at the start of the class =
definition)

My $0.02=

--Apple-Mail=_A0C1480A-D155-4D05-BA04-006F88B7C93E
Content-Disposition: attachment;
	filename=smime.p7s
Content-Type: application/pkcs7-signature;
	name=smime.p7s
Content-Transfer-Encoding: base64

MIAGCSqGSIb3DQEHAqCAMIACAQExCzAJBgUrDgMCGgUAMIAGCSqGSIb3DQEHAQAAoIILJDCCBTYw
ggQeoAMCAQICEQCBSieaFmHJtubZY/a2JhCmMA0GCSqGSIb3DQEBCwUAMIGXMQswCQYDVQQGEwJH
QjEbMBkGA1UECBMSR3JlYXRlciBNYW5jaGVzdGVyMRAwDgYDVQQHEwdTYWxmb3JkMRowGAYDVQQK
ExFDT01PRE8gQ0EgTGltaXRlZDE9MDsGA1UEAxM0Q09NT0RPIFJTQSBDbGllbnQgQXV0aGVudGlj
YXRpb24gYW5kIFNlY3VyZSBFbWFpbCBDQTAeFw0xODAxMDQwMDAwMDBaFw0xOTAxMDQyMzU5NTla
MCYxJDAiBgkqhkiG9w0BCQEWFWp5YXZlbmFyZEBtb3ppbGxhLmNvbTCCASIwDQYJKoZIhvcNAQEB
BQADggEPADCCAQoCggEBAOt9s5pE1IsPSfvpowRfZN3jD+6Ki/wevXHp0PYL9Ko6dnZZDVbKolbV
Mc87X6KGNtGDBM2O0ojc9bQmohoEX+dBJevnxnTWBmGUqBN7g6lQIxqAlp2osCZy2GaqliowkBFq
ZUiBmzLQ3yf5tcoletYceH4NyRaBGgcCl1IP7VFITsAMplt65BsY1lSZdgQqegi6DB6NywfikLc6
+0K0z3uzmCnKTKpnoXtrAVebqcBwRLPWWf8bXkGsRmdBg5YrL/FOG6WHkVTCf1yfDpJ8woaZeBRN
a3mOJ7eQG2VQgrnBQxndVdxnGY+r9MUf2ssP1V+Ji0/CO8wtBN2SAwgAWTUCAwEAAaOCAeswggHn
MB8GA1UdIwQYMBaAFIKvbIz4xf6WYXzoHz0rcUhexIvAMB0GA1UdDgQWBBTkrzUBvD5lKF4LO+Ha
fJfJE0wcvTAOBgNVHQ8BAf8EBAMCBaAwDAYDVR0TAQH/BAIwADAgBgNVHSUEGTAXBggrBgEFBQcD
BAYLKwYBBAGyMQEDBQIwEQYJYIZIAYb4QgEBBAQDAgUgMEYGA1UdIAQ/MD0wOwYMKwYBBAGyMQEC
AQEBMCswKQYIKwYBBQUHAgEWHWh0dHBzOi8vc2VjdXJlLmNvbW9kby5uZXQvQ1BTMFoGA1UdHwRT
MFEwT6BNoEuGSWh0dHA6Ly9jcmwuY29tb2RvY2EuY29tL0NPTU9ET1JTQUNsaWVudEF1dGhlbnRp
Y2F0aW9uYW5kU2VjdXJlRW1haWxDQS5jcmwwgYsGCCsGAQUFBwEBBH8wfTBVBggrBgEFBQcwAoZJ
aHR0cDovL2NydC5jb21vZG9jYS5jb20vQ09NT0RPUlNBQ2xpZW50QXV0aGVudGljYXRpb25hbmRT
ZWN1cmVFbWFpbENBLmNydDAkBggrBgEFBQcwAYYYaHR0cDovL29jc3AuY29tb2RvY2EuY29tMCAG
A1UdEQQZMBeBFWp5YXZlbmFyZEBtb3ppbGxhLmNvbTANBgkqhkiG9w0BAQsFAAOCAQEAJxnChKvx
dNNwyycLtwgfvLUwVsI2Nwtn1N4ZnHrD/TKbct+oc/AfvVhC0ixptvzGD+2v+peCbCCoyXUtltBi
IND7tZqC+RJqwqz0yvKF2aaeig/hLza6vDEDbflMM4IeOeCCBT4LYgxSREG2eKIJUiyrfNdidqPT
OMcigZCS7leZuTDEb19UGtggvMDHQzHhEVmFuFixUEXkNxDmbsaBwBaWB274Sw52IQmw04dOoo9i
D1TmpXr/HOgx5B4uorKZwhKHRT0s7l2A6Nx/XeD5z9nynTDCJ+7xCTNGCjRiD9SLv4DgzAjGiDwL
CkHn0JhCwLui51mII3bUNTw65TZf4TCCBeYwggPOoAMCAQICEGqb4Tg7/ytrnwHV2binUlYwDQYJ
KoZIhvcNAQEMBQAwgYUxCzAJBgNVBAYTAkdCMRswGQYDVQQIExJHcmVhdGVyIE1hbmNoZXN0ZXIx
EDAOBgNVBAcTB1NhbGZvcmQxGjAYBgNVBAoTEUNPTU9ETyBDQSBMaW1pdGVkMSswKQYDVQQDEyJD
T01PRE8gUlNBIENlcnRpZmljYXRpb24gQXV0aG9yaXR5MB4XDTEzMDExMDAwMDAwMFoXDTI4MDEw
OTIzNTk1OVowgZcxCzAJBgNVBAYTAkdCMRswGQYDVQQIExJHcmVhdGVyIE1hbmNoZXN0ZXIxEDAO
BgNVBAcTB1NhbGZvcmQxGjAYBgNVBAoTEUNPTU9ETyBDQSBMaW1pdGVkMT0wOwYDVQQDEzRDT01P
RE8gUlNBIENsaWVudCBBdXRoZW50aWNhdGlvbiBhbmQgU2VjdXJlIEVtYWlsIENBMIIBIjANBgkq
hkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvrOeV6wodnVAFsc4A5jTxhh2IVDzJXkLTLWg0X06WD6c
pzEup/Y0dtmEatrQPTRI5Or1u6zf+bGBSyD9aH95dDSmeny1nxdlYCeXIoymMv6pQHJGNcIDpFDI
MypVpVSRsivlJTRENf+RKwrB6vcfWlP8dSsE3Rfywq09N0ZfxcBa39V0wsGtkGWC+eQKiz4pBZYK
jrc5NOpG9qrxpZxyb4o4yNNwTqzaaPpGRqXB7IMjtf7tTmU2jqPMLxFNe1VXj9XB1rHvbRikw8lB
oNoSWY66nJN/VCJv5ym6Q0mdCbDKCMPybTjoNCQuelc0IAaO4nLUXk0BOSxSxt8kCvsUtQIDAQAB
o4IBPDCCATgwHwYDVR0jBBgwFoAUu69+Aj36pvE8hI6t7jiY7NkyMtQwHQYDVR0OBBYEFIKvbIz4
xf6WYXzoHz0rcUhexIvAMA4GA1UdDwEB/wQEAwIBhjASBgNVHRMBAf8ECDAGAQH/AgEAMBEGA1Ud
IAQKMAgwBgYEVR0gADBMBgNVHR8ERTBDMEGgP6A9hjtodHRwOi8vY3JsLmNvbW9kb2NhLmNvbS9D
T01PRE9SU0FDZXJ0aWZpY2F0aW9uQXV0aG9yaXR5LmNybDBxBggrBgEFBQcBAQRlMGMwOwYIKwYB
BQUHMAKGL2h0dHA6Ly9jcnQuY29tb2RvY2EuY29tL0NPTU9ET1JTQUFkZFRydXN0Q0EuY3J0MCQG
CCsGAQUFBzABhhhodHRwOi8vb2NzcC5jb21vZG9jYS5jb20wDQYJKoZIhvcNAQEMBQADggIBAHhc
soEoNE887l9Wzp+XVuyPomsX9vP2SQgG1NgvNc3fQP7TcePo7EIMERoh42awGGsma65u/ITse2hK
ZHzT0CBxhuhb6txM1n/y78e/4ZOs0j8CGpfb+SJA3GaBQ+394k+z3ZByWPQedXLL1OdK8aRINTsj
k/H5Ns77zwbjOKkDamxlpZ4TKSDMKVmU/PUWNMKSTvtlenlxBhh7ETrN543j/Q6qqgCWgWuMAXij
nRglp9fyadqGOncjZjaaSOGTTFB+E2pvOUtY+hPebuPtTbq7vODqzCM6ryEhNhzf+enm0zlpXK7q
332nXttNtjv7VFNYG+I31gnMrwfHM5tdhYF/8v5UY5g2xANPECTQdu9vWPoqNSGDt87b3gXb1AiG
GaI06vzgkejL580ul+9hz9D0S0U4jkhJiA7EuTecP/CFtR72uYRBcunwwH3fciPjviDDAI9SnC/2
aPY8ydehzuZutLbZdRJ5PDEJM/1tyZR2niOYihZ+FCbtf3D9mB12D4ln9icgc7CwaxpNSCPt8i/G
qK2HsOgkL3VYnwtx7cJUmpvVdZ4ognzgXtgtdk3ShrtOS1iAN2ZBXFiRmjVzmehoMof06r1xub+8
5hFQzVxZx5/bRaTKTlL8YXLI8nAbR9HWdFqzcOoB/hxfEyIQpx9/s81rgzdEZOofSlZHynoSMYID
ujCCA7YCAQEwga0wgZcxCzAJBgNVBAYTAkdCMRswGQYDVQQIExJHcmVhdGVyIE1hbmNoZXN0ZXIx
EDAOBgNVBAcTB1NhbGZvcmQxGjAYBgNVBAoTEUNPTU9ETyBDQSBMaW1pdGVkMT0wOwYDVQQDEzRD
T01PRE8gUlNBIENsaWVudCBBdXRoZW50aWNhdGlvbiBhbmQgU2VjdXJlIEVtYWlsIENBAhEAgUon
mhZhybbm2WP2tiYQpjAJBgUrDgMCGgUAoIIB4TAYBgkqhkiG9w0BCQMxCwYJKoZIhvcNAQcBMBwG
CSqGSIb3DQEJBTEPFw0xODA2MDUxMjUwMjZaMCMGCSqGSIb3DQEJBDEWBBTm94gzWZBP8bPPbQB+
RqS4K3utbjCBvgYJKwYBBAGCNxAEMYGwMIGtMIGXMQswCQYDVQQGEwJHQjEbMBkGA1UECBMSR3Jl
YXRlciBNYW5jaGVzdGVyMRAwDgYDVQQHEwdTYWxmb3JkMRowGAYDVQQKExFDT01PRE8gQ0EgTGlt
aXRlZDE9MDsGA1UEAxM0Q09NT0RPIFJTQSBDbGllbnQgQXV0aGVudGljYXRpb24gYW5kIFNlY3Vy
ZSBFbWFpbCBDQQIRAIFKJ5oWYcm25tlj9rYmEKYwgcAGCyqGSIb3DQEJEAILMYGwoIGtMIGXMQsw
CQYDVQQGEwJHQjEbMBkGA1UECBMSR3JlYXRlciBNYW5jaGVzdGVyMRAwDgYDVQQHEwdTYWxmb3Jk
MRowGAYDVQQKExFDT01PRE8gQ0EgTGltaXRlZDE9MDsGA1UEAxM0Q09NT0RPIFJTQSBDbGllbnQg
QXV0aGVudGljYXRpb24gYW5kIFNlY3VyZSBFbWFpbCBDQQIRAIFKJ5oWYcm25tlj9rYmEKYwDQYJ
KoZIhvcNAQEBBQAEggEANiWo/jIEN8AesfB2cX6I8I/I6fz6t5N+Rs6lV4Akd6P0aYf5K53uJivh
N4zURlVrvF8UfJfyVpesQsK7eS5JZIk4PaGpiaeHN3uSzjkonpY+0qjWt/XWUDd0cKMlHrbHUtdn
QzIhECR0NS/JTkHRoWD2TKzaqcxpF0g1HkSwRkT4KUf4/Rkr4eiAkzkDdv1E9vwHn0+OhyX+K34d
0DPwV0dv0k1dfJuDzrj5UGCJwMD3vHHIBwXz+9ov43NocJhKZIFuY8PV4MwbC8nE+tS6kXC0FBA0
5yQ9KIoLWx2xpPyq25tX3avbptiTKbC3Mk46nsmTccTogEVpZeLzWpjT5gAAAAAAAA==
--Apple-Mail=_A0C1480A-D155-4D05-BA04-006F88B7C93E--

Reading back through I think the consensus, at least for initializer lists
was:

   1. Prefer parenthesis, ie:
   , mBool(true)
   2. If using braces, maintain the same spacing you would use with
   parenthesis, ie:
   , mStructWithoutCtor{42}

1. was pragmatic as this is what we already do, 2. was for consistency with
1.

To answer Bogdan's question, it looks like we prefer [1], although it would
be nice to see that codified in our style doc.

jya, you make some interesting points, but we should keep the scope of this
discussion focused. You might want to raise them in separate threads --
"Should we recommend initialization at member declaration", "Should we
recommend where a ctor should is defined", etc.

-e


On Tue, Jun 5, 2018 at 5:50 AM, Jean-Yves Avenard <jyavenard@mozilla.com>
wrote:

>
>
> > On 5 Jun 2018, at 12:54 pm, bpostelnicu@mozilla.com wrote:
> >
> > I would like to resurrect this thread since it would help us a lot for
> bug 1453795 to come up to a consensus on when to use bracelets and when t=
o
> use parenthesis. Also I must point out a thing here, that was also
> mentioned here earlier, that there are situations where we cannot use
> parenthesis. This is when we want to initialize a structure that doesn't
> have a ctor, like:
> > [1]
> > struct str {
> >  int a;
> >  int b;
> > };
> >
> > class Str {
> >  str s;
> >  int a;
> > public:
> >  Str() : s{0}, a(0) {}
> > };
> >
> > Also it would help a lot if we would establish how many, spaces should
> be between the parenthesis or the bracelets, like how do we prefer [1] or
> [2]
> >
> > [2]
> > class Str {
> >  str s;
> >  int a;
> > public:
> >  Str() : s{ 0 }, a( 0 ) {}
> > };
> >
> > I don't have a personal preference here, but right now there are severa=
l
> places in our code that combine spaces between parenthesis/bracelets with
> no spaces.
>
> The current coding style: https://developer.mozilla.org/
> en-US/docs/Mozilla/Developer_guide/Coding_Style states to not use space.
>
> There=E2=80=99s no case where a parenthesis should be followed by a space=
..
>
> Many things wrong here:
> First the bracket { should be on a new line :
>
> class/struct str
> {
> =E2=80=A6
> }
>
> Initialization are to be on multiple-lines.
>
> clang-format would have made it:
>   class Str
>   {
>     str s;
>     int a;
>
>   public:
>     Str()
>       : s{ 0 }
>       , a(0)
>     {
>     }
>   };
>
> IMHO, should be going for C++11 initializer, it=E2=80=99s much clearer, a=
nd avoid
> duplicated code when you need multiple constructors.
> What is str? I assume not a plain object, so it should have its own
> initializer.
>
> so it all becomes:
>   class Str
>   {
>     str s;
>     int a =3D 0;
>
>   public:
>     Str() {}
>   };
>
> or:
>   class Str
>   {
>     str s;
>     int a =3D 0;
>
>   public:
>     Str() =3D default;
>   };
>
> (and I prefer constructors to be defined at the start of the class
> definition)
>
> My $0.02
> _______________________________________________
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
>

On 06/05/2018 06:48 PM, Eric Rahm wrote:
> Reading back through I think the consensus, at least for initializer lists
> was:
> 
>     1. Prefer parenthesis, ie:
>     , mBool(true)
>     2. If using braces, maintain the same spacing you would use with
>     parenthesis, ie:
>     , mStructWithoutCtor{42}
> 
> 1. was pragmatic as this is what we already do, 2. was for consistency with
> 1.

I personally would prefer one space at each side when using braces:

  , mFoo { 0 }

But people seemed to think that for consistency we should use no spaces 
with braces as well... So if more people agree with that, oh well :)

Though I really find no-spaces-around-braces harder to read, not only in 
constructors but in general initializer lists. For example, I find:

   return { Foo::Bar, bar, baz };

easier to read than:

   return {Foo::Bar, bar, baz};

(and also more consistent with what you write in Javascript or Rust).

But nor sure if this discussion would expand to those cases.

  -- Emilio

> 
> To answer Bogdan's question, it looks like we prefer [1], although it would
> be nice to see that codified in our style doc.
> 
> jya, you make some interesting points, but we should keep the scope of this
> discussion focused. You might want to raise them in separate threads --
> "Should we recommend initialization at member declaration", "Should we
> recommend where a ctor should is defined", etc.
> 
> -e
> 
> 
> On Tue, Jun 5, 2018 at 5:50 AM, Jean-Yves Avenard <jyavenard@mozilla.com>
> wrote:
> 
>>
>>
>>> On 5 Jun 2018, at 12:54 pm, bpostelnicu@mozilla.com wrote:
>>>
>>> I would like to resurrect this thread since it would help us a lot for
>> bug 1453795 to come up to a consensus on when to use bracelets and when to
>> use parenthesis. Also I must point out a thing here, that was also
>> mentioned here earlier, that there are situations where we cannot use
>> parenthesis. This is when we want to initialize a structure that doesn't
>> have a ctor, like:
>>> [1]
>>> struct str {
>>>   int a;
>>>   int b;
>>> };
>>>
>>> class Str {
>>>   str s;
>>>   int a;
>>> public:
>>>   Str() : s{0}, a(0) {}
>>> };
>>>
>>> Also it would help a lot if we would establish how many, spaces should
>> be between the parenthesis or the bracelets, like how do we prefer [1] or
>> [2]
>>>
>>> [2]
>>> class Str {
>>>   str s;
>>>   int a;
>>> public:
>>>   Str() : s{ 0 }, a( 0 ) {}
>>> };
>>>
>>> I don't have a personal preference here, but right now there are several
>> places in our code that combine spaces between parenthesis/bracelets with
>> no spaces.
>>
>> The current coding style: https://developer.mozilla.org/
>> en-US/docs/Mozilla/Developer_guide/Coding_Style states to not use space.
>>
>> There’s no case where a parenthesis should be followed by a space.
>>
>> Many things wrong here:
>> First the bracket { should be on a new line :
>>
>> class/struct str
>> {
>> …
>> }
>>
>> Initialization are to be on multiple-lines.
>>
>> clang-format would have made it:
>>    class Str
>>    {
>>      str s;
>>      int a;
>>
>>    public:
>>      Str()
>>        : s{ 0 }
>>        , a(0)
>>      {
>>      }
>>    };
>>
>> IMHO, should be going for C++11 initializer, it’s much clearer, and avoid
>> duplicated code when you need multiple constructors.
>> What is str? I assume not a plain object, so it should have its own
>> initializer.
>>
>> so it all becomes:
>>    class Str
>>    {
>>      str s;
>>      int a = 0;
>>
>>    public:
>>      Str() {}
>>    };
>>
>> or:
>>    class Str
>>    {
>>      str s;
>>      int a = 0;
>>
>>    public:
>>      Str() = default;
>>    };
>>
>> (and I prefer constructors to be defined at the start of the class
>> definition)
>>
>> My $0.02
>> _______________________________________________
>> dev-platform mailing list
>> dev-platform@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-platform
>>
>>
> _______________________________________________
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
> 

On 6/5/18 3:10 PM, Emilio Cobos Álvarez wrote:
> I personally would prefer one space at each side when using braces:
> 
>   , mFoo { 0 }

I think the reason people tend to think of this as not wanting spaces is 
that they are thinking of it as a constructor call.  The parentheses 
syntax for initializer lists helps think of things that way, of course.

> Though I really find no-spaces-around-braces harder to read, not only in 
> constructors but in general initializer lists. For example, I find:
> 
>    return { Foo::Bar, bar, baz };
> 
> easier to read than:
> 
>    return {Foo::Bar, bar, baz};

Right, whereas this one feels more like a struct literal of some sort, 
where you do want the spaces delimiting things...

-Boris

On Wednesday, June 6, 2018 at 5:35:59 AM UTC+10, Boris Zbarsky wrote:
> On 6/5/18 3:10 PM, Emilio Cobos =C3=81lvarez wrote:
> > I personally would prefer one space at each side when using braces:
> >=20
> >  =C2=A0, mFoo { 0 }
>=20
> I think the reason people tend to think of this as not wanting spaces is=
=20
> that they are thinking of it as a constructor call.  The parentheses=20
> syntax for initializer lists helps think of things that way, of course.
> [...]
> -Boris

I also prefer `m{ 0 }`.

`m(0)` (direct initialization) and `m{ 0 }` (list initialization) are reall=
y different operations, and may in fact call different constructors -- E.g.=
, the latter will prefer constructors that take an std::initializer_list.
So I think it is important to emphasize the difference, especially for revi=
ewers to be able to pick on that difference.

g.

On Fri, 13 Apr 2018 10:22:06 -0400, Boris Zbarsky wrote:

> On 4/13/18 9:37 AM, Emilio Cobos Álvarez wrote:
>> Would people agree to use:
>>
>>   , mIsRootDefined { false }
>>
>> Instead of:
>>
>>   , mIsRootDefined{ false }
>
> So my take is that we should not use braced initializer syntax in
> constructor initializer lists.  The reason for that is that it
> makes it much harder to scan for where the constructor body
> starts.  Doubly so when ifdefs in the initializer list are
> involved.  Triply so when someone writes it as:
>
>   explicit TTextAttr()
>     , mIsRootDefined
>   {
>     false
>   }
> #ifdef SOMETHING
> #endif
>   {
>   }
>
> which is what clang-format did in some of the cases in the patch
> for bug 525063.
>
> In particular, I think this should have just used:
>
>   , mIsRootDefined(false)

One advantage of list-initialization over direct initialization is
that narrowing conversions are prohibited in list-initialization.
This is particularly useful in member initializer lists, where the
types are not readily visible (which is another good reason for
default member initialization).

I guess that doesn't matter for bool initializers, but consistency
....

Below, D will compile, but L will not.

struct D
{
  int i;
  D() : i(3.14) {}
};

struct L
{
  int i;
  L() : i{ 3.14 } {}
};

One advantage of lack of space between identifier and
brace-init-list is that it is visibly different from the space
before the constructor body.

On Wednesday, June 6, 2018 at 5:21:05 AM UTC+3, gsqu...@mozilla.com wrote:
> On Wednesday, June 6, 2018 at 5:35:59 AM UTC+10, Boris Zbarsky wrote:
> > On 6/5/18 3:10 PM, Emilio Cobos =C3=81lvarez wrote:
> > > I personally would prefer one space at each side when using braces:
> > >=20
> > >  =C2=A0, mFoo { 0 }
> >=20
> > I think the reason people tend to think of this as not wanting spaces i=
s=20
> > that they are thinking of it as a constructor call.  The parentheses=20
> > syntax for initializer lists helps think of things that way, of course.
> > [...]
> > -Boris
>=20
> I also prefer `m{ 0 }`.
>=20
> `m(0)` (direct initialization) and `m{ 0 }` (list initialization) are rea=
lly different operations, and may in fact call different constructors -- E.=
g., the latter will prefer constructors that take an std::initializer_list.
> So I think it is important to emphasize the difference, especially for re=
viewers to be able to pick on that difference.
>=20
> g.

I completely agree with your point regarding the {} list initialize, and al=
so the rest of opinions posted here.=20
If what Eric wrote is the common ground here, I will be more than happy to =
update the the coding style doc.=20
I think we=E2=80=99ve started to use =E2=80=9C{ }=E2=80=9D list initialiser=
 on a mass scale since it was easier to implement the auto-fix for bug 5250=
63.