Skip to content

checking: handle rigid variables in open-row instance matching#124

Open
bneiswander wants to merge 2 commits into
purefunctor:mainfrom
bneiswander:pr/checking-open-row-instance-matching
Open

checking: handle rigid variables in open-row instance matching#124
bneiswander wants to merge 2 commits into
purefunctor:mainfrom
bneiswander:pr/checking-open-row-instance-matching

Conversation

@bneiswander

Copy link
Copy Markdown
Contributor

checking: handle rigid variables in open-row instance matching

Base branch: main

Review Order

This PR is independent and can be reviewed against main at any time. It touches nearby row/matching internals, so reviewing it before the larger instance-chain PR may make later review easier, but it is not required.

Summary

  • Fix open-row instance matching when rows contain rigid variables.
  • Teach row matching to preserve rigid tail information instead of treating those rows as apart too aggressively.
  • Add coverage for an else-instance path that should remain available when an open row match contains a rigid variable.

Motivation

The analyzer produced false-positive instance errors for open-row programs accepted by PureScript. The issue showed up when matching instance heads involving row primitives such as Union, Cons, or Lacks against rows whose tails were rigid rather than ordinary inference variables.

Row matching needs to distinguish genuine apartness from a match that remains possible because a rigid open row may still satisfy the instance. Without that distinction, instance-chain selection can incorrectly fall through to an else branch or report ambiguity.

Motivating Example

The checker should not treat an open rigid row as immediately apart from an instance that can still match through the row tail:

class HasA (row :: Row Type)
instance hasARecord :: HasA (a :: Int | tail)
else instance hasAFallback :: HasA row

useOpenRow :: forall row. HasA (a :: Int | row) => Proxy row -> Unit
useOpenRow _ = unit

Before this change, similar open-row instance matches could be considered apart too aggressively.

Implementation Notes

  • Updates primitive row matching and row-list handling to account for rigid row variables.
  • Adjusts instance matching so else candidates are not selected merely because an open rigid row could not be decomposed immediately.
  • Keeps this PR independent from the row-fundep PR; the only conflict when exporting was an existing row snapshot, resolved to the expected output for this change.

Tests

Added fixture:

  • 1778704620_else_instance_open_row_match

Updated existing snapshot:

  • 1772440440_prim_row_open

Validation run locally:

cargo check -p checking --tests

@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2e06c7a2-63c8-4b3a-bc4a-444593612b81

📥 Commits

Reviewing files that changed from the base of the PR and between 3429efa and 8198efb.

📒 Files selected for processing (2)
  • compiler-core/checking/src/core/constraint/compiler/prim_row.rs
  • compiler-core/checking/src/core/constraint/compiler/prim_row_list.rs
🚧 Files skipped from review as they are similar to previous changes (2)
  • compiler-core/checking/src/core/constraint/compiler/prim_row_list.rs
  • compiler-core/checking/src/core/constraint/compiler/prim_row.rs

Summary

This PR fixes instance matching for open-row types containing rigid variables. Previously, instance patterns with rigid row variables were treated as non-matching candidates, leading to false-positive "no instance found" errors and incorrect fallback to else-instance branches even when an open rigid row could satisfy the instance requirement.

Changes

Constraint Compiler Updates:

  • Modified match_cons in prim_row.rs to defer handling (return Ok(None)) when the row or tail type argument is a rigid variable, preventing premature dismissal of potential matches
  • Updated match_row_to_list in prim_row_list.rs to defer when the row argument or its tail is a rigid type variable, allowing instance matching to proceed rather than attempting immediate row-list construction

Test Coverage:

  • Added new fixture test 1778704620_else_instance_open_row_match demonstrating correct instance selection for a MatchRow typeclass with overlapping instances and open record rows
  • Updated snapshot 1772440440_prim_row_open to reflect expected output from rigid variable handling

Validation

  • Locally validated with cargo check -p checking --tests
  • One snapshot conflict resolved to expected output
  • Designed to be independent of concurrent row-fundep changes

Merge Confidence

4/5

The PR addresses a genuine constraint-solving bug with targeted fixes and appropriate test coverage. The approach of deferring instance matching when encountering rigid variables is sound and prevents premature elimination of valid candidates. However, constraint compiler changes carry inherent risk for broader type system impact, and full validation would benefit from extended integration test coverage and verification of related constraint patterns.

Walkthrough

The pull request adds rigid variable detection to the constraint solver's row-type matchers. When match_cons or match_row_to_list encounter a rigid row or tail variable, they defer matching by returning Ok(None), allowing instance selection to proceed. A new integration test validates else-instance selection with overlapping typeclass instances and row-list constraints on a closed record.

Changes

Rigid row variable deferral and else-instance matching

Layer / File(s) Summary
Rigid detection in row cons and row-list matchers
compiler-core/checking/src/core/constraint/compiler/prim_row.rs, compiler-core/checking/src/core/constraint/compiler/prim_row_list.rs
match_cons and match_row_to_list now detect rigid row and tail variables via early guards and return Ok(None) to defer structural decomposition, preventing matchers from expanding rows that cannot be statically analysed.
Integration test for else-instance row matching
tests-integration/fixtures/checking/1778704620_else_instance_open_row_match/Main.purs
New test module defines a MatchRow typeclass with a generic fallback instance and a more specific "else instance" gated by RowToList. The test value for a closed record validates that else-instance selection is correctly enabled by rigid deferral.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description is directly related to the changeset, providing clear context about fixing open-row instance matching when rows contain rigid variables.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@bneiswander bneiswander changed the title fix(compiler): handle rigid variables in instance row matching checking: handle rigid variables in open-row instance matching May 15, 2026
Fix false NoInstanceFound errors when instances have open row patterns
(e.g., { | r }) that should match any row type including closed rows.

Three fixes:
1. match_row_type: treat rigid variables in instance patterns as matching any row
2. Row.Cons: defer to instance matching when arguments are rigid variables
3. RowToList: defer to instance matching when arguments or tails are rigid variables

Also commits pending fixture snapshots and adds test for open row instance matching.
Fix two snapshot mismatches on pr/checking-open-row-instance-matching:
- 1772440440: swap ambiguous Lacks error order to match actual output
- 1778704620: normalize source path to stable repo path
@bneiswander bneiswander force-pushed the pr/checking-open-row-instance-matching branch from b0dfee5 to 8198efb Compare May 19, 2026 01:12
@codecov

codecov Bot commented May 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.01%. Comparing base (b50beaa) to head (8198efb).

Files with missing lines Patch % Lines
...king/src/core/constraint/compiler/prim_row_list.rs 66.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #124      +/-   ##
==========================================
- Coverage   83.09%   83.01%   -0.08%     
==========================================
  Files         130      130              
  Lines       23812    23822      +10     
  Branches    23812    23822      +10     
==========================================
- Hits        19786    19777       -9     
- Misses       2238     2252      +14     
- Partials     1788     1793       +5     

☔ View full report in Codecov by Sentry.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

1 participant