Cppcoreguidelines: Consider recommending prefix increment/decrement unless postfix *semantics* are needed

[devjgm]

TL;DR

Prefix increment/decrement are both simpler for humans to think about and simpler to implement (see also #362). Therefore, I propose a core guideslines rule/recommendation of the form:

"Always prefer to call prefix increment/decrement unless postfix semantics are needed."

Rationale for this suggestion:

• Postfix increment/decrement is at best equally efficient to the prefix form, but sometimes the postfix form is less efficient. Therefore, efficiency favors the prefix form.
• The postfix form has more complicated semantics since it modifies a value but returns the unmodifed value. Therefore, simplicity favors the prefix form.
• If the result of the postfix expression is ignored (e.g., (void)x++;), then the postfix semantics were not needed. Therefore, the prefix semantics were sufficient for the use case.

Therefore, I claim that the only reason to ever call postfix increment/decrement is if the postfix semantics are needed. That is, callers should use the result of all postfix expressions; otherwise, they should have used the prefix form.

Example:

void F() {
  ...
  // The following expression slows down readers because they can clearly see that the 
  // postfix semantics were not needed, but now they have to hunt around to see the type
  // of `x` to know if the postfix expression is incurring an unnecessary copy.
  x++;  // BAD

  // Same effect as above, but the reader doesn't care whether `x` is `int` or some
  // complex iterator type
  ++x;  // GOOD
  ...
}

void G() {
  ...
  // Performance isn't in question here because the caller is using the result of the postfix expression
  // so it's reasonable to assume they need the postfix *semantics*.
  auto y = x++;  // GOOD
  ...
}

[xtofl]

I've had sooo many discussions to 'prove' this... I totally like this.

The most common place to find 'rogue' post-increments are loops.

for(auto i=0; i!=n; i++)...// Bad

[MikeGitb]

The most common place to find 'rogue' post-increments are loops.

And that is also the place, where it usually doesn't matter at all ...

I'm not against the guideline in general, but I think it needs a stronger, motivational example (preferably as concrete as possible)

[neithernut]

What about:

big_int a; // <- a really big integer type, e.g. a few kb in size
++a; // <- just increments
a++; // <- depending on the implementation, copies the big integer, just to throw it away afterwards

[devjgm]

Yes, an example like that works. But also, I'm not sure how strong the motivation needs to be because I think this advice has essentially no downside. I mean, there's no case where preincrement is worse if you didn't need the post-increment semantics. And if one does need the postincrement semantics, then there is no choice -- you need postincrement in that case.

To me, this seems like a small and simple nugget of guidance that should be relatively uncontroversial.

[cubbimew]

a really big integer type

I just gave it a try with a boost::multiprecision::cpp_int num("1000000000000000"):. On the random computer I ran this, a loop doing 1'000'000'000L post-increments takes 14.3 seconds, a loop doing the same number of preincrements takes 2.6 seconds. Seems like a viable demo.

[MikeGitb]

Yes, I think showing an actual type, where postfix increment does allocate and prefix doesn't sounds pretty convincing.

To me, this seems like a small and simple nugget of guidance that should be relatively uncontroversial.

The problem is not that it is controversial, but rather that such a check will probably be incredibly noisy when turned on on an existing code base.

[neithernut]

Well, post-increment only makes sense if the result is actually used. If we encounter a post-increment with an unused return-value, it should be safe to transform it into a pre-increment. In cases were the result _is_used, the author probably wanted the post-increment semantics. Maybe it could be avoided in some cases, but identifying those is (probably) generally expensive.

So maybe it's sufficient to detect post-increments where the result is not used. Maybe it's a good idea to mark post-increment operator overloads [[nodiscard]] in general.

[devjgm]

@MikeGitb yes, a check for this could be noisy, but I think the noise would have very few false positives. That is, any post-increment that is flagged by a check like this is almost certainly a case that could be easily (and automatically) "fixed" to use pre-increment.

I agree with the approach mentioned by @neithernut . That is, a check would only flag uses of postfix increment/decrement when the expression's return value is ignored, in which case that expression should be changed to prefix increment/decrement.

[hsutter]

Editors call: Thanks! Added as an Enforcement in P.9 instead of creating a whole item just for this narrow case.

[devjgm]

I just realized that the wording added only applies to "user-defined non-defaulted" postfix op++ and op--. That's different than the suggestion in this Issue. This issue was proposing to recommend prefix op++ and op-- always regardless of user-defined or defaulted. Postfix should only be used when the postfix semantics are needed, which is never the case when the return value is being ignored. The rational for this is described above.

I don't mean to argue with the decision, but I'm just checking to make sure this decision was made deliberately and not accidentally. Is the reason the advice only applies to user-defined non-default types to minimize noise from a tool (as suggested in the wording)?