Update cppguide

[zachfang]

Relate to RobotLocomotion/drake#22409

This change is 

[zachfang]

+@jwnimmer-tri for review as discussed in a Slack thread.
The second commit is an extra thing I crossed out but can be dropped if it doesn't makes sense contextually.

Reviewable status: LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers (waiting on @jwnimmer-tri)

[jwnimmer-tri]

feature.

For meaningful styleguide changes, we always tag the full platform team for review. However, I'd say these are just wordsmithing typos to adjust the guide to our already-evident reality, so don't need the full team.

So, for next steps, please open a Drake (draft) PR that bumps to this sha, so that we can preview the changes. Then, assign the platform reviewer on-call to platform review this PR.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: needs at least two assigned reviewers (waiting on @zachfang)

[zachfang]

+@ggould-tri for platform review, thanks!

Reviewable status: LGTM missing from assignee ggould-tri(platform) (waiting on @ggould-tri)

[ggould-tri]

as to the filesystem change.
I believe the exception change is potentially too controversial to evade whole-team review and and have commented it as such.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: 1 unresolved discussion (waiting on @zachfang)

---

cppguide.html line 1328 at r1 (raw file):

  <li>There is no easy way for constructors to signal errors, short of
  crashing the program (not always appropriate) or using exceptions
  <code class="nondrake">(which are <a href="#Exceptions">forbidden</a>)</code>.</li>

This is a potentially controversial change: Throwing an exception in a constructor is a matter of dispute, in part because it prevents the corresponding destructor from running; a nontrivial destructor (for instance one that manages global resources such as file handles or network sockets) would make an exceptionful constructor unsound.

(This is even more true if this ctor is called from another nontrivial ctor, as the caller will have some but not all of its members destroyed, and this member freed without destruction, which the author may not have planned for. See https://stackoverflow.com/questions/10212842/what-destructors-are-run-when-the-constructor-throws-an-exception and the Herb Sutter post it references for the reason that this is usually reasonable.)

So while "exceptions are forbidden" is no longer true, this paragraph is in the specific context of ctors where exceptions are at least something that would require broader discussion.

I don't think it belongs in the same PR as the extremely uncontroversial change below.

[jwnimmer-tri]

Reviewable status: 1 unresolved discussion (waiting on @ggould-tri)

---

cppguide.html line 1328 at r1 (raw file):

Previously, ggould-tri wrote…

This is a potentially controversial change: Throwing an exception in a constructor is a matter of dispute, in part because it prevents the corresponding destructor from running; a nontrivial destructor (for instance one that manages global resources such as file handles or network sockets) would make an exceptionful constructor unsound.

(This is even more true if this ctor is called from another nontrivial ctor, as the caller will have some but not all of its members destroyed, and this member freed without destruction, which the author may not have planned for. See https://stackoverflow.com/questions/10212842/what-destructors-are-run-when-the-constructor-throws-an-exception and the Herb Sutter post it references for the reason that this is usually reasonable.)

So while "exceptions are forbidden" is no longer true, this paragraph is in the specific context of ctors where exceptions are at least something that would require broader discussion.

I don't think it belongs in the same PR as the extremely uncontroversial change below.

More PRs is just hassle with no upside. Let's just keep everything in one place here.

I suppose the next step is to clarify the wording here beyond just the strikethrough. We throw from our constructor all the time (e.g., DRAKE_THROW_UNLESS to validate the ctor arguments) so it's important to not forbid it here. What's missing is a bit more text explaining the caveats -- the dtor will not run, so e.g. be sure to do all your throw-checking up front before starting into resource-claiming actions especially ones with out proper RAII.

[zachfang]

Reviewable status: 1 unresolved discussion (waiting on @ggould-tri and @jwnimmer-tri)

---

cppguide.html line 1328 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

More PRs is just hassle with no upside. Let's just keep everything in one place here.

I suppose the next step is to clarify the wording here beyond just the strikethrough. We throw from our constructor all the time (e.g., DRAKE_THROW_UNLESS to validate the ctor arguments) so it's important to not forbid it here. What's missing is a bit more text explaining the caveats -- the dtor will not run, so e.g. be sure to do all your throw-checking up front before starting into resource-claiming actions especially ones with out proper RAII.

I have added a note right after the strikethrough. Let me know if the wording needs tweaking. The rendered HTML looked good to me in terms of formatting.

[jwnimmer-tri]

Reviewed all commit messages.
Reviewable status: 1 unresolved discussion (waiting on @ggould-tri and @zachfang)

---

cppguide.html line 1328 at r1 (raw file):

Previously, zachfang wrote…

I have added a note right after the strikethrough. Let me know if the wording needs tweaking. The rendered HTML looked good to me in terms of formatting.

I am OK with it. It's not really enough to teach developers how throwing from constructors works (the member fields will still be destroyed correctly), but it at least reminds them to think about it.

We could link to maybe https://isocpp.org/wiki/faq/exceptions#selfcleaning-members but that's not the best exposition either.

We'll see what Grant thinks, and after the three of us agree we can cycle into the whole platform crew.

[ggould-tri]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all discussions resolved, LGTM from assignees ggould-tri(platform),jwnimmer-tri(platform) (waiting on @zachfang)

[jwnimmer-tri]

+@rpoyner-tri +@sammy-tri +@SeanCurtis-TRI +@sherm1 +@xuchenhan-tri for final approvals, please.

[jwnimmer-tri]

Reviewed 1 of 1 files at r2.
Reviewable status: LGTM missing from assignees rpoyner-tri(platform),sammy-tri(platform),SeanCurtis-TRI(platform),sherm1(platform),xuchenhan-tri (waiting on @rpoyner-tri, @sammy-tri, @SeanCurtis-TRI, @sherm1, and @xuchenhan-tri)

[sammy-tri]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: LGTM missing from assignees rpoyner-tri(platform),sammy-tri(platform),SeanCurtis-TRI(platform),sherm1(platform),xuchenhan-tri (waiting on @rpoyner-tri, @SeanCurtis-TRI, @sherm1, and @xuchenhan-tri)

[sammy-tri]

Reviewable status: LGTM missing from assignees rpoyner-tri(platform),SeanCurtis-TRI(platform),sherm1(platform),xuchenhan-tri (waiting on @rpoyner-tri, @SeanCurtis-TRI, @sherm1, and @xuchenhan-tri)

[SeanCurtis-TRI]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: LGTM missing from assignees rpoyner-tri(platform),sherm1(platform),xuchenhan-tri (waiting on @rpoyner-tri, @sherm1, and @xuchenhan-tri)

[sherm1]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: LGTM missing from assignees rpoyner-tri(platform),xuchenhan-tri (waiting on @rpoyner-tri and @xuchenhan-tri)

[xuchenhan-tri]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: LGTM missing from assignee rpoyner-tri(platform) (waiting on @rpoyner-tri)

[rpoyner-tri]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all discussions resolved, LGTM from assignees rpoyner-tri(platform),ggould-tri(platform),sammy-tri(platform),jwnimmer-tri(platform),SeanCurtis-TRI(platform),sherm1(platform),xuchenhan-tri (waiting on @zachfang)