lfreist / hwinfo

cross platform C++ library for hardware information (CPU, RAM, GPU, ...)
MIT License
433 stars 74 forks source link

Return objects by value to exploit NRVO #70

Closed dario-loi closed 9 months ago

dario-loi commented 9 months ago

The problem

Some simple getter functions, such as the ones in gpu.h, return string attributes by const&, such as:

  HWI_NODISCARD const std::string& vendor() const;

This is an antipattern which results in potential pessimizations across the code, and it also introduces the possibility of dangerous dangling references.

Rationale

Strings should be returned by value since the C++ standard guarantees copy elision since C++17, this means that any function that returns an object by value will actually construct that object at the call site, avoiding any copies and saving destructor calls.

What happens currently is that a reference (pointer address) to an internally held string is returned, if this string's value is used to initialize another string, a copy will then occur (which will result in a call to strncpy. Moreover, since references do not implement direct ownership semantics, there is no way to know if the original owner of these strings has left the scope of the program or not, which means that if this reference is kept around, it could potentially become dangling at any time.

Hence in the current state there are two ways that these returns will be consumed:

  1. Directly, where a pass-by-value would be more efficient thanks to copy elision
  2. After some time, whereas they become more and more dangerous as the risk of becoming dangling increases

Solution

Simply unmarking all the strings as const& would immediately solve the problem.