-
Notifications
You must be signed in to change notification settings - Fork 787
Don't use std::ignore
for unused parameters
#17219
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
Conversation
82a7af9
to
abfd946
Compare
abfd946
to
c38dde1
Compare
58b63a7
to
7ca2018
Compare
7ca2018
to
e269227
Compare
ping @intel/dpcpp-nativecpu-reviewers, @intel/sycl-graphs-reviewers, @intel/unified-runtime-reviewers, and/or @intel/unified-runtime-reviewers-level-zero |
`std::ignore` seems to have been adopted as some sort of ersatz `(void)` cast or `[[maybe_unused]]` annotation on unused parameters. Since the following are true: 1. The behavior of `std::ignore` outside of `std::tie` is not formally specified; 2. C++ already has builtin ways to express ignorance of a value; 3. Compiling `std::tie` is much more expensive than using the builtin alternatives [1]; 4. It's confusing to read; 5. It requires an extra header inclusion: I decided to simply omit the argument name in all cases where this is possible. This is a mostly mechanical transformation, using the following script: ``` find unified-runtime -name "*.[ch]pp" \ | xargs sed -i '/^\s*std::ignore\s*=\s*[a-zA-Z_][a-zA-Z0-9_]*;/d' \ && git ls-files -m | parallel clang-tidy \ '--checks="-*,misc-unused-parameters"' --fix --fix-errors -p build ``` followed by manual fixup around conditional compilation blocks. [1]: Both clang and gcc with libstdc++ and libc++ show similar slowdowns compiling the std::ignore vs void. I didn't try with MSVC but I see no reason it wouldn't be similar ``` $ ipython3 In [1]: def ignore(n): ...: ignores = 'std::ignore = x;\n' * n ...: return f'''#include <tuple>\nint foo(int x) {{ {ignores} \n return x;}}''' ...: In [2]: def void(n): ...: voids = '(void)x;\n' * n ...: return f'''int foo(int x) {{ {voids} \n return x;\n}}''' ...: In [3]: with open("ignore.cpp", "w") as f: ...: f.write(ignore(100000)) ...: In [4]: with open("voids.cpp", "w") as f: ...: f.write(void(100000)) ...: $ time c++ -S voids.cpp c++ -S voids.cpp 0.18s user 0.03s system 99% cpu 0.211 total $ time c++ -S ignore.cpp c++ -S ignore.cpp 4.50s user 0.34s system 99% cpu 4.836 total ```
c39bff0
to
18b01d9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Graphs/command-buffer changes LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NativeCPU looks good, thanks.
std::ignore
seems to have been adopted as some sort of ersatz(void)
cast or[[maybe_unused]]
annotation on unused parameters. Since the following are true:std::ignore
outside ofstd::tie
is not formally specified;std::tie
is much more expensive than using the builtin alternatives [1];This is a mostly completely mechanical transformation, using the following script:
Followed by manually introducting
[[maybe_unused]]
inNDEBUG
blocks in a few places.[1]: Both clang and gcc with libstdc++ and libc++ show similar slowdowns compiling the std::ignore vs void.
I didn't try with MSVC but I see no reason it wouldn't be similar