Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incorrect use of structured bindings from std::div_t #1655

Closed
dzzzb opened this issue Jul 29, 2020 · 9 comments · Fixed by #1840
Closed

incorrect use of structured bindings from std::div_t #1655

dzzzb opened this issue Jul 29, 2020 · 9 comments · Fixed by #1840
Assignees

Comments

@dzzzb
Copy link

dzzzb commented Jul 29, 2020

At https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#example-c17
we see this:

auto [ quotient, remainder ] = div(123456, 73); // break out the members of the div_t result

This is wrong as std::div_t et al. don't have any defined order of their two members according to the Standard, so it's unreliable and error-prone to assume they'll be in that or any order, so we can't use structured bindings on them.

This should be rewritten to use a valid, not risky example.

@shaneasd
Copy link
Contributor

Wow that is certainly surpising. I'm not sure div was the best example anyway since div_t already gives meaningful names to its data and used of those members still eschews a type specifier.

Perhaps std::minmax would be a good replacement?

@dzzzb
Copy link
Author

dzzzb commented Jul 30, 2020

div() would be a good example, and I'd use it thusly, iff the order of members would be made defined - and hey, maybe someone on the committee could help with that ;-) ...but maybe there's an ASM reason not to.

@jwakely
Copy link
Contributor

jwakely commented Jul 30, 2020

The C committee owns that type not the C++ committee. The order of members (and the existence of other non-standard members) is left unspecified for many C types to allow pre-ANSI implementations to remain valid if they used a different order.

@cubbimew
Copy link
Member

std::minmax might be good. Or std::set.insert (I think it's used elsewhere). Or std::to_chars to show something that's not a pair.

@MikeGitb
Copy link

Or std::to_chars to show something that's not a pair

The return type of to_chars already has properly named members, so I see little benefit in using structured binding there.

@cubbimew
Copy link
Member

the return type of std::div has properly named members too

@MikeGitb
Copy link

the return type of std::div has properly named members too

Yes. And for that reason it would imho not be a good example, even if the order of its members were defined.

@JohelEGP
Copy link
Contributor

the return type of std::div has properly named members too

Yes. And for that reason it would imho not be a good example, even if the order of its members were defined.

Then well-known domain-specific names could be used, rather than the same generic names of the members. Perhaps something like:

auto [apples_per_kid, leftover_apples] = std::div(apples, kids);

@hsutter
Copy link
Contributor

hsutter commented Sep 3, 2020

Editors call: @cubbimew please update the example to something like container insert that returns a pair of iterator and bool. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants