CppCoreGuidelines icon indicating copy to clipboard operation
CppCoreGuidelines copied to clipboard

Parameter pass-by-value heuristic: trivial copyability issue, and incomplete type issue

Open hsutter opened this issue 2 years ago • 5 comments

F.16 says:

(Simple) ((Foundation)) Warn when a parameter being passed by value has a size greater than 2 * sizeof(void*). Suggest using a reference to const instead. (Simple) ((Foundation)) Warn when a parameter passed by reference to const has a size less or equal than 2 * sizeof(void*). Suggest passing by value instead.

Two problems that have come up with this advice:

  • (easy) It should also say "and is trivially copyable"
  • (harder) When the parameter is of an incomplete user-defined type (class, enum, union) then we can't tell its size and trivial copyability

hsutter avatar Jul 26 '23 22:07 hsutter

Is the (harder) enforcement needed? Is it true that incomplete types can't be passed by value so the rule wouldn't apply?

Also, would relaxing the enforcement some by only warning <= sizeof(void*) or > 8 * sizeof(void*) be good? Like it says in the Reason section, the cutoff depeds on the architecture. In most cases, it probably wont matter for the middle range.

bgloyer avatar Jul 27 '23 08:07 bgloyer

tl; dr I've misread, and didn't consider this suggestion is specifically for not taking ownership.

I think (easy) is a good explanation, but 2 * sizeof(void*) is too small. For example, types such as __mm512 are trivially copyable but 8 * sizeof(void*) bytes, and I don't think anyone would pass these by const&. To be fair, these are special because they are passed via a single zmm register, however, I also don't think most developers would recommend passing std::complex<long double> by const&, which is 4 * sizeof(void*) on x86_64 with 80-bit floats.

It's probably best to shift the line to approximately one cache line. For example, std::any in MSVC STL has a size of 64 bytes, which just barely fits into one x86_64 cache line.

std::string also has a size that varies between 24-32 bytes, but when taking ownership of a std::string, there is nothing wrong with passing it by value instead of using perfect forwarding or something. 2 * sizeof(void*) is definitely too strict, and doesn't match recommended practice in the C++ community. In general, passing containers by value is perfectly acceptable if one is taking ownership, even if 2 * sizeof(void*) is exceeded.

eisenwave avatar Jul 27 '23 12:07 eisenwave

I'm not well versed with calling conventions outside of x86_64, but the trend is definitely going towards more registers in general. IIRC RISC-V has 64 GPRs, and Intel's new APX extension is expanding the number of GPRs from 16 to 32. I'm unsure if this changes much about calling conventions, however, at a surface level, it is becoming more practical to pass larger objects by value.

My suggestion is:

-warn on > 2 * sizeof(void*) passed by value
+warn on > 8 * sizeof(void*) passed by value

and warn on <= 2 * sizeof(void*) passed by const& should stay untouched. This would leave everything from two to eight pointers as a case-base-case decision. This is the only sane solution imo, because the specific decision is arch-dependent, and also boils down to what registers are actually being used to pass the (sub)objects.

eisenwave avatar Jul 27 '23 12:07 eisenwave

std::string also has a size that varies between 24-32 bytes, but when taking ownership of a std::string, there is nothing wrong with passing it by value instead of using perfect forwarding or something. 2 * sizeof(void*) is definitely too strict, and doesn't match recommended practice in the C++ community. In general, passing containers by value is perfectly acceptable if one is taking ownership, even if 2 * sizeof(void*) is exceeded.

F.16 is about "in" parameters. This part of your suggestion appertains to F.18, which is about "will-move-from" parameters.

JohelEGP avatar Jul 27 '23 13:07 JohelEGP

  • (harder) When the parameter is of an incomplete user-defined type (class, enum, union) then we can't tell its size and trivial copyability

Note that enumerations can't be forward declared. So the Enforcement can always be apply to enumerations. For more, see the thread starting at https://github.com/hsutter/cppfront/commit/a89af8fcf1c258fb03ea17947eb76b1f6e410a25#r107567338.

JohelEGP avatar Sep 09 '23 00:09 JohelEGP