Ensure standard-conformance and eliminate warnings #752
Comments
|
Thanks Damian @rouson - yes I completely agree that we should be standard conforming in fpm and I believe this is already something that has been unofficially enforced in PR reviews. I cannot (off the top of my head) think of any areas where we are not standard conforming. I can spot two extensions mentioned in your excerpt for line length and non-constant error stop code, are there any others that you've encountered? I notice you're compiling the single-source version here - have you tried compiling with Regarding warnings-as-errors, I'm hesitant to enforce this for developers based on the argument in this article, however it is something we could perhaps incorporate into our CI. The harder problem is the person-hours required to tackling the large number of warnings that already exist across the code-base and in our dependencies. I'm also unsure of how to deal with the well-known spurious gfortran warnings for allocation-on-assignment. Unfortunately, I don't have access to |
|
Maybe a bit more insightful building bb44917 with nagfor 7.1 and fpm 0.6.0: The error in the end is a compiler bug in nagfor 7.1, which should be reported to the NAG support. |
An internal compiler error is strictly a bug in the compiler, please report this to the Cray support. Given that Cray Fortran requires access to a Cray machine, there is nothing we can do to work around this bug in the compiler, unless you contribute a patch with such a workaround.
It's a nice idea but flawed in practice because most available implementation are imperfect to some degree. In case of a specific issue, this should be raised with the respective project and fixed if there is an actual standard violation present. If not this can be brought up with the compiler developers / support or worked around in the respective project.
I don't think this is a good idea, there are false positive warnings in many compilers, treating them as errors just puts an additional burden and unreasonable demand on the project maintainers (for fpm and its dependencies). |
Agreed. The first person to point this issue out to me was Cray support engineer who noted several indications of likely non-conformance based on the warning messages so Cray is aware of this. As I mentioned, I will investigate this further myself and submit pull requests. I have submitted several hundred compiler bugs on more than 6 compilers over more than 20 years. At the peak, I was submitting more than 50 bug reports annually. The most common cause of ICEs that I've observed were non-conforming code: the developer(s) wrote code in a manner that the compiler developers hadn't anticipated. The NAG compiler is developed by the Editor of the Fortran standard and was the first compiler to reach Fortran 90 compliance. The Cray compiler was the first to reach Fortran 2018 compliance. These are high-quality compilers. Causing an ICE in both should be a cause for concern.
This is a reasonably common practice -- nowhere near universal but not especially unusual. Ignoring 1, 2, or 10 warnings ultimately leads to ignoring 1,000 and eventually problems arise whether one recognizes that the problems were warned about or not. It's a sign of code smell. At a minimum, it would be far better to address the warnings on a case-by-case basis rather than to state a blanket opposition to addressing them. I will go through them in more detail and submit related pull requests. I've already observed some warnings that could be eliminated easily by deleting code the that is almost certainly pointless and must have been written by someone who didn't understand the language well. |
|
@LKedward thanks for your thoughtful response and especially for the link to such a great article -- one worth everyone reading. The caveats the author lists at the start of the article are important -- especially the first caveat: the author enforces a zero-warning policy on their projects. The issue about a zero-warning policy introducing a toolchain dependency is especially apropos in light of the spurious |
|
Tried to implement this in CI for TOML Fortran (toml-f/toml-f#112), the majority of issues were false positives due to automatic LHS reallocation, which are now worked around with a dummy allocation. |
|
I think issue #754 effectively supersedes this issue so I'm closing this one. |
|
What about the Cray failure? |
|
@awvwgk I might come back to the Cray issue, but
|
Description
I propose adopting the following two practices to
fpm's portability and robustness:This issue was inspired by my attempt to build fpm-0.6.0.F90 with the Cray Fortran compiler version 13.0.1. The result is an internal compiler error (ICE). In my experience, the most common cause of an ICE is non-conforming code.
The Numerical Algorithms Group (NAG) Fortran Compiler is widely considered the best compiler for checking standard conformance so I then tried building with NAG's compiler using the command
nagfor -fpp fpm-0.6.0.F90, where the-fppinvokes NAG's Fortran preprocessor. The result below contains a long list of warnings and compiler messages about questionable code followed by what appear to be four ICEs with NAG. As time permits, I'll investigate whether I can address these and submit related pull requests.Possible Solution
Additional Information
No response
The text was updated successfully, but these errors were encountered: