320 nonstandard stop #451
320 nonstandard stop #451
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #451 +/- ##
=========================================
Coverage ? 92.14%
=========================================
Files ? 86
Lines ? 13834
Branches ? 0
=========================================
Hits ? 12747
Misses ? 1087
Partials ? 0 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thanks very much for this Joerg. Generally all looks good but I'd like a couple more tests to show that parsing STOP statements works in the context of a full parse and that the extension can be switched off. I'd also like some tests to show that the F2003 forms are still accepted by the F2008 parser.
Updated docs build fine (modulo the hundreds of existing warnings) and look good.
| Extended arguments for STOP | ||
| +++++++++++++++++++++++++++ | ||
|
|
||
| May compiler support extended arguments for the STOP statement before Fortran 2008. |
| # While non-standard, many compilers support negative numbers, and string | ||
| # operations in stop statements, e.g. `stop -1` or `stop str1//str2`. | ||
| # With this extension, these statements will be allowed. | ||
| _EXTENSIONS += ["extended-stop-format"] |
There was a problem hiding this comment.
Perhaps s/-format/-args/ here (and elsewhere) as "format" makes me think of Fortran FORMAT statements?
| @@ -2110,6 +2110,14 @@ def test_stop_stmt(): # R849 | |||
| assert isinstance(obj, tcls), repr(obj) | |||
There was a problem hiding this comment.
I'm afraid we're following the "you touch it, you fix it" approach here so please add a docstring for this test.
| """Test that invalid stop codes are handled.""" | ||
| with pytest.raises(NoMatchError) as err: | ||
| Stop_Code(string) | ||
| assert f"Stop_Code: '{string}'" in str(err.value) |
There was a problem hiding this comment.
Just to be on the safe side, please could you add a slightly higher-level test where a STOP statement is parsed in the context of e.g. a program. There have been times when I've added tests like this only to find that the new functionality didn't actually work in practise!
| @@ -2110,6 +2110,14 @@ def test_stop_stmt(): # R849 | |||
| assert isinstance(obj, tcls), repr(obj) | |||
| assert str(obj) == "STOP 'hey you'" | |||
|
|
|||
| obj = tcls('stop "123"//"456"') | |||
There was a problem hiding this comment.
Please could you also test when this extension is disabled.
| or scalar-int-constant-expr | ||
| """ | ||
|
|
||
| subclass_names = ["Scalar_Default_Char_Expr", "Scalar_Int_Expr"] |
There was a problem hiding this comment.
Does this still permit the Fortran2003 forms to match? Please add a test.
arporter
added
reviewed with actions
Fixes #320. Adds support for extended stop codes to Fortran 2003, and adds full 2008 support for (even more) flexible expressions.
I hope I did the 2008 part correctly. I esp. wasn't certain how to check if the expressions are constant :(