Skip to content

performance-unnecessary-value-param false positive with iterator as template parameter #101181

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
firewave opened this issue Jul 30, 2024 · 3 comments
Labels
clang-tidy false-positive Warning fires when it should not

Comments

@firewave
Copy link

#include <list>

template<class Iterator>
void func(Iterator it)
{
}

void f()
{
    std::list<int> l;
    func(l.rbegin());
}
<source>:4:20: warning: the parameter 'it' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
    4 | void func(Iterator it)
      |                    ^
      |           const   &

https://godbolt.org/z/611TeePzP

@llvmbot
Copy link
Member

llvmbot commented Jul 30, 2024

@llvm/issue-subscribers-clang-tidy

Author: Oliver Stöneberg (firewave)

```cpp #include <list>

template<class Iterator>
void func(Iterator it)
{
}

void f()
{
std::list<int> l;
func(l.rbegin());
}


<source>:4:20: warning: the parameter 'it' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
4 | void func(Iterator it)
| ^
| const &

https://godbolt.org/z/611TeePzP
</details>

@vbvictor
Copy link
Contributor

I think that this is not false-positive if we strictly follow check conditions:

The check is only applied to parameters of types that are expensive to copy which means they are not trivially copyable or have a non-trivial copy constructor or destructor.

std::reverse_iterator which is returned from rbegin is not trivially copyable and trivially copy constructible
https://godbolt.org/z/5KK7zs171

@firewave
Copy link
Author

firewave commented Jan 12, 2025

Thanks for pointing that out.

Interestingly it is both when using libc++ - https://godbolt.org/z/bq5971onb.
Same with MSVC - https://godbolt.org/z/1zE96cYa8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang-tidy false-positive Warning fires when it should not
Projects
None yet
Development

No branches or pull requests

3 participants