-
-
Notifications
You must be signed in to change notification settings - Fork 244
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
Fixes unused parameter warnings in the null VFD #1179
Conversation
fwiw, we should abandon the H5_ATTR_UNUSED scheme and replace it with (void) casts, which are less ridiculous and more portable. Then we can get rid of craziness like H5_ATTR_NDEBUG_UNUSED and H5_ATTR_PARALLEL_UNUSED and related. |
On Tue, Nov 09, 2021 at 03:01:19PM -0800, Dana Robinson wrote:
fwiw, we should abandon the H5_ATTR_UNUSED scheme and replace it with (void) casts, which are less ridiculous and more portable. Then we can get rid of craziness like H5_ATTR_NDEBUG_UNUSED and H5_ATTR_PARALLEL_UNUSED and related.
Generally I think that the attributes should be used sparingly, and I
definitely do not like the way that these attributes are spelled. I'd
like to lower the case, drop the _attr, and invert the double-negative
of _NDEBUG_UNUSED: h5_unused h5_debug_used h5_parallel_unused.
I appreciate the granularity of the existing attributes, though. Seems
like (void) casts cannot express "used by debug code."
Dave
|
Also agree with Dave, I think attributes are useful and we should keep them, casting to void is not good imo. We should just simplify them. FWIW you should have a look there: https://github.com/torvalds/linux/blob/master/include/linux/compiler_attributes.h |
I'll take a look at those that link and think about this some more. We can discuss at the next engineering meeting. I just want to avoid the macro combinatorial explosion as we started addressing warnings in the non-debug, parallel, etc. cases. |
In this case, though, we're not including H5private.h so the macros are not available. Using (void) casts in this instance might be the right choice even if we improve the "attribute unused" macro system for use in the library/tools/tests. |
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.
This change is fine for code that's external to the library.
No description provided.