Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue #3135. Type Bound procedures in Fortran submodules #3324

Closed
wants to merge 2 commits into from
Closed

Fix issue #3135. Type Bound procedures in Fortran submodules #3324

wants to merge 2 commits into from

Conversation

pdiener
Copy link
Contributor

@pdiener pdiener commented Mar 4, 2019

This fixes issue #3135 regarding the issue with using type bound
procedures in Fortran submodules. The fix consists of changing the
regex used in the scanner and the emitter to ignore lines starting
with:

module subroutine

and

module function

as these are used to define type bound procedures instead of modules
named 'subroutine' or 'function'. The regex is case insensitive.

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated master/src/CHANGES.txt directory (and read the README.txt in that directory)
  • I have updated the appropriate documentation

This fixes issue #3135 regarding the issue with using type bound
procedures in Fortran submodules. The fix consists of changing the
regex used in the scanner and the emitter to ignore lines starting
with:

module subroutine

and

module function

as these are used to define type bound procedures instead of modules
named 'subroutine' or 'function'.  The regex is case insensitive.
@bdbaddog
Copy link
Contributor

bdbaddog commented Mar 4, 2019

Isn't this a duplicate of #3137 ?
We still need a test to cover.

@pdiener
Copy link
Contributor Author

pdiener commented Mar 4, 2019 via email

# characters that make up the defined
# module name and save it in a group

def_regex = """(?i)^\s*MODULE\s+(?!PROCEDURE|SUBROUTINE|FUNCTION)(\w+)"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're fiddling with the regexes anyway, please make them raw strings. Python 3.8 is going to complain noisily otherwise - regex constructs containg \c where c is meaningful to re will take a warning that they're not valid Python escapes if it's not a raw string, like this:

SyntaxWarning: invalid escape sequence \s

@pdiener
Copy link
Contributor Author

pdiener commented Mar 4, 2019 via email

@bdbaddog
Copy link
Contributor

bdbaddog commented Mar 4, 2019

Wouldn't it be better to open a separete ticket for this as this should presumably be done consistently across SCons and not be limited to Fortran?

On Monday 2019-03-04 11:13, Mats Wichmann wrote: Date: Mon, 4 Mar 2019 11:13:24 From: Mats Wichmann @.> Reply-To: SCons/scons <reply+0028765c7f0e442e959d07187dd124da3693ec670d77f59f92cf0000000118951eb4 @.> To: SCons/scons @.> Cc: pdiener @.>, Author @.> Subject: Re: [SCons/scons] Fix issue #3135. (#3324) @mwichmann commented on this pull request. ____________________________________________________________________________ In src/engine/SCons/Scanner/Fortran.py: > - def_regex = """(?i)^\sMODULE\s+(?!PROCEDURE)(\w+)""" +# (?i) : regex is case insensitive +# ^\s : any amount of white space +# MODULE : match the string MODULE, case +# insensitive +# \s+ : match one or more white space +# characters +# (?!PROCEDURE|SUBROUTINE|FUNCTION) : but don't match if the next word +# matches PROCEDURE, SUBROUTINE or +# FUNCTION (negative lookahead +# assertion), case insensitive +# (\w+) : match one or more alphanumeric +# characters that make up the define d +# module name and save it in a group + + def_regex = """(?i)^\sMODULE\s+(?!PROCEDURE|SUBROUTINE|FUNCTION)(\w+)" "" If you're fiddling with the regexes anyway, please make them raw strings. Python 3.8 is going to complain noisily otherwise - regex constructs containg \c where c is meaningful to re will take a warning that they're not valid Python escapes if it's not a raw string, like this: SyntaxWarning: invalid escape sequence \s — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.[ACh2XCpqAUyDi-5P4-tDN-rfeXnaMKjHks5vTVQ0gaJpZM4bcs3s.gif]

@mwichmann has already resolved most/all of these. He's just trying to prevent any more being added.

@mwichmann
Copy link
Collaborator

Well... no, I haven't resolved most of them. It's just one less thing to think about later. There's an overall task for the Py 3.8 stuff, not a lot of specific tickets yet (3.8 isn't that imminent)

@pdiener
Copy link
Contributor Author

pdiener commented Mar 6, 2019 via email

@mwichmann
Copy link
Collaborator

I'm seeing them in this PR, nothing extra to do.

@bdbaddog
Copy link
Contributor

bdbaddog commented Mar 6, 2019

Okay, I have introduced raw strings in the regexes used in the Fortran scanner and emitter and committed that to the fortran_issue_3135 branch of my fork. I have tested that it still works with my build. Do I have to create another pull request or is this additional change automatically included?

On Monday 2019-03-04 16:50, Mats Wichmann wrote: Date: Mon, 4 Mar 2019 16:50:41 From: Mats Wichmann @.> Reply-To: SCons/scons <reply+0028765c3ac1e6e6ef565a4dcf2cf931362f8d82ab5023ff92cf0000000118956dc1 @.> To: SCons/scons @.> Cc: pdiener @.>, Author @.***> Subject: Re: [SCons/scons] Fix issue #3135. (#3324) Well... no, I haven't resolved most of them. It's just one less thing to think about later. There's an overall task for the Py 3.8 stuff, not a lot of specific tickets yet (3.8 isn't that imminent) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.[ACh2XMEd5VCd4y7AtMINphaIsGSarcD7ks5vTaNBgaJpZM4bcs3s.gif]

Thanks! Every push you do to this branch will update the Pull Request until we merge it.
Thankfully the workflow doesn't require new PR's or anything to complicated.

@bdbaddog bdbaddog changed the title Fix issue #3135. Fix issue #3135. Type Bound procedures in Fortran submodules Apr 24, 2019
@bdbaddog
Copy link
Contributor

Closing. I've pull your fixes into #3359 and added tests

@bdbaddog bdbaddog closed this Apr 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants