-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Removed compiler warnings (unused variable). #484
base: master
Are you sure you want to change the base?
Conversation
Memory usage change @ 8486269
Click for full report table
Click for full report CSV
|
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.
Thanks for your PR! Change looks good to me. I added these parameters in 07b6bd1 and didn't think about the unused param warning, so good to get them fixed.
As you already say, #474 and #476 fix this exact same problem, but rather than removing the parameter name, they add a cast to void to suppress the warning (one using C-style cast, the other using a C++-style cast). Personally, I think removing the parameter names as this PR does is the cleanest, since it does not add extra stuff that only clutters the code.
It is sometimes useful to keep the parameter name in a comment, but in this case the parameter has no real value except for selecting the right version of the operator, which should be clear to anyone who knows about operator new and looks at this code, so I think it is fine to just omit the name entirely.
@per1234, @facchinm, I would propose merging this (trivial) change, and closing the others.
I just noticed #456, which is another fix for this, using |
|
This "patch" will result in the suppression of a number of warnings that are the result of unused parameters. In particular the following warnings are removed.
P.S. There are some more warnings about the use of binary constants (e.g.,
0b11111000
). I can also take care of them if you like.