Skip to content

Fix function pointer subtyping - #1737

Open
kimjune01 wants to merge 1 commit into
flux-rs:mainfrom
kimjune01:fix/fnptr-subtyping
Open

Fix function pointer subtyping#1737
kimjune01 wants to merge 1 commit into
flux-rs:mainfrom
kimjune01:fix/fnptr-subtyping

Conversation

@kimjune01

@kimjune01 kimjune01 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Relates function pointer signatures using function subtyping instead of asserting structural equality. This handles alpha-renamed bound regions in #1718 and default refinement differences in #1185.

This follows the approach discussed in the Zulip thread on moving function subtyping into flux-infer.

Adds regression coverage for generic inputs and outputs, nested function pointers, higher-ranked lifetimes, multiple binders, mutable references, and unsafe function pointers. All new cases ICE on main and pass with this change.

Fixes #1718.
Fixes #1185.

@kimjune01
kimjune01 force-pushed the fix/fnptr-subtyping branch from 88ea13a to a5680f1 Compare August 28, 2026 03:08
@kimjune01
kimjune01 marked this pull request as ready for review August 28, 2026 03:16
@ranjitjhala

Copy link
Copy Markdown
Contributor

Thanks @kimjune01 !! I'll take a look first thing tomorrow!

@nilehmann

nilehmann commented Aug 31, 2026

Copy link
Copy Markdown
Member

This is the right approach to solve issues #1718 and #1184, but I'm a bit concerned that this duplicates the code for function subtyping.

It's a bit tricky to abstract this because we have to support subtyping with two levels of binders (EarlyBinder and Binder), and binders on the right have to be peeled out before binders on the left. I haven't given it a ton of cycles, but not having a convincing way to avoid code duplication is the main reason I haven't implemented function pointer subtyping.

@ranjitjhala

Copy link
Copy Markdown
Contributor

In the spirit of "not letting the perfect be the enemy of the good" I'd rather we merge and try to coalesce later.

@ranjitjhala

Copy link
Copy Markdown
Contributor

Plus Having two concrete chunks that we want to abstract/share likely makes it easier to do the refactor..

@nilehmann

Copy link
Copy Markdown
Member

Agree, but historically I've been the one doing these cleanups, and since I don't have much time to do it anymore, merging this PR means the code is going to stay like that unless someone else takes on the burden.

@ranjitjhala

ranjitjhala commented Aug 31, 2026 via email

Copy link
Copy Markdown
Contributor

@nilehmann

Copy link
Copy Markdown
Member

We want to refactor the two subtyping impls to be the same, yes?

Ideally, but it's tricky because we need to move things around, since check_fn_subtyping depends on stuff defined in refineck.rs. That function also uses a different reason for the generated constraints. But also, check_fn_subtyping does extra stuff like unfolding pointers, which may be overkill for function pointer subtyping. Unclear...

@ranjitjhala

Copy link
Copy Markdown
Contributor

Ah ok, so then we'd want to

  1. Pass the extra reason as a parameter?
  2. Just do the unfolding and then, if it turns out to be unnecessary, somehow "gate" it?

@nilehmann

Copy link
Copy Markdown
Member

Yes, but if we end up gating parts of the implementation, I don't know if that makes the overall code better.

@ranjitjhala

ranjitjhala commented Aug 31, 2026 via email

Copy link
Copy Markdown
Contributor

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.

ICE: assertion left == right failed ICE in infer.rs due to FnPtr subtyping

3 participants