I have C++ code that looks (simplified) like this:
#include <unistd.h>
#include <iostream>
#include <memory>
#include <string>
#include <thread>
std::thread the_thread;
void my_thread_func(const std::shared_ptr<std::string>& string_ptr) {
std::cout << "at thread begin: " << *string_ptr << std::endl;
sleep(1);
std::cout << "at thread finish: " << *string_ptr << std::endl;
}
void start_thread() {
auto local_string_ptr = std::make_shared<std::string>("foobar");
the_thread = std::thread(&my_thread_func, local_string_ptr);
}
int main(int, char**) {
start_thread();
sleep(2);
the_thread.join();
}
(on Compiler Explorer: https://godbolt.org/z/9GYTf7orz)
When I run clang-tidy --checks=performance-unnecessary-value-param
for this code, it warns:
<source>:10:50: warning: the parameter 'string_ptr' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
10 | void my_thread_func(std::shared_ptr<std::string> string_ptr) {
But my understanding is that passing string_ptr
as const reference to my_thread_func()
would be incorrect, since then the thread would have a reference to an object that is local to start_thread()
and that is destroyed while the thread is running.
My questions:
void my_thread_func(const std::shared_ptr<std::string>& string_ptr)
, or would that be an error?My experience is that clang-tidy is usually correct :-) that's why I'm confused about the warning here.
The clang-tidy warning assumes a single-threaded context: an ordinary call to a function
int get(std::shared_ptr<int> p,std::binary_semaphore &s) {
s.acquire();
return *p;
}
might very well need the std::shared_ptr
parameter (object, not reference) to keep the int
alive long enough to return it. (The caller might have obtained a reference to the std::shared_ptr
from some table rather than constructing it for the call, and whatever signals the semaphore might drop the only other reference immediately after doing so.)
Separately, even if the function called for a thread has a reference parameter, it will not be a reference to an argument to the std::thread
constructor: each such argument is copied into a temporary T whose lifetime is that of the thread, and the reference refers to T. (As mentioned in the comments, you can use std::ref
to produce a std::reference_wrapper
that can convert back into a reference to the original argument.) That means that you might not need a std::shared_ptr
here at all: you can just construct the object as an argument, and it will be moved into T.