Skip to content

Fix missing source locations in Python → Laurel translation#843

Merged
keyboardDrummer merged 4 commits into
mainfrom
fix/842-add-missing-source-locations
Apr 13, 2026
Merged

Fix missing source locations in Python → Laurel translation#843
keyboardDrummer merged 4 commits into
mainfrom
fix/842-add-missing-source-locations

Conversation

@keyboardDrummer-bot
Copy link
Copy Markdown
Collaborator

Summary

Fixes #842 — Adds missing source location metadata in PythonToLaurel.lean so that diagnostics in the expected_laurel tests no longer report "unknown location".

Root Cause

Three places in PythonToLaurel.lean were producing nodes with defaultMetadata (which has FileRange.unknown) instead of passing through the source location from the Python AST:

  1. Return type constraint ensurespyFuncDefToPythonFunctionDecl used returns.ann (the Ann wrapper's annotation, which is {0,0} for DDM-generated optional fields) instead of retTy.ann (the actual return type expression's source range).

  2. For-loop assume statements — The PIn, Any_to_bool, and range-check Assume nodes in the For case of translateStmt used mkStmtExprMd (unknown location) instead of mkStmtExprMdWithLoc with the for-statement's md.

  3. Return statement — The LaurelResult assignment and Exit "$body" in the Return case used mkStmtExprMd instead of mkStmtExprMdWithLoc with the return-statement's md.

Changes

  • Strata/Languages/Python/PythonToLaurel.lean — 3 targeted fixes (described above)
  • StrataTest/Languages/Python/expected_laurel/*.expected — 15 test expectation files updated (all unknown location: prefixes replaced with actual file(line, col): locations)

Verification

  • lake build succeeds
  • run_py_analyze.sh passes all tests
  • grep "^unknown location:" expected_laurel/*.expected returns zero matches

Three fixes in PythonToLaurel.lean:

1. Return type constraint ensures: Use retTy.ann (the actual return type
   expression's source range) instead of returns.ann (the Ann wrapper's
   annotation, which is {0,0}).

2. For-loop assume statements: Use mkStmtExprMdWithLoc with md for the
   PIn/Any_to_bool assumes and range-based loop checks, instead of
   mkStmtExprMd which uses defaultMetadata (unknown location).

3. Return statement: Use mkStmtExprMdWithLoc with md for the LaurelResult
   assignment and exit statements.

Updated all expected_laurel test files to reflect the new source locations.

Fixes #842
Comment thread Strata/Languages/Python/PythonToLaurel.lean Outdated
Address review nit: use f.ann (the FunctionDef's own source range) instead
of returns.ann for the __init__ branch, avoiding the DDM wrapper's {0,0}
source range.
The merge from main brought in test_for_else_break and test_list_empty
which had 'unknown location' in their expected output. Regenerated these
files to reflect the correct source locations from our fix.
@keyboardDrummer-bot
Copy link
Copy Markdown
Collaborator Author

The CI failure was caused by the branch being behind main. Specifically, PR #823 ("Add synthetic Python tests covering many Python constructs") merged into main after this branch was created, adding new test expected files — test_for_else_break.expected and test_list_empty.expected — that contained unknown location strings (since they were generated without our source location fix).

Fix: Merged main into this branch and regenerated the two affected expected files so they now contain the correct source locations instead of unknown location.

Verified locally:

  • lake build succeeds
  • run_py_analyze.sh passes all tests (including the two newly updated ones)
  • grep "unknown location" expected_laurel/*.expected returns zero matches

@keyboardDrummer keyboardDrummer removed their assignment Apr 12, 2026
Copy link
Copy Markdown
Contributor

@MikaelMayer MikaelMayer left a comment

Choose a reason for hiding this comment

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

Thank you very much !!

Comment thread Strata/Languages/Python/PythonToLaurel.lean
@keyboardDrummer keyboardDrummer added this pull request to the merge queue Apr 13, 2026
Merged via the queue into main with commit b5879b9 Apr 13, 2026
17 checks passed
@keyboardDrummer keyboardDrummer deleted the fix/842-add-missing-source-locations branch April 13, 2026 13:26
keyboardDrummer-bot added a commit that referenced this pull request Apr 14, 2026
Conflicts in expected test output files resolved by combining:
- main's source location fixes from PR #843 (unknown location → actual locations)
- this PR's line number changes from explicit source location propagation
keyboardDrummer-bot added a commit that referenced this pull request Apr 14, 2026
Resolved conflicts in 13 expected_laurel test output files by
regenerating them after merging main (which includes source location
fixes from #843 and --keep-all-files changes from #840).
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.

Add missing source location information in Python -> Laurel translation

4 participants