refactor: Combine ExtensionSolutions (no separate closure)

[acl-cqc]

• infer::infer_extensions returns only a combined solution (for previously-open locations), after variables instantiated
• Hugr::infer_extensions writes (all parts of) the solution into place and returns it
• validate_with_extension_closure left in-place, with test demonstrating usage w/ sub-DFGs
• This should open the way (in future PRs) to changing implementation of infer::infer_extensions

[codecov]

Codecov Report

Attention: Patch coverage is 83.33333% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 85.49%. Comparing base (b05dd6b) to head (178f9c8).

Files	Patch %	Lines
hugr/src/extension/infer/test.rs	79.24%	3 Missing and 8 partials ⚠️
hugr/src/extension/infer.rs	83.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #884      +/-   ##
==========================================
- Coverage   85.53%   85.49%   -0.04%     
==========================================
  Files          78       78              
  Lines       14176    14220      +44     
  Branches    14176    14220      +44     
==========================================
+ Hits        12125    12158      +33     
- Misses       1423     1428       +5     
- Partials      628      634       +6     

Flag	Coverage Δ
rust	85.49% <83.33%> (-0.04%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[acl-cqc]

IIUC coverage is not reduced by the PR, just that the lines affected by the PR have lower-than-average coverage anyway, and AFAICS that is because of assert_matches!, so I think this is OK. (Run codecov/project shows this increases overall coverage by 0.04%.)

[croyzor]

Casting my mind back, I think the motivation for returning the open solution is that we might want to run extension inference on a subgraph before we add it to the main graph. I'm not really sure when this would be the case -- I think the case is that when doing replacements, we run inference on the complete graph before and after, so it's probably unnecessary

[acl-cqc]

Casting my mind back, I think the motivation for returning the open solution is that we might want to run extension inference on a subgraph before we add it to the main graph. I'm not really sure when this would be the case -- I think the case is that when doing replacements, we run inference on the complete graph before and after, so it's probably unnecessary

Ah-ha, I see. So the flow would have been:

1. Run inference on the subgraph. Write into the subgraph only the solutions that do not depend on the subgraph inputs.
2. Substitute the subgraph into the main graph.
3. Rerun inference on the main graph in order to figure out the solutions that did depend on the subgraph inputs.

Noting that if in step 1 we wrote in all the solutions, including those dependent on the subgraph inputs, then step 3 would fail because the inserted graph would have bad solutions (e.g. emptyset) that conflict.

This PR removes the ability to write only those solutions into the subgraph. However, the flow is largely intact if one does not write any solutions into the subgraph before insertion. (You can still validate the subgraph using the full set of solutions, if required.)

Close enough? I think I can realize that in a test.

[acl-cqc]

@croyzor added a much more thorough test which I hope covers that use-case (as well as we can - there'll be a better, but separate, solution if we do #709). I had to make infer_extensions(&Hugr) return only the new solutions rather than existing ones to get around assertions in gather_extensions. Can you take another look?

[croyzor]

Looks reasonable 👍

[croyzor]

Nice, let's update this once can give a better answer for polymorphic hugrs!