General Improvements to umdp3 checker #234
Conversation
Pierre Siddall (Pierre-siddall)
left a comment
There was a problem hiding this comment.
Thanks R Sharp (@r-sharp), this looks good to me passing over to Sam Clarke-Green (@t00sa) for CR
|
Picking this up for review today |
Sam Clarke-Green (t00sa)
left a comment
There was a problem hiding this comment.
Looks good. I've got a question about whether the assumption that all pre-processor directives must start in column zero, but otherwise it's all fine.
| def remove_cpp_commands(line: str) -> str: | ||
| """Remove cpp commands from the lines : | ||
| There is a bit of an assumption here that quoted text has already been removed, so that we don't accidentally remove text after an "#" in a string. | ||
| Also that cpp commands have the '#' in col 1 of the line.""" |
There was a problem hiding this comment.
It might be safer to use a pattern than tolerates zero or more leading spaces, i.e. r"^\s*#.*$". It is not entirely clear, but it appears as though the C pre-processor will tolerate leading spaces provided the first non-whitespace character on a non-continuation line is a #.
This also assumes that there are no multi-line pre-processor directives in the codebase.
PR Summary
Sci/Tech Reviewer: Pierre Siddall (@Pierre-siddall)
Code Reviewer: Sam Clarke-Green (@t00sa)
Now that UMDP3 checker has been unleashed on an unsuspectin user base, some "real world" issues are arrising.
This change is intended to address the first few common ones to be highlighted...
l_switch = .FALSE.#endiftriggering "unseparated keywords" test.= [1, 2, 3, 4]") was triggering failures in the uppercase variable checkerAdditional unit tests have been added to ensure the problems identified above don't creep back in.
Prior to this change, the UMDP3 conformance checker identifies 779 files failing in the UM main branch.
After this change, that drops to 282 files. (of which a dozen are possibly false "obsolescent intrinsic" failures)
The issue with variables called LONG (and possibly others) being identified as a deprecated intrinsic, will most likely have to be dealt with by making changes to the UM itself.
Code Quality Checklist
Testing
Security Considerations
AI Assistance and Attribution
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review