Fix crash on function-like macros with no arguments - #537
Conversation
A source file containing `#define ok() ...` made fortls abort with
re.PatternError: invalid group reference 10
and the file, along with every module it defines, silently failed to parse.
`expand_func_macro` splits the argument list with `def_args.split(",")`.
For a zero-argument macro the argument list is the empty string, and
`"".split(",")` returns `[""]` rather than `[]`, so the macro was treated as
having a single unnamed argument. The substitution loop then ran
re.sub(r"\b()\b", r"\1", sub)
whose pattern matches the empty string at every word boundary, so group
references were injected throughout the replacement text. In the reported
macro body `if(ie/=0) then; return; end if;` the reference landed immediately
before a digit, producing the literal `\10`, which `re` reads as a reference
to group 10 when the text is used as a substitution template.
Empty names are now dropped before the argument count is taken, so a
zero-argument macro compiles to `\bok\s*\(\)` and no substitution runs over
its body. Argument names are also escaped, which they were not before.
Adds tests for zero-, one- and two-argument macros.
Fixes fortran-lang#486
There was a problem hiding this comment.
Pull request overview
Fixes a fortls preprocessor crash triggered by zero-argument function-like macros (e.g., #define ok() ...) by correctly treating empty argument lists as arity 0 and safely escaping argument names during substitution.
Changes:
- Filter out empty argument names when expanding function-like macros to prevent erroneous substitution on
()and subsequentregroup-reference failures. - Escape macro argument names with
re.escapebefore building substitution patterns. - Add regression tests covering zero-, one-, and two-argument macro expansion; document the fix in the changelog.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
fortls/parsers/internal/parser.py |
Fixes macro argument parsing/substitution for zero-argument function-like macros and escapes argument names. |
test/test_preproc_parser.py |
Adds regression tests for zero-argument macros and arity handling. |
CHANGELOG.md |
Notes the crash fix and links to the reported issue. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| Regression test for #486. `"".split(",")` is `[""]`, not `[]`, so a macro | ||
| such as `ok()` was treated as having one unnamed argument. The argument | ||
| substitution then ran `\b()\b`, which matches at every word boundary, and | ||
| injected group references throughout the replacement text -- `ie/=0` became | ||
| `...\10`, and `re` raised "invalid group reference 10". |
There was a problem hiding this comment.
Good catch — the docstring was not raw, so it held a literal backspace instead of the text. Fixed in 17950af.
The docstring quotes the offending regex and the \10 group reference. In a normal string literal Python parses those as escape sequences, so the docstring held control characters rather than the text it was meant to show.
|
The red X here is not this change. All 8 failing jobs fail at the same single step, I re-ran the CI sequence locally against this branch on Python 3.13 to be sure the change itself is clean: Nothing needed from me here as far as I can tell, but happy to rebase if that would help. |
|
The red builds here are not from this change. On every failing job the only failing step is Upload coverage to Codecov — It is repo-wide rather than specific to this PR: the most recent Happy to rebase once CI is healthy if that would help. |
Problem
A file containing a function-like macro with no arguments makes fortls abort while preprocessing:
The file, and every module it defines, then silently fails to parse — which is why #486 reports that go-to-definition, hover and completion all stop working, including for code outside the module.
Minimal reproduction from the issue:
The reporter noticed that
#define ok ...works while#define ok() ...does not, which is the giveaway: the object-like and function-like paths differ.Cause
expand_func_macrosplits the argument list withdef_args.split(","). For a zero-argument macro the argument list is the empty string, and"".split(",")returns[""], not[]— so the macro is treated as having one unnamed argument.The substitution loop then runs:
That pattern matches the empty string at every word boundary, so group references get injected all through the replacement text. In the reported macro body one lands immediately before a digit in
ie/=0, producing the literal\10— whichrereads as a reference to group 10 when the text is later used as a substitution template. Hence the error, and hence why the value 10 looks arbitrary.Solution
Drop empty names before the argument count is taken. A zero-argument macro then compiles to
\bok\s*\(\)and no argument substitution runs over its body.Argument names are also passed through
re.escape, which they were not before.Testing
Two tests added to
test/test_preproc_parser.py:test_pp_zero_argument_function_macro— the issue's macro expands to its body instead of raisingtest_pp_function_macro_arities— zero, one and two argument macros all expand correctly, so the fix does not regress the argument pathBoth fail without the change. Full suite: 183 passed.
pre-commit(flake8, black, isort, pyupgrade) clean.One note: the two-argument expectation asserts
((1) + ( 2))with the leading space intact. Argument capture does not strip whitespace, which is pre-existing behaviour and left alone so this change stays about the crash.Fixes #486