Adding strict order statements (closes #353)

[rupertford]

Some of the rules using blockbase have a strict order. This PR enforces this where it was not already used. The parser runs a bit faster as a result.

However, there is an additional case where we only want blockbase to allow 0 or 1 instances of a particular rule to match. At the moment that is not enforced and we can end up with multiple Specification_Part entries within a Main_Program if, for example, an implicit none is incorrectly added before a use statement. This is not a good outcome as tools might rightly rely on there only being one of these and we end up with strange errors, rather than it being caught by fparser in the first place. Note that on master we only get one Specification_Part but the implicit none and use statements are allowed to be in the wrong order.

[rupertford]

I've fixed the 0 or 1 instance problem but now break the addition of comments so I think I'll need to add a comment loop before incrementing the class pointer.

Note, there may be cases where it might be worth having a list of classes that are 0 or 1, as a subset of the classes might require this. I should look at this.

[rupertford]

There appears to be a noticable improvement in performance for the example benchmark ...

master: Time taken for parse = 36.27s Branch: Time taken for parse = 29.06s

[codecov[bot]]

Codecov Report

Base: 91.48% // Head: 91.25% // Decreases project coverage by -0.22% :warning:

Coverage data is based on head (9f0dcbc) compared to base (8df46ac). Patch coverage: 100.00% of modified lines in pull request are covered.

:exclamation: Current head 9f0dcbc differs from pull request most recent head 512cdfe. Consider uploading reports for the commit 512cdfe to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #358      +/-   ##
==========================================
- Coverage   91.48%   91.25%   -0.23%     
==========================================
  Files          37       37              
  Lines       13138    13134       -4     
==========================================
- Hits        12019    11986      -33     
- Misses       1119     1148      +29     

Impacted Files	Coverage Δ
src/fparser/two/Fortran2003.py	93.61% <ø> (-0.60%)	:arrow_down:
src/fparser/two/utils.py	95.47% <100.00%> (-0.12%)	:arrow_down:
src/fparser/two/Fortran2008.py	100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[rupertford]

When testing the latest version with LFRic algorithm code I am finding parse errors in code like ...

...
use x
#if defined(UM_PHYSICS)
use y
...

which is not correct (although PSyclone does not promise to support ifdef'ed code).

I guess it is because we are incrementing the class match without dealing with comments and ifdefs. I need to look at how to fix this. This probably related to the change in location of comments that is observed in tests (which I convinced myself was OK).

[rupertford]

Interestingly comments between use statements seem to be OK but ifdef's are not. I thought we processed these in the same way. I guess there must be a difference somewhere.

[rupertford]

OK, I now realise that comment, include and preprocess rules are added to the general blockbase rules, so if we do them in order they will not be picked up at the right point. I think we need to not add them to the rules and then consume all of them as we go along (without incrementing the class reference), but it needs thinking about.

[rupertford]

Comment restructuring seems to be working. All tests pass, LFRic code is being parsed OK with psyclone and psyclone tests pass after finding and fixing another bug in one of the examples.

Annoyingly, running psyclone on all the lfric algorithm codes does not seem to give any performance improvement.

[arporter]

Hi @rupertford, do you think you could add in a test that I wrote for #366 into this PR:

def test_do_loop_coments():
    """Check that comments around and within do loops appear in the expected
    places in the tree."""
    source = """\
program test
integer :: arg1
integer :: iterator
!comment_out 1
arg1 = 10
!comment_out 2
do iterator = 0,arg1
    !comment_in
    print *, iterator
end do
!comment_out 3
end program test"""
    reader = get_reader(source, isfree=True, ignore_comments=False)
    obj = Program(reader)
    comments = walk(obj, Comment)
    assert isinstance(comments[1].parent, Execution_Part)
    assert isinstance(comments[2].parent, TODO)

[rupertford]

Added test, which seems to work as expected, giving the following (which preserves comment locations) ...

PROGRAM test
  INTEGER :: arg1
  INTEGER :: iterator
  !comment_out 1
  arg1 = 10
  !comment_out 2
  DO iterator = 0, arg1
    !comment_in
    PRINT *, iterator
  END DO
  !comment_out 3
END PROGRAM test

[rupertford]

I'm not sure why I didn't make this ready for review before so here we go ...

[rupertford]

I started adding a test for strict_order and found that the implementation is not working correctly. For example

integer :: a
use my_mod

should not match with Specification_Part as use my_mod should precede integer :: a.

The rule order is ...

[Use_Stmt, Import_Stmt, Implicit_Part, Declaration_Construct]

However, integer :: a is returned as a match with use my_mod being ignored.