Skip to content

MAINT: crackfortran regex simplify#18072

Merged
charris merged 2 commits intonumpy:masterfrom
tylerjereddy:treddy_crackfortran_regex_simplify_1
Dec 27, 2020
Merged

MAINT: crackfortran regex simplify#18072
charris merged 2 commits intonumpy:masterfrom
tylerjereddy:treddy_crackfortran_regex_simplify_1

Conversation

@tylerjereddy
Copy link
Copy Markdown
Contributor

  • remove extraneous character class markers used in
    crackline_re_1: \w and = on their own have no
    benefit to character class [] inclusion

  • name_match has a character class that can be
    simplified because \w metacharacter already
    encompasses the digit metacharacter and the
    underscore

* remove extraneous character class markers used in
`crackline_re_1`: `\w` and `=` on their own have no
benefit to character class `[]` inclusion

* `name_match` has a character class that can be
simplified because `\w` metacharacter already
encompasses the digit metacharacter and the
underscore
Copy link
Copy Markdown
Member

@mattip mattip left a comment

Choose a reason for hiding this comment

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

Looks correct to me


dep_matches = {}
name_match = re.compile(r'\w[\w\d_$]*').match
name_match = re.compile(r'\w[\w$]*').match
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I suspect the original here was wrong.

A Fortran identifier must satisfy the following rules:
The first character must be a letter, The remaining characters, if any, may be letters, digits, or underscores, Fortran identifiers are case insensitive. That is, Smith, smith, sMiTh, SMiTH, smitH are all identical identifiers.

The first \w should probably be [A-Za-z]. The addition of \d_ is consistent with misunderstanding of \w not to include digits and underscores. The $ could probably be omitted, but it seems to work as is.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I revised based on your feedback

* `name_match` regular expression now starts by
matching a letter only, based on reviewer
feedback
@charris charris merged commit caa27a9 into numpy:master Dec 27, 2020
@charris
Copy link
Copy Markdown
Member

charris commented Dec 27, 2020

Thanks Tyler. Looks like $ is sometimes a valid character in names, so that is also correct.

As vendor extension, the dollar sign ($) is additionally permitted with the option -fdollar-ok, but not as first character and only if the target system supports it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants