Remove unused variables#112
Conversation
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #112 +/- ##
=========================================
Coverage 28.76% 28.77%
=========================================
Files 248 248
Lines 72976 72954 -22
=========================================
- Hits 20995 20991 -4
+ Misses 51981 51963 -18
Continue to review full report at Codecov.
|
| real, dimension(4) :: u_tmp ! A temporary array of cell average properties [A] | ||
| real, parameter :: C1_12 = 1.0 / 12.0 | ||
| real :: dx, xavg ! Differences and averages of successive values of x [H] | ||
| real :: dx ! Differences and averages of successive values of x [H] |
There was a problem hiding this comment.
The comment on this line and below at lines 408 and 715 are now inconsistent with the declaration of a single variable.
| integer :: k, k_bot_min, k_top_max !< k-indices, min and max for bottom and top, respectively | ||
| integer :: k_bot_max, k_top_min !< k-indices, max and min for bottom and top, respectively | ||
| integer :: k_bot_diff, k_top_diff !< different between left and right k-indices for bottom and top, respectively | ||
| integer :: k, k_bot_min !< k-indices, min and max for bottom and top, respectively |
There was a problem hiding this comment.
k_top_max, k_top_min, and k_top_diff are in commented out code, and are reasonably described in the comments. Even though they are not used yet, perhaps they should be retained in comments.
Hallberg-NOAA
left a comment
Hallberg-NOAA
left a comment
There was a problem hiding this comment.
Thank you for eliminating the large number of unused variables in the MOM6 code. I have examined all of these changes, and I think that this will be ready to go as soon as 3 minor comments (listed separately) have been addressed.
This patch removes any unused variables detected by -Wunused-variables in GCC. This may include variables which are referenced in commented "zombie code". In some cases, the variables were preserved in comments, but not in all cases. If the zombie code is restored, it should be possible to reconstruct the original declaration. (All known cases were simple scalars, such as loop indices). In one case, a variable was conditionally used within a preprocessor `#ifdef __DO_SAFETY_CHECKS__` block. This patch moves the declaration into another `#ifdef` block, but perhaps it is preferable to remove the block. Certain documentation variables such as `mdl`, which typically contain the function name, have been removed if unused. The strongest motivation for this patch is to allow us to enable the `-Wall` flag, which includes unused variables, and will strengthen our ability to detect potential errors in the codebase.
|
I've made the changes recommended by @Hallberg-NOAA, and have also re-added the variable used by @ashao. |
Hallberg-NOAA
left a comment
Hallberg-NOAA
left a comment
There was a problem hiding this comment.
Thank you for eliminating all of these unused variables and moving around the content in comments as appropriate.
This PR should be accepted and merged in as soon as it passes the pipeline testing.
|
This PR passed the pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/jobs/77063, but with some irregularities in the units of a variable in the available_diags file stemming from another PR that was accepted earlier today. I am accepting this PR, but will look into that other issue. |
2 participants
This patch removes any unused variables detected by -Wunused-variables
in GCC.
This may include variables which are referenced in commented "zombie
code". In some cases, the variables were preserved in comments, but no
in all cases. If the zombie code is restored, it should be possible to
reconstruct the original declaration. (All known cases were simple
scalars, such as loop indices).
In one case, a variable was conditionally used within a preprocessor
#ifdef __DO_SAFETY_CHECKS__block. This patch moves the declarationinto another
#ifdefblock, but perhaps it is preferable to remove theblock.
Certain documentation variables such as
mdl, which typically containthe function name, have been removed if unused.
The strongest motivation for this patch is to allow us to enable the
-Wallflag, which includes unused variables, and will strengthen ourability to detect potential errors in the codebase.