-
Notifications
You must be signed in to change notification settings - Fork 169
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
Fix conversion warnings #536
base: master
Are you sure you want to change the base?
Fix conversion warnings #536
Conversation
Obviously this is a subjective judgement. My feeling is that, if the original code is legal, then the extra code complexity isn't worth it. It would be ideal if were compiler options to turn off this kind of warning. But I don't know of any. |
Usually it is the other way around: You can turn these warnings on with compiler options. Build systems tend to activate every warning available, because these warnings are useful (in general). |
Yes, I think for this reason gfortran supports turning warnings off, like ( |
We are getting a lot of false-positive and spurious warning from our current CMake setup. See #387 for further details. |
Are those conversion warnings I fixed really false-positive? There are implicit conversions happening, so I think these are true-positive warnings. The question is, should we:
|
We are using Line 27 in 8bfcdf9
|
Okay, so do we want to leave the code as is and simply ignore the warnings? |
My suggestion would be to check our compiler warning options first and trim those down to the useful ones. Once we get rid of the 90% false positives, we fix the remaining warnings. |
I wonder if removing |
I did a quick test if we can leave out the |
Most of them are due to specific compiler flags. I would first remove the useless ones, and then check the remaining warnings. |
I agree with this. The next step would be to open a bug report with the compiler(s) that should not report false positives obfuscating the useful warnings. I don't think that we should bend stdlib to the needs of the compiler(s). |
What do you think about these changes?
pro
fixes 12 warnings (the remaining 12 warnings should be easy to fix, too), which are ~ 60 lines to
stderr
contra
complexity of code (readability, maintainability, etc.)
Maybe there are better options to fix the warnings, please let me know.