isocpp / isocpp/CppCoreGuidelines

"Lifetime Safety" paper copy editing comments

Open
#621 1 comment 0 reactions 1 assignee View on GitHub

@hsutter is already working on this.

Since Sep 26, 2016.

open
Dominant language
CSS
Stars
45.3k
Forks
5.6k
PR merge metrics
No merged PRs in 30d

Description

I don't know if you consider the stuff in docs/ to be "officially" part of the repo... but in case you do I have some copy-editing comments for the Lifetime Safety paper; there's also one substantive question I have about one of the rules. Feel free to do whatever.

Substantive question/comment

p17 says "do not throw a Pointer with lifetime other than static." If I am understanding these rules correctly and read them as-written, then:

  • A std::string contains a const char* (or moral equivalent) to some heap block, so it is a Pointer
  • Most/all standard exception types contain a std::string (or perhaps a const char*), so they are all Pointers
  • I should thus not throw any standard exception types, or anything else containing a dynamically-generated or dynamically-stored string, except perhaps by saying static std::runtime_error the_exception; throw the_exception;

Am I missing something?

Wording suggestions
  • p8. Second "note" sidebar, "The only way to obtain an owner<> is from new" could maybe continue with "or by transferring ownership from another owner<>." (Or maybe "moving" instead of transferring?)
  • p10. Very near top. "whose lset depends on" (2x). I found "depends on" to be confusing wording. Maybe "contains" and "has a non-empty intersection with" for the two occurrences, respectively? Assuming I'm understanding it well enough anyway.
  • General statement; p19 7.2 is the first time it comes up. I think it's worth an explicit mention that (assuming I'm even supposing correctly here) that [[lifetime]] annotations are not really overriding the rules in the same way that, say, [[suppress]] would be. Maybe something like "[[lifetime]] attributes do not imply that the analyzer is leaving code unchecked; code that uses [[lifetime]] is still fully-checked on both sides of the interface. The purpose of [[lifetime]] is to provide information to the analyzer that would be impractical or impossible for it to gather on its own, and allow it to efficiently check each function in isolation rather than your entire program at once."
Errors, assuming I'm understanding things correctly
  • p8. The nittiest of nitpicks -- in the three "notes" bullet points 1/4 down the page, (i) (anything, invalid) == invalid should probably be {anything, invalid} == {invalid} for consistency, and (ii) there is a black instead of blue brace in the first bullet at lset(p) == {a, null}
  • p8. Example 1.1: I think int i = 0; // lset(i) = {i} should be lset(i) = {}
  • p9. Example 1.3: int* a[100]; int* p = a[0]; // lset(p) = {a} should be something else; presumably int** p = &a[0]; to match the lset comment.
  • p16. The second of four bullets near the start of section 5: "A branch of the form if(a || b) {...} is treated as if (a) { if(b) {} else {...} }" should be... something else. This was reported as #101 by vladtepesch; (s)he suggested if (a) {...} else if (b) {...}"
  • p22. Example 8.3. "See examples later in the paper, notably 8.2 and 10", concerning [[lifetime(lset)]]. However, I cannot discern much a connection to 8.2 (which is also earlier), though perhaps it's intended.
  • p24. Example 8.9, first code snippet. delete buf; I think should be delete buf.data();
  • p25. Example 9. owning<int*> should be owner<int*>.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.