Skip to content

Call elimination: requires violation diagnostic should point to call site#741

Merged
shigoel merged 18 commits intomainfrom
issue-531-call-elimination-requires-violation-diag
Apr 6, 2026
Merged

Call elimination: requires violation diagnostic should point to call site#741
shigoel merged 18 commits intomainfrom
issue-531-call-elimination-requires-violation-diag

Conversation

@MikaelMayer
Copy link
Copy Markdown
Contributor

@MikaelMayer MikaelMayer commented Apr 2, 2026

Fixes #531

Problem

During call elimination, the generated asserts for requires preconditions used only the call site metadata (md), discarding the requires clause's own metadata. This meant origin-based labels and the requires clause's file range were lost, producing generic diagnostic names like callElimAssert_requires_0_4 and no link back to the original requires clause location.

Fix

In createAsserts and createAssumes, use check.md.setCallSiteFileRange md instead of md alone. This:

  • Preserves the requires/ensures clause's metadata (including origin labels) as the base
  • Sets the call site as the primary file range (which was already the reported location)
  • Adds the original clause file range as a related file range for richer SARIF diagnostics

Testing

All existing tests pass. The correctness proofs (CallElimCorrect) compile without changes. A SARIF output test verifies that related locations are emitted correctly.

In CallElim's createAsserts, use setCallSiteFileRange to make the call
site the primary diagnostic location. The requires clause's original
location is preserved as a related file range for richer diagnostics.
Ensures clause metadata is also preserved as a related file range,
matching the pattern used in createAsserts and ProcedureInlining.
Copy link
Copy Markdown
Contributor Author

@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.

Clean, focused PR. The approach of reusing setCallSiteFileRange (already established in ProcedureInlining) is the right call — it keeps the pattern consistent across transforms. The correctness proofs compiling without changes is a good sign that the metadata change is semantics-preserving. One minor comment on comment consistency between the two functions.

Comment thread Strata/Transform/CoreTransform.lean
…n-diag

# Conflicts:
#	StrataTest/Languages/Python/expected_laurel/test_class_methods.expected
#	StrataTest/Languages/Python/expected_laurel/test_missing_models.expected
#	StrataTest/Languages/Python/expected_laurel/test_multi_function.expected
#	StrataTest/Languages/Python/expected_laurel/test_precondition_verification.expected
…n-diag

# Conflicts:
#	StrataTest/Languages/Python/expected_laurel/test_class_methods.expected
#	StrataTest/Languages/Python/expected_laurel/test_class_with_methods.expected
#	StrataTest/Languages/Python/expected_laurel/test_function_def_calls.expected
#	StrataTest/Languages/Python/expected_laurel/test_multi_function.expected
After merging main, the type constraint preconditions from #577 changed
diagnostic names from callElimAssert_requires_N to descriptive
(funcName requires) Type constraint of paramName format.
@MikaelMayer MikaelMayer marked this pull request as ready for review April 3, 2026 21:11
@MikaelMayer MikaelMayer requested a review from a team April 3, 2026 21:11
@MikaelMayer MikaelMayer enabled auto-merge April 3, 2026 21:11
Copy link
Copy Markdown
Contributor

@atomb atomb left a comment

Choose a reason for hiding this comment

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

This looks great. My one small request is the same as for #748: could you change the PR title to describe a change rather than describing a problem?

@shigoel shigoel added this pull request to the merge queue Apr 6, 2026
Merged via the queue into main with commit 742b1ba Apr 6, 2026
15 checks passed
@shigoel shigoel deleted the issue-531-call-elimination-requires-violation-diag branch April 6, 2026 01:33
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.

Call elimination: requires violation diagnostic should point to call site

4 participants