C++17 feature proposal: Structured bindings

171 views
Skip to first unread message

Victor Costan

unread,
Dec 24, 2021, 1:10:56 AM12/24/21
to cxx
Hi folks,

Structured bindings is my favorite C++17 feature. https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/c++11.md#Structured-bindings-tbd

Therefore, I'm nominating this as (one of) the first C++17 features to be allowed in Chromium.

Looking forward to your thoughts!
    Victor

Honglin Yu

unread,
Dec 24, 2021, 5:01:37 AM12/24/21
to Victor Costan, cxx
+1, it can help get rid of ".first" and ".second" (which are vague and verbose) when doing range-based loops through maps.

--
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/CAP_mGKojW-XjiLBdavuh4iKx56S92-oqzvQB-nssa6u_-SWwBw%40mail.gmail.com.

Peter Kasting

unread,
Dec 24, 2021, 9:42:04 AM12/24/21
to Victor Costan, cxx
+1

PK

Roland Bock

unread,
Dec 24, 2021, 10:09:32 AM12/24/21
to Peter Kasting, Victor Costan, cxx
+1

On Fri, Dec 24, 2021 at 3:42 PM 'Peter Kasting' via cxx <c...@chromium.org> wrote:
+1

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.

Daniel Cheng

unread,
Dec 24, 2021, 12:27:48 PM12/24/21
to Roland Bock, Peter Kasting, Victor Costan, cxx
Are there any subtleties worth noting? For example, structured binding uses 'auto' but always creates an alias, whereas 'auto' elsewhere follows the normal rules for template argument deduction.

Daniel

On Fri, 24 Dec 2021 at 07:09, 'Roland Bock' via cxx <c...@chromium.org> wrote:
+1

On Fri, Dec 24, 2021 at 3:42 PM 'Peter Kasting' via cxx <c...@chromium.org> wrote:
+1

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/CAAHOzFDRhOrsD0Z170XaQxLFsQ%3DPBesJqP3_xM%3DLYGz5U8K_Ng%40mail.gmail.com.

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

Avi Drissman

unread,
Dec 24, 2021, 12:55:02 PM12/24/21
to Daniel Cheng, Roland Bock, Peter Kasting, Victor Costan, cxx
It's not really an "alias" and it's different for tuple types. https://jguegant.github.io/blogs/tech/structured-bindings.html has great info on how to think of it.

Victor Costan

unread,
Dec 24, 2021, 7:00:41 PM12/24/21
to Avi Drissman, Daniel Cheng, Roland Bock, Peter Kasting, cxx
CL proposing this change:  https://crrev.com/c/3356325

I'd appreciate reviews!
    Victor

Daniel Cheng

unread,
Dec 26, 2021, 12:05:25 PM12/26/21
to Victor Costan, Avi Drissman, Roland Bock, Peter Kasting, cxx
Well, I wasn't quite sure what to call it. I guess my point is that they're not really normal variables (for example, they do not work with lambda capture in C++17).

Daniel

Victor Costan

unread,
Dec 26, 2021, 8:47:56 PM12/26/21
to Daniel Cheng, Avi Drissman, Roland Bock, Peter Kasting, cxx
https://crrev.com/c/3356325 has 1 LGTM. I'm planning to submit on Monday, unless I hear otherwise by then.

Thank you!
    Victor

Peter Kasting

unread,
Dec 27, 2021, 1:36:48 PM12/27/21
to Victor Costan, Daniel Cheng, Avi Drissman, Roland Bock, cxx
I'd give things a few more days, just because there are so many holidays around now that some people may not have had a chance to weigh in.

PK

Honglin Yu

unread,
Dec 27, 2021, 3:01:56 PM12/27/21
to Peter Kasting, Victor Costan, Daniel Cheng, Avi Drissman, Roland Bock, cxx
As Daniel mentioned, it seems there is indeed inconsistent behavior between compilers regarding capturing the bindings in lambdas: e.g. it is allowed in gcc but not in clang (even adding -std=c++20). Maybe we should at least be careful with such a controversial situation? 

Daniel Cheng

unread,
Dec 27, 2021, 5:07:57 PM12/27/21
to Honglin Yu, Peter Kasting, Victor Costan, Avi Drissman, Roland Bock, cxx
The clang behavior is standards-compliant and is what we'd expect in Chrome. I think it's still worth a mention in our documentation as a kind of weird edge case. I believe there's an internal C++ TotW that discusses this, but that particular one has not been made public yet.


2021年12月27日(月) 12:01 Honglin Yu <hong...@google.com>:

dan...@chromium.org

unread,
Jan 4, 2022, 10:39:16 AM1/4/22
to Daniel Cheng, Honglin Yu, Peter Kasting, Victor Costan, Avi Drissman, Roland Bock, cxx

Nico Weber

unread,
Jan 7, 2022, 2:20:21 PM1/7/22
to Dana Jansens, Daniel Cheng, Honglin Yu, Peter Kasting, Victor Costan, Avi Drissman, Roland Bock, cxx
I have two questions about the CL:

1. It replaces

    FontEnumerationStatus status;
    base::ReadOnlySharedMemoryRegion region;
    std::tie(status, region) = manager_sync_->EnumerateLocalFonts();

with

    auto [status, region] = manager_sync_->EnumerateLocalFonts();

How does this interact with the guidance of spelling out the type unless it's obvious (https://google.github.io/styleguide/cppguide.html#Type_deduction)?

2. In the same function, a few lines further down, it replaces

    std::tie(status, region) = manager_sync_->EnumerateLocalFonts();

with

    auto [status2, region2] = manager_sync_->EnumerateLocalFonts();

(and then it replaces status and region with status2 and region2 in the following code.)

Is introducing two new variables with a trailing `2` preferable over just calling tie() here?


I don't have strong opinions about what the answers to these questions should be, but it might be nice to think about this and then have some guidance summarizing community consensus in the guide.

Peter Kasting

unread,
Jan 10, 2022, 11:55:09 AM1/10/22
to Nico Weber, Dana Jansens, Daniel Cheng, Honglin Yu, Victor Costan, Avi Drissman, Roland Bock, cxx
On Fri, Jan 7, 2022 at 11:20 AM Nico Weber <tha...@chromium.org> wrote:
I have two questions about the CL:

1. It replaces

    FontEnumerationStatus status;
    base::ReadOnlySharedMemoryRegion region;
    std::tie(status, region) = manager_sync_->EnumerateLocalFonts();

with

    auto [status, region] = manager_sync_->EnumerateLocalFonts();

How does this interact with the guidance of spelling out the type unless it's obvious (https://google.github.io/styleguide/cppguide.html#Type_deduction)?

That guidance does go on to give specific guidance for structured bindings that say that they're often a net readability win, so my reading of the upstream guide is that it thinks usage of structured bindings is OK even though they sacrifice type names, and the precise usage at each location is "judgement call".  Here, for example, as a reviewer I would judge that the variable names are already giving much the same information as the type names, so this change would probably be OK.  That said, one could argue for a change to "auto [font_enumeration_status, shared_memory_region] = ..." or the like.

2. In the same function, a few lines further down, it replaces

    std::tie(status, region) = manager_sync_->EnumerateLocalFonts();

with

    auto [status2, region2] = manager_sync_->EnumerateLocalFonts();

(and then it replaces status and region with status2 and region2 in the following code.)

Is introducing two new variables with a trailing `2` preferable over just calling tie() here?

I don't love "2"s, but I do prefer SSA-style usage of variables where possible over "reuse a temp for a similar purpose".  It makes lifetimes clearer.  (Even better: individual scope blocks with {}, that use the same temporary variable names inside each one.)  That seems like total individual preference, though.  I'm not sure we need to give any guidance about it?

PK

Roland Bock

unread,
Jan 10, 2022, 12:46:40 PM1/10/22
to Peter Kasting, Nico Weber, Dana Jansens, Daniel Cheng, Honglin Yu, Victor Costan, Avi Drissman, cxx
I also read the guidance this way. The structured bindings code is

 - more concise
 - allows for more const

This improvement in readability should balance the absence of explicit types.

Nico Weber

unread,
Jan 10, 2022, 1:00:25 PM1/10/22
to Peter Kasting, Dana Jansens, Daniel Cheng, Honglin Yu, Victor Costan, Avi Drissman, Roland Bock, cxx
On Mon, Jan 10, 2022 at 11:55 AM Peter Kasting <pkas...@google.com> wrote:
On Fri, Jan 7, 2022 at 11:20 AM Nico Weber <tha...@chromium.org> wrote:
I have two questions about the CL:

1. It replaces

    FontEnumerationStatus status;
    base::ReadOnlySharedMemoryRegion region;
    std::tie(status, region) = manager_sync_->EnumerateLocalFonts();

with

    auto [status, region] = manager_sync_->EnumerateLocalFonts();

How does this interact with the guidance of spelling out the type unless it's obvious (https://google.github.io/styleguide/cppguide.html#Type_deduction)?

That guidance does go on to give specific guidance for structured bindings that say that they're often a net readability win, so my reading of the upstream guide is that it thinks usage of structured bindings is OK even though they sacrifice type names, and the precise usage at each location is "judgement call".

That's not my reading of the style guide (but maybe I'm reading it wrong). As far as I can tell, the structure is (represented as a nested list):

  Type Deduction
    Definition: Here are contexts in which type deduction can or must be used
      List of examples that includes, among other things, structured bindings. The examples describe how the features work but don't give style advice on them.
    Decision: "Use type deduction only to make the code clearer or safer, and do not use it merely to avoid the inconvenience of writing an explicit type. [...] These principles apply to all forms of type deduction, but the details vary, as described in the following sections."

As I read this, the section gives an overview over the various types, and then the unifying guidance "generally, only use if type is clear".

That is, the Google C++ style guide as I read it doesn't say how to reconcile the general advice "only omit type when it's clear" with "structured bindings force you to omit the type".

Peter Kasting

unread,
Jan 10, 2022, 1:17:59 PM1/10/22
to Nico Weber, Dana Jansens, Daniel Cheng, Honglin Yu, Victor Costan, Avi Drissman, Roland Bock, cxx
On Mon, Jan 10, 2022 at 10:00 AM Nico Weber <tha...@chromium.org> wrote:
On Mon, Jan 10, 2022 at 11:55 AM Peter Kasting <pkas...@google.com> wrote:
On Fri, Jan 7, 2022 at 11:20 AM Nico Weber <tha...@chromium.org> wrote:
I have two questions about the CL:

1. It replaces

    FontEnumerationStatus status;
    base::ReadOnlySharedMemoryRegion region;
    std::tie(status, region) = manager_sync_->EnumerateLocalFonts();

with

    auto [status, region] = manager_sync_->EnumerateLocalFonts();

How does this interact with the guidance of spelling out the type unless it's obvious (https://google.github.io/styleguide/cppguide.html#Type_deduction)?

That guidance does go on to give specific guidance for structured bindings that say that they're often a net readability win, so my reading of the upstream guide is that it thinks usage of structured bindings is OK even though they sacrifice type names, and the precise usage at each location is "judgement call".

That's not my reading of the style guide (but maybe I'm reading it wrong). As far as I can tell, the structure is (represented as a nested list):

  Type Deduction
    Definition: Here are contexts in which type deduction can or must be used
      List of examples that includes, among other things, structured bindings. The examples describe how the features work but don't give style advice on them.
    Decision: "Use type deduction only to make the code clearer or safer, and do not use it merely to avoid the inconvenience of writing an explicit type. [...] These principles apply to all forms of type deduction, but the details vary, as described in the following sections."

I think this omits the important followon bit:

"Unlike other forms of type deduction, structured bindings can actually give the reader additional information, by giving meaningful names to the elements of a larger object. This means that a structured binding declaration may provide a net readability improvement over an explicit type, even in cases where auto would not. Structured bindings are especially beneficial when the object is a pair or tuple..."

To me, that amounts to a caveat that partially (but not completely) speaks to your concern.

PK

dan...@chromium.org

unread,
Jan 11, 2022, 9:32:17 AM1/11/22
to Peter Kasting, Nico Weber, Daniel Cheng, Honglin Yu, Victor Costan, Avi Drissman, Roland Bock, cxx
FWIW I generally let auto get used in a lot more places in tests than in prod code as it helps to avoid testing the types involved which are not the point of the test.

If this was production code I would not like the use of auto here, as it's not _more_ clear by using auto, and it's not an iterator in a loop. I think the correct solution here for production is to use a struct with named fields, not a pair, as the return type.

Victor Costan

unread,
Jan 11, 2022, 10:31:58 PM1/11/22
to dan...@chromium.org, Peter Kasting, Nico Weber, Daniel Cheng, Honglin Yu, Avi Drissman, Roland Bock, cxx
I updated the CL based on the feedback posted in the code review.

Nico, would you be OK with concluding that the C++ style guide allows the changes in the CL?

Thank you,
    Victor


Nico Weber

unread,
Jan 12, 2022, 10:31:14 AM1/12/22
to Victor Costan, Dana Jansens, Peter Kasting, Daniel Cheng, Honglin Yu, Avi Drissman, Roland Bock, cxx
Yes, fine with me.

Personally, I think it'd be nice if the blurb included a "This feature forces omitting the type name, so only use it where knowledge of the concrete type isn't important, consistent with the general guidance for auto".

Avi Drissman

unread,
Jan 25, 2022, 1:54:47 PM1/25/22
to Nico Weber, Victor Costan, Dana Jansens, Peter Kasting, Daniel Cheng, Honglin Yu, Roland Bock, cxx
I'm seeing a dangerous situation with structured bindings that I'm failing to get a warning on. Filed as https://crbug.com/1290897 but noting here for people's thoughts.

Something that structured bindings can be good for is iterating over a std::map. I reviewed https://crrev.com/c/3413750 which turned

for (const auto& pr : element_to_data_lookup_)

into

for (const auto [element, data] : element_to_data_lookup_)

which is missing the &. Now, this should warn, and a sample I threw into Compiler Explorer did warn. If I took the sample and dumped it into Chromium, I got a warning, but the actual CL doesn't warn.

As noted again, I filed this, but folks should be careful in migrating this.

Avi

Avi Drissman

unread,
Jan 25, 2022, 2:15:25 PM1/25/22
to Nico Weber, Victor Costan, Dana Jansens, Peter Kasting, Daniel Cheng, Honglin Yu, Roland Bock, cxx
Actually, never mind. I mis-read what map was being looped on in the CL.
Reply all
Reply to author
Forward
0 new messages