Designated Initializer Lists

[rob...@chromium.org]

Since the switch to clang, I've seen some use of C99/C++20 Designated Initializer Lists start appearing.

Examples:

https://cs.chromium.org/chromium/src/components/password_manager/core/browser/form_parsing/form_parser.cc?rcl=c0dbb3daf5f5639ddd98f8601c9578d1b85d2d23&l=503

https://cs.chromium.org/chromium/src/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc?rcl=c0dbb3daf5f5639ddd98f8601c9578d1b85d2d23&l=336

Particularly fun is that some C99 expressions are not allowed in C++20:

Change:​ In C++, designated initialization support is restricted compared to the corresponding functionality in C. In C++, designators for non-static data members must be specified in declaration order, designators for array elements and nested designators are not supported, and designated and non-designated initializers cannot be mixed in the same initializer list.

Example:

struct A { int x, y; };

struct B { struct A a; };

struct A a = {.y = 1, .x = 2}; // valid C, invalid C++ 

int arr[3] = {[1] = 5}; // valid C, invalid C++ 

struct B b = {.a.x = 0}; // valid C, invalid C++ 

struct A a = {.x = 1, 2}; // valid C, invalid C++ 

Given that this doesn't seem to yet be finalized for C++20 and it isn't quite ready on Clang, do we want to ban such usage in Chromium until it is fully defined?

[Peter Kasting]

It should already be banned AFAIK, since it is not in the C++11/14 approved language features list, and nothing past those is supposed to be supported.

I'm surprised we compile in a mode where clang allows this.

PK

[Nico Weber]

clang allows C99 features in C++ mode as an extension. We could pass -Wc99-extensions to flag those as warnings and not accept them, but it seems fine to me to allow them if people think they're useful. (If others disagree, I don't feel super strongly though.)

If C++20 makes some C99 things invalid, I'm sure there will come a warning to flag the incompatible bits soon, so maybe we should wait for that to happen and then turn on only that. Given that we're still working on the switch to 17, and 20 is still years away from being ratified, I'm not sure that's a huge cause for concern.

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFCS%2BnNKcAvyN0gcjvgy0iHn1mB2M6FiMgoFtfGf%3D%3DYUAA%40mail.gmail.com.

[Peter Kasting]

On Thu, Jun 21, 2018 at 4:11 PM Nico Weber <tha...@chromium.org> wrote:

clang allows C99 features in C++ mode as an extension. We could pass -Wc99-extensions to flag those as warnings and not accept them, but it seems fine to me to allow them if people think they're useful. (If others disagree, I don't feel super strongly though.)

Does that warn only for things that are "valid C99 but not valid C++"?

If so, I'd vote for turning on that warning.   The C++ standards committee thinks about the question of how to maintain compatibility with C over time, so until they allow something into C++ itself, I would defer to their judgment.

PK

[Nico Weber]

On Thu, Jun 21, 2018 at 7:18 PM Peter Kasting <pkas...@google.com> wrote:

On Thu, Jun 21, 2018 at 4:11 PM Nico Weber <tha...@chromium.org> wrote:

clang allows C99 features in C++ mode as an extension. We could pass -Wc99-extensions to flag those as warnings and not accept them, but it seems fine to me to allow them if people think they're useful. (If others disagree, I don't feel super strongly though.)

Does that warn only for things that are "valid C99 but not valid C++"?

I think so, but I haven't tried. I only looked up the designated init code in clang in response to this thread and saw that this warning exists. You can try sending a tryjob with it set and checking what it finds :-) (disable warnings as errors to see more than just the first TU.)

[Peter Kasting]

On Thu, Jun 21, 2018 at 4:22 PM Nico Weber <tha...@chromium.org> wrote:

You can try sending a tryjob with it set and checking what it finds :-) (disable warnings as errors to see more than just the first TU.)

Sadly I can't spare bandwidth for this right now -- but I filed https://bugs.chromium.org/p/chromium/issues/detail?id=855271 in case someone else would like to.

PK 

[Sébastien Marchand]

(resurrecting an old thread)

It looks like we never came up with a decision on this? There's at least 30 places where designated initializers are being used in the codebase (https://cs.chromium.org/search/?q=%5C%7B%5Cs*%5C.%5Cw%2B%5Cs*%3D+-f:third_party&sq=package:chromium&type=cs) and I'm guilty for a few of those . If you think that these should be removed then I can remove my use cases and open a crbug to get the other one removed (IMO it's probably not worth the trouble but I'm not an expert on this topic :) ).

On Thursday, June 21, 2018 at 7:34:00 PM UTC-4, Peter Kasting wrote:

[Peter Kasting]

On Wed, Apr 24, 2019 at 4:28 PM Sébastien Marchand <sebma...@chromium.org> wrote:

(resurrecting an old thread)

It looks like we never came up with a decision on this? There's at least 30 places where designated initializers are being used in the codebase (https://cs.chromium.org/search/?q=%5C%7B%5Cs*%5C.%5Cw%2B%5Cs*%3D+-f:third_party&sq=package:chromium&type=cs) and I'm guilty for a few of those . If you think that these should be removed then I can remove my use cases and open a crbug to get the other one removed (IMO it's probably not worth the trouble but I'm not an expert on this topic :) ).

The Google styleguide forbids nonstandard extensions, explicitly mentioning designated initializers as an example.  See http://google.github.io/styleguide/cppguide.html#Nonstandard_Extensions .

Given this, I think we should remove these.

PK

[Robert Liao]

+1 to that. 

-wc99-extensions warns on a few other features that we do use in Chrome. It wasn't possible to just single out designated initializers.

I didn't have the bandwidth to add this support to clang and to fix up Chrome, so I let things stand as we waited for MSVC to catch up.

[Peter Kasting]

On Wed, Apr 24, 2019 at 5:57 PM Robert Liao <rob...@chromium.org> wrote:

+1 to that. 

-wc99-extensions warns on a few other features that we do use in Chrome. It wasn't possible to just single out designated initializers.

I didn't have the bandwidth to add this support to clang and to fix up Chrome, so I let things stand as we waited for MSVC to catch up.

FWIW, the style guide prohibition on extensions is broad and covers more things, likely including those "features we do use in Chrome", so I would vote to remove those too :)

PK 

[Jeremy Roman]

+1, with the obvious exception that such things are often useful if contained to limited areas and protected by macros (__attribute__ and __declspec being the most obvious, but not only, examples).

PK 

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFAEZHeLtyEgAo9OHLE8e7rUS3qB839U4dh7-VoLYP%2BbQw%40mail.gmail.com.

[Sébastien Marchand]

Sounds good to me! Thanks for the clarifications on this, I'll remove my use cases and open a crbug to get the other ones removed (I'll try to cc all the persons who owns the parts of the code using this).

--

Sébastien Marchand | Software Developer | sebma...@google.com 

[Sébastien Marchand]

FYI I've opened crbug.com/956581 to track this.

[Sébastien Marchand]

The internal style guide now allows designated initializers ( go/cstyle#Designated_initializers), they are using the "-Wreorder-init-list" flag to prevent potentially bad use cases.

Should we reconsider supporting this feature or should we wait for the public style guide to be updated?  

[Peter Kasting]

On Thu, Dec 12, 2019 at 10:12 AM Sébastien Marchand <sebma...@chromium.org> wrote:

The internal style guide now allows designated initializers ( go/cstyle#Designated_initializers), they are using the "-Wreorder-init-list" flag to prevent potentially bad use cases.

Should we reconsider supporting this feature or should we wait for the public style guide to be updated?

Do these even compile with --std=c++14 and the other flags we pass?  That is, is the extension to support these on by default?

(Personally, I'd prefer to wait until Chromium is actually C++20 to support these; I'm uncomfortable using nonstandard extensions unless we have to.)

PK 

[Sébastien Marchand]

On Thu, Dec 12, 2019 at 1:54 PM Peter Kasting <pkas...@google.com> wrote:

On Thu, Dec 12, 2019 at 10:12 AM Sébastien Marchand <sebma...@chromium.org> wrote:

The internal style guide now allows designated initializers ( go/cstyle#Designated_initializers), they are using the "-Wreorder-init-list" flag to prevent potentially bad use cases.

Should we reconsider supporting this feature or should we wait for the public style guide to be updated?

Do these even compile with --std=c++14 and the other flags we pass?  That is, is the extension to support these on by default?

I don't know, using designated initializers in //src seems to work fine but I don't see the "--std=c++14" flag on the compile command line...

I don't have a strong opinion on this. Designated initializer lists are convenient and can sometimes save us from using some quite verbose builder patterns but if people are not comfortable using them for now then it's fine, we can wait. I mostly wanted to mention that this is not allowed in the Google Style guide.

 

(Personally, I'd prefer to wait until Chromium is actually C++20 to support these; I'm uncomfortable using nonstandard extensions unless we have to.)

PK 

[Peter Kasting]

On Thu, Dec 12, 2019 at 11:37 AM Sébastien Marchand <sebma...@chromium.org> wrote:

On Thu, Dec 12, 2019 at 1:54 PM Peter Kasting <pkas...@google.com> wrote:

On Thu, Dec 12, 2019 at 10:12 AM Sébastien Marchand <sebma...@chromium.org> wrote:

The internal style guide now allows designated initializers ( go/cstyle#Designated_initializers), they are using the "-Wreorder-init-list" flag to prevent potentially bad use cases.

Should we reconsider supporting this feature or should we wait for the public style guide to be updated?

Do these even compile with --std=c++14 and the other flags we pass?  That is, is the extension to support these on by default?

I don't know, using designated initializers in //src seems to work fine but I don't see the "--std=c++14" flag on the compile command line...

From chatting with Rob earlier it sounds like we support these right now due to compiling with C99 extensions enabled, which means not only are there some in our codebase already, but people may add ones that violate the C++20 rules.

MSVC never supported C99, so any uses here must have been added since we went clang-only.  My preference would be to try and remove any use of C99 extensions, disable C99 extensions in our build globally, and then wait for C++20.  But maybe that isn't the right cost/benefit decision.

PK 

[Scott Graham]

MSVC >= 2015 supported the majority of C99 (including designated initializers) so I don't think there's any problem in practice.

 

PK 

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFB7ULn781-oTOzAGPJzKFHAL%2By%2BOFPLBoprvigDgSHAHA%40mail.gmail.com.

[Robert Liao]

While MSVC did have some C99 support, it didn't support the designated initializers. Chrome didn't start getting them until the full switch to clang. (I used to do a parallel MSVC build, finding these new uses).

[Sébastien Marchand]

On Thu, Dec 12, 2019 at 4:15 PM Robert Liao <rob...@chromium.org> wrote:

While MSVC did have some C99 support, it didn't support the designated initializers. Chrome didn't start getting them until the full switch to clang. (I used to do a parallel MSVC build, finding these new uses).

On Thu, Dec 12, 2019 at 12:59 PM Scott Graham <sco...@chromium.org> wrote:

On Thu, Dec 12, 2019 at 12:47 PM 'Peter Kasting' via cxx <c...@chromium.org> wrote:

On Thu, Dec 12, 2019 at 11:37 AM Sébastien Marchand <sebma...@chromium.org> wrote:

On Thu, Dec 12, 2019 at 1:54 PM Peter Kasting <pkas...@google.com> wrote:

On Thu, Dec 12, 2019 at 10:12 AM Sébastien Marchand <sebma...@chromium.org> wrote:

The internal style guide now allows designated initializers ( go/cstyle#Designated_initializers), they are using the "-Wreorder-init-list" flag to prevent potentially bad use cases.

Should we reconsider supporting this feature or should we wait for the public style guide to be updated?

Do these even compile with --std=c++14 and the other flags we pass?  That is, is the extension to support these on by default?

I don't know, using designated initializers in //src seems to work fine but I don't see the "--std=c++14" flag on the compile command line...

From chatting with Rob earlier it sounds like we support these right now due to compiling with C99 extensions enabled, which means not only are there some in our codebase already, but people may add ones that violate the C++20 rules.

MSVC never supported C99, so any uses here must have been added since we went clang-only.  My preference would be to try and remove any use of C99 extensions, disable C99 extensions in our build globally, and then wait for C++20.  But maybe that isn't the right cost/benefit decision.

I've opened crbug.com/956581 to track the removal of these use cases but never found the energy to push on this (I've fixed my use cases).

 

MSVC >= 2015 supported the majority of C99 (including designated initializers) so I don't think there's any problem in practice.

 

PK 

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFB7ULn781-oTOzAGPJzKFHAL%2By%2BOFPLBoprvigDgSHAHA%40mail.gmail.com.