Improve code standards according to Fortitude #298
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
That looks like a great tool!!! Seeing this is worrisome for me: Possibly also avoid the 23 externals unless we understand: And only 29 of these: And strange that we have "line-too-long" because we don't use flags to allow longer line lengths in ectrans. |
I checked each case and none seem to be genuine. Fortitude seems to struggle parsing files with preprocessor macros (not always).
Most of these refer to the
All fixed!
The linter defines "too long" as > 100 characters, just as a rule of thumb: |
|
Latest report: |
samhatfield
force-pushed
the
satisfy_linter
branch
from
72f4704 to
bdd701c
Compare
This commit fixes most violations of Fortitude rules: - C072: assumed-size-character-intent - OB061: deprecated-character-syntax - S071: missing-double-colon
MOD021: deprecated-relational-operator Checks for deprecated relational operators Fortran 90 introduced the traditional symbols for relational operators: `>`, `>=`, `<`, and so on. Prefer these over the deprecated forms `.gt.`, `.le.`, and so on.
Checks for local variables with implicit `save` Initialising procedure local variables in their declaration gives them an implicit `save` attribute: the initialisation is only done on the first call to the procedure, and the variable retains its value on exit. For example, this subroutine: ```f90 subroutine example() integer :: var = 1 print*, var var = var + 1 end subroutine example ``` when called twice: ```f90 call example() call example() ``` prints `1 2`, when it might be expected to print `1 1`. Adding the `save` attribute makes it clear that this is the intention: ```f90 subroutine example() integer, save :: var = 1 print*, var var = var + 1 end subroutine example ``` Unfortunately, in Fortran there is no way to disable this behaviour, and so if it is not intended, it's necessary to have a separate assignment statement: ```f90 subroutine example() integer :: var var = 1 print*, var var = var + 1 end subroutine example ``` If the variable's value is intended to be constant, then use the `parameter` attribute instead: ```f90 subroutine example() integer, parameter :: var = 1 print*, var end subroutine example ```
samhatfield
force-pushed
the
satisfy_linter
branch
5 times, most recently
from
a6c5a8e to
0b6ad1d
Compare
Checks for use of raw number literals as kinds
Rather than setting an intrinsic type's kind using an integer literal, such as
`real(8)` or `integer(kind=4)`, consider setting kinds using parameters in the
intrinsic module `iso_fortran_env` such as `real64` and `int32`. For
C-compatible types, consider instead `iso_c_binding` types such as
`real(c_double)`.
Although it is widely believed that `real(8)` represents an 8-byte floating
point (and indeed, this is the case for most compilers and architectures),
there is nothing in the standard to mandate this, and compiler vendors are free
to choose any mapping between kind numbers and machine precision. This may lead
to surprising results if your code is ported to another machine or compiler.
For floating point variables, we recommended using `real(sp)` (single
precision), `real(dp)` (double precision), and `real(qp)` (quadruple precision),
using:
```f90
use, intrinsic :: iso_fortran_env, only: sp => real32, &
dp => real64, &
qp => real128
```
Or alternatively:
```f90
integer, parameter :: sp = selected_real_kind(6, 37)
integer, parameter :: dp = selected_real_kind(15, 307)
integer, parameter :: qp = selected_real_kind(33, 4931)
```
Some prefer to set one precision parameter `wp` (working precision), which is
set in one module and used throughout a project.
Integer sizes may be set similarly:
```f90
integer, parameter :: i1 = selected_int_kind(2) ! 8 bits
integer, parameter :: i2 = selected_int_kind(4) ! 16 bits
integer, parameter :: i4 = selected_int_kind(9) ! 32 bits
integer, parameter :: i8 = selected_int_kind(18) ! 64 bits
```
Or:
```f90
use, intrinsic :: iso_fortran_env, only: i1 => int8, &
i2 => int16, &
i4 => int32, &
i8 => int64
```
samhatfield
force-pushed
the
satisfy_linter
branch
from
0b6ad1d to
db4dd9f
Compare
|
It seems you can configure Fortitude with the standard 132 line length too: |
|
Yeah, still some pretty long lines... |
|
This is cool: |
Long lines are more difficult to read, and may not fit on some developers' terminals. The line continuation character '&' may be used to split a long line across multiple lines, and overly long expressions may be broken down into multiple parts. The maximum line length can be changed using the flag `--line-length=N`. The default maximum line length is 100 characters. This is a fair bit more than the traditional 80, but due to the verbosity of modern Fortran it can sometimes be difficult to squeeze lines into that width, especially when using large indents and multiple levels of indentation. Some lines that are longer than the maximum length may be acceptable, such as long strings or comments. This is to allow for long URLs or other text that cannot be reasonably split across multiple lines. Note that the Fortran standard states a maximum line length of 132 characters, and while some modern compilers will support longer lines, for portability it is recommended to stay beneath this limit.
samhatfield
force-pushed
the
satisfy_linter
branch
from
3732bbb to
2349480
Compare
|
I think good to merge :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've been playing around with the Fortitude linter. This PR fixes code standards according to a few selected rules. I picked ones that could be implemented without changing too many lines of code. Gradually it would be nice to tick off more and more. This is the report after the commits in this branch: