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

uv_type: supress -Werror=unused compiler error #296

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

aloisklink
Copy link
Contributor

@aloisklink aloisklink commented Aug 21, 2023

The uv_type() constructor does not use its token parameter, which causes a -Werror=unused compiler error on GCC (and probably on other compilers too).

Luckily, C++17 adds the [[maybe_unused]] attribute that all standards compliant compilers will support to hide unused errors. Removed, see #296 (comment).


This fixes the error reported in #294 (comment), but I didn't want to add a Fixes: #294 into my commit or PR, since it seems like that issue is more about disabling all potential warnings using CMake, not just this single error.

@skypjack
Copy link
Owner

Oh, well, since the parameter name is the same of the type, I'd just drop the first one. As in:

explicit uv_type(loop::token, std::shared_ptr<loop> ref) noexcept

Rather than:

explicit uv_type([[maybe_unused]] loop::token token, std::shared_ptr<loop> ref) noexcept

@skypjack skypjack self-requested a review August 21, 2023 11:40
@skypjack skypjack self-assigned this Aug 21, 2023
The `uv_type()` constructor does not use its `token` parameter,
which causes a `-Werror=unused` compiler error on GCC (and probably
on other compilers too).

on-behalf-of: @nqminds <[email protected]>
@aloisklink
Copy link
Contributor Author

Oh, well, since the parameter name is the same of the type, I'd just drop the first one.

That makes sense! You only really need [[maybe_unused]] if you need to give the parameter a name (e.g. for Doxygen). I've fixed this in 619bf5d, and I've updated the commit/PR description to remove any references to [[maybe_unused]].

@skypjack
Copy link
Owner

Thank you (and sorry for the late reply, I'm out of home).

@aloisklink
Copy link
Contributor Author

Thank you (and sorry for the late reply, I'm out of home).

No worries! 2 days isn't really a late reply! I might take a while to reply too, since although we're generally allowed to submit patches to open-source, we're only allowed to do it in our personal time!

@skypjack skypjack merged commit 87e5440 into skypjack:main Aug 25, 2023
48 checks passed
@aloisklink aloisklink deleted the fix-Werror-unused branch August 26, 2023 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants