fix(fortran): map source_location back through cpp linemarkers (#2092) by SinghAman21 · Pull Request #2105 · Graphify-Labs/graphify · GitHub
Skip to content

fix(fortran): map source_location back through cpp linemarkers (#2092) - #2105

Open
SinghAman21 wants to merge 2 commits into
Graphify-Labs:v8from
SinghAman21:fix/2092-fortran-cpp-source-location
Open

SinghAman21 wants to merge 2 commits into
Graphify-Labs:v8from
SinghAman21:fix/2092-fortran-cpp-source-location

Conversation

@SinghAman21

Copy link
Copy Markdown
Contributor

What

Capital-F Fortran files (.F .F90 .F95 .F03 .F08) are run through cpp,
which was invoked with -P. -P suppresses linemarkers, and since cpp blanks
directive lines, drops inactive #ifdef blocks, and collapses blank runs, the
line numbers tree-sitter sees no longer match the original file.

Result: every symbol got a wrong source_location — including files with no
preprocessor directives at all (blank-line collapse). A confidently wrong line
sends explain / affected / the MCP read surface to unrelated code.

How

  • Drop -P so cpp keeps its # <n> "<file>" linemarkers.
  • _cpp_preprocess now returns (source, line_map); _strip_linemarkers
    consumes the markers to build a preprocessed-line → original-line map and
    hands tree-sitter marker-free source (blanks kept, so rows stay in lockstep).
  • add_node / add_edge translate each line through _orig_line.
  • Unchanged: the -nostdinc -I /dev/null security flags, and the raw-bytes
    fallback for lowercase .f90 and when cpp is absent (line_map = None).

Verified on the issue's example: module solver → L9, compute_volume → L12,
zebrafunc (after 5 blank lines) → L6.

Files

  • graphify/extractors/fortran.py — run cpp without -P, parse linemarkers,
    translate node/edge lines.
  • tests/test_languages.py — 2 location tests (directive case + blank-line case).
  • tests/test_cpp_preprocess.py — updated for the new (bytes, line_map)
    signature; keeps the argument-injection check and adds a -P guard.
  • CHANGELOG.md — entry under 0.9.24 (unreleased).

Reference

Conclusion

Fortran symbols now report their true source line regardless of extension or
preprocessor directives. Full suite green (pre-existing unrelated
test_ollama_retry_cap.py failures aside).

@nothariharan nothariharan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Diagnosis is right and the bug is live. On current v8, module shapes on L6 is reported as L1 and compute_volume() on L9 as L4, so cpp -P is still mislocating every symbol in a capital-F file.

The problem is that the change does not currently do what it says it does, and it needs a rebase.

v8 has moved _cpp_preprocess on since this branch was cut: #include directives are stripped and the sanitized source is fed to cpp on stdin, to close the arbitrary-read vector. That is what the diff conflicts with (fortran.py, test_cpp_preprocess.py, test_languages.py).

Once resolved onto that version, the linemarker mapping turns out to be dead code. With stdin input cpp emits # 1 "<stdin>", and _strip_linemarkers compares the decoded marker path against str(path.resolve()), so in_target is False on every row and line_map comes back all zeros. _orig_line then falls through to the preprocessed line. I checked this directly against a rebuilt merge: the map comes back [0, 0, ...] while the reported locations are still correct, because dropping -P alone is enough to make cpp preserve the line count.

So the fix works, but by dropping -P rather than by the machinery, and the mapping is silently inert as shipped against v8. That matters because the linemarkers are precisely what is supposed to rescue a line-count-changing case, and right now nothing exercises that path.

Could you rebase onto v8 and pick one of:

  • match the stdin filename too (<stdin>, -) so the map is actually populated, or
  • drop _strip_linemarkers / _orig_line entirely and state in the comment that line preservation now comes from not passing -P.

Either is fine, I just do not want the second option to look like it is doing something it is not.

Also the CHANGELOG entry lands under ## 0.9.24 (2026-07-22), already released, so it needs the unreleased heading.

…ify-Labs#2092)

The preprocessor removes directives and inactive branches, so parser rows after them no longer identify the original Fortran lines. Keep cpp linemarkers and translate each emitted row back to the source line while continuing to pass sanitized source through stdin; passing a filename would re-open the include-read and option-injection risks addressed in current v8.
@SinghAman21
SinghAman21 force-pushed the fix/2092-fortran-cpp-source-location branch from bee3db8 to f4fc0fb Compare October 20, 2026 18:40
Add a real-cpp regression using #line to prove stdin linemarkers populate the original source map, rather than relying on preserved output row counts. Move the Graphify-Labs#2092 changelog entry into Unreleased.

This branch has not been deployed

No deployments
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.

2 participants