Skip to content
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

Add pcl::utils::ignore function to remove Doxygen warnings for unused arguments #3942

Merged
merged 3 commits into from
Apr 20, 2020

Conversation

aPonza
Copy link
Contributor

@aPonza aPonza commented Apr 17, 2020

As I was discussing in #3932 (comment) adding PCL_UNUSED as it was was causing -Wunused-values in GCC, Clang and XCode. This solves the issue at least for the first two, so if XCode is ok with it I'm too.

@aPonza aPonza marked this pull request as draft April 19, 2020 15:13
@aPonza aPonza changed the title Simplify PCL_UNUSED for compilers other than MSVC Remove PCL_UNUSED macro in favour of ignore function Apr 19, 2020
@aPonza
Copy link
Contributor Author

aPonza commented Apr 19, 2020

OOM on windows

@aPonza aPonza marked this pull request as ready for review April 19, 2020 20:16
Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I expect a follow up PR touching all the (void) coming next?

If it's not too much bother, please create an issue anyways.

@kunaltyagi kunaltyagi added the changelog: new feature Meta-information for changelog generation label Apr 19, 2020
@kunaltyagi kunaltyagi changed the title Remove PCL_UNUSED macro in favour of ignore function Add ignore function to remove Doxygen warnings for unused arguments Apr 19, 2020
@kunaltyagi kunaltyagi added module: common needs: code review Specify why not closed/merged yet labels Apr 19, 2020
@SergioRAgostinho SergioRAgostinho merged commit 9e599c1 into PointCloudLibrary:master Apr 20, 2020
@SergioRAgostinho SergioRAgostinho removed the needs: code review Specify why not closed/merged yet label Apr 20, 2020
@aPonza aPonza deleted the unused_fix branch April 20, 2020 17:25
@taketwo
Copy link
Member

taketwo commented May 10, 2020

Sorry, I'm late for the party (just a bit 😆). Did you consider backporting std::ignore that is available in C++17?

@taketwo taketwo changed the title Add ignore function to remove Doxygen warnings for unused arguments Add pcl::utils::ignore function to remove Doxygen warnings for unused arguments May 10, 2020
@aPonza
Copy link
Contributor Author

aPonza commented May 11, 2020

I could really only find references saying not to use it because it's not its intended use (e.g. SO). Moreover from its syntax I wouldn't really bet on it working that well with Doxygen. Can you point me to a "do use it" source?

Also it seems to be some inline constexpr implementation in GCC which is from C++1z (inline variables in general are the new bit): link. Clang uses still an inline constexpr but with an unsigned char(?): link.

@taketwo
Copy link
Member

taketwo commented May 11, 2020

Can you point me to a "do use it" source?

I don't have any such source; only the logic that it's better to use standard solutions when possible. But actually, I was not aware that ignore has an affinity with tuples. I use it to silence "unused variable" warnings everywhere.

@kunaltyagi
Copy link
Member

kunaltyagi commented May 11, 2020

I create a global lambda static constexpr auto ignore = [](...) {}; and then ignore(var_a, var_b, var_c);

truhoang pushed a commit to truhoang/pcl that referenced this pull request Jun 30, 2020
…PointCloudLibrary#3942)

* remove PCL_UNUSED(x) in favour of ignore()
* remove PCL_UNUSED from codebase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: new feature Meta-information for changelog generation module: common
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants