CODEWITHSUNDEEP
c++auto-ptrcppcheck
Wolf
Wolf

Reputation: 10238

Misleading performance warning for auto_ptr type passed by value

I'm checking one of my projects with Cppcheck 1.75, and for this code (reduced for clarity):

class TJob
{
public:
    TJob(std::auto_ptr<ITask> task);
    // ...
private:
    std::auto_ptr<ITask> m_task;
};

TJob::TJob(
    std::auto_ptr<ITask> task // <-- HERE the performance warning
    ): m_task(task)
{
    trace("TJob ctr");
}

I'm getting this new performance warning:

Id: passedByValue

Summary: Function parameter 'task' should be passed by reference.

Message: Parameter 'task' is passed by value. It could be passed as a (const) reference which is usually faster and recommended in C++.

which is obviously a false positive. In my opinion it's a bug, because following this suggestion leads to a serious bug[1], but maybe there is a switch that I missed to set, or some template I can provide to declare that ownership is passed by auto_ptr?

I searched the web for this and the only I found so far is that Cppcheck incorporates some checks for Check for invalid usage of STL, for example

- using auto pointer (auto_ptr)

I'm aware, that auto_ptr isn't the best choice but wouldn't it be the same with unique_ptr? Could it be that two checks interfere here?

Besides using inline supression, is it possible to suppress warnings for those cases?

Edit: added the footnote.
[1] There is no serious bug, only a misunderstanding about using std::auto_ptr. I somehow mixed up compile-time with run-time semantics: the ownership isn't passed by some compile-time magic, but rather at runtime by invoking the "copy" constructor.

Upvotes: 1

Views: 402

Answers (1)

1stCLord
1stCLord

Reputation: 880

It has correctly detected that you are passing by value to a copy constructor when you could pass by reference and amortise one of the copies. Your optimiser will almost certainly fix the 'issue' for you but there is not real need to rely on it.

I would say your expression of the interface is preferable in the auto_ptr's case because it tells the caller that you are taking the pointer, however I would say that the core issue is actually that auto_ptr is a bad citizen (can be left in an invalid state by a copy operation), which is why it was deprecated.

Upvotes: 3

Related Questions