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

Add exceptions to F.15, F.16, and F.18 for shared_ptr types #2010

Merged
merged 1 commit into from
Jan 19, 2023

Conversation

russellmcc
Copy link
Contributor

@russellmcc russellmcc commented Dec 15, 2022

Currently these guidelines conflict with R.34, R.35, and R.36.

This conflict has led to confusion, where it's unclear which guidelines to prefer for shared_ptr types.

In a previous PR I proposed preferring the "F" series of guidelines. This PR takes the opposite approach and prefers the "R" guidelines for shared_ptr types.

I don't feel strongly about which guidelines to prefer, I just want to make sure the guidelines are internally consistent.

Currently these guidelines conflict with R.34, R.35, and R.36.

This conflict has led to confusion, where it's unclear which
guidelines to prefer for `shared_ptr` types.

In a [previous
PR](isocpp#1989) I proposed
preferring the "F" series of guidelines.  This PR takes the opposite
approach and prefers the "R" guidelines for `shared_ptr` types.

I don't feel strongly about which guidelines to prefer, I just want to make
sure the guidelines are internally consistent.
@russellmcc russellmcc force-pushed the add-exceptions-to-f-15-18 branch from 622de22 to e67026c Compare December 15, 2022 22:10
@russellmcc russellmcc changed the title Add exceptions to F.16, F.16, and F.18 for shared_ptr types Add exceptions to F.15, F.16, and F.18 for shared_ptr types Dec 15, 2022
@jwakely
Copy link
Contributor

jwakely commented Dec 16, 2022

This conflict has led to confusion, where it's unclear which guidelines to prefer for shared_ptr types.

It seems pretty obvious to me that for shared_ptr you should follow the rule that is specifically referring to shared_ptr, not a more general rule that covers all types and doesn't mention shared_ptr.

If there's a real problem to be solved (which I'm not convinced about), rather than three additions, wouldn't it be simpler to just add a single note in R.34 saying that for shared_ptr this guideline overrides the more general rules?

@russellmcc
Copy link
Contributor Author

russellmcc commented Dec 16, 2022

In a large document there’s much value in having the guidelines be “locally understandable” in the sense that developers are able to correctly follow a guideline after reading it. Allowing guidelines to conflict harms this, since in that case one can only understand exceptions to the guideline after one has read every other guideline.

Since this is a living document that changes over time, allowing conflicts means that changing or adding a guideline in one section could cause a change to the enforcement rules for a guideline in a totally different section. Maintainers of tooling for a guideline-specific enforcement would then have to monitor the whole document for changes rather than just one guideline.

In this case the explicit exceptions required to make the guidelines non-conflicting are quite minor, so it seems worth it to me.

@hsutter
Copy link
Contributor

hsutter commented Jan 19, 2023

Editors call: Thanks!

@hsutter hsutter merged commit 1ba3371 into isocpp:master Jan 19, 2023
hsutter added a commit that referenced this pull request Jan 19, 2023
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 this pull request may close these issues.

3 participants