Skip to content

Fix char-selector parsing when the length is an expression containing a comma - #520

Merged
arporter merged 3 commits into
stfc:masterfrom
perazz:519_char_selector_comma_len
Jul 24, 2026
Merged

Fix char-selector parsing when the length is an expression containing a comma#520
arporter merged 3 commits into
stfc:masterfrom
perazz:519_char_selector_comma_len

Conversation

@perazz

@perazz perazz commented Jul 16, 2026

Copy link
Copy Markdown

Summary

Char_Selector.match splits the KIND and LEN parts of a char-selector on the top-level comma of the (placeholder-substituted) selector body. The length-first branch restored the placeholder-hidden sub-expressions with repmap() before building the child nodes, but the kind-first and positional branches did not. A length whose value contains a comma — e.g. an intrinsic call — was therefore left as the internal placeholder max(F2PY_EXPR_TUPLE_n), i.e. a MAX with a single argument.

Reproducer

module m
  implicit none
  integer, parameter :: ck = 1
contains
  subroutine s(n)
    integer, intent(in) :: n
    character(kind=ck, len=max(n, 0)), pointer :: tmp
  end subroutine s
end module m

On master this raises FortranSyntaxError: Intrinsic 'MAX' expects at least 2 args but found 1.

The failure only surfaces when every symbol in scope is resolved (all use … , only:). If the module has a wildcard USE, all_symbols_resolved is False, the intrinsic arg-count check defers instead of raising, and the same mis-parse silently leaks the F2PY_EXPR_TUPLE_n placeholder into the parse tree — so character(len=max(n,0), kind=ck) (length-first) always worked while character(kind=ck, len=max(n,0)) did not.

Fix

Call repmap() on the length and kind sub-expressions in the kind-first and positional branches, matching the existing length-first branch. Adds regression cases to test_char_selector.

Fixes #519.

Char_Selector.match splits the kind and length parts of a char-selector
on the top-level comma of the parenthesised expression that has been
processed by string_replace_map(). The length-first branch restored the
comma-hidden sub-expressions with repmap() before building the child
nodes, but the kind-first and positional branches did not. As a result a
length such as max(len, 0) was left as the placeholder max(F2PY_EXPR_
TUPLE_n), whose single argument makes the intrinsic argument-count check
report "Intrinsic 'MAX' expects at least 2 args but found 1" and reject
otherwise valid declarations, e.g.

    character(kind=CK, len=max(len, 0)), pointer :: tmp

The failure only surfaces once all symbols in scope are resolved; while a
wildcard USE leaves them unresolved the same mis-parse silently leaks the
placeholder into the parse tree instead of raising.

Apply repmap() in the kind-first and positional branches so the length
and kind sub-expressions are restored, matching the length-first branch,
and extend the Char_Selector test with the comma-containing length forms.
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.25%. Comparing base (fb85941) to head (febe466).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #520   +/-   ##
=======================================
  Coverage   92.24%   92.25%           
=======================================
  Files          89       89           
  Lines       13882    13886    +4     
=======================================
+ Hits        12806    12810    +4     
  Misses       1076     1076           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@arporter arporter left a comment

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.

All good, thanks very much for this fix @perazz. (A note for future - you can leave the changelog update as the reviewer does that as part of the final merge).

@arporter
arporter merged commit 743d453 into stfc:master Jul 24, 2026
6 checks passed
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.

character specifier mis-parses length when it is an expression containing a comma

2 participants