fix: case node should not have an external signature

[ss2165]

Previously it was incorrectly reporting the internal signature as the node signature.

Fixes #750

Required further fixes to replace.rs:

• test was looking for an error that should have been hidden behind another (that was not previously reported because of Case nodes have unconnected ports #750). Look for the latter, not the former.
• Fix node indices not being correctly reported from apply

[ss2165]

@acl-cqc This has induced a replace test to fail at two points:

hugr/src/hugr/rewrite/replace.rs

Line 684 in c48fe7a

assert_eq!(verify_res, apply_res);

and

hugr/src/hugr/rewrite/replace.rs

Lines 818 to 824 in c48fe7a

	assert_eq!(
	verify_apply(Replacement {
	mu_out: vec![new_out_edge.clone()],
	..r.clone()
	}),
	Err(ReplaceError::NoRemovedEdge(new_out_edge))
	);

The errors do not match the expected ones. I am having a hard time figuring out whether this is an actual regression or just that the expected error needs updating. Any pointers are appreciated.

Other tests are fortunately happy.

[acl-cqc]

@acl-cqc This has induced a replace test to fail at two points:

hugr/src/hugr/rewrite/replace.rs

Line 684 in c48fe7a

assert_eq!(verify_res, apply_res);

and

hugr/src/hugr/rewrite/replace.rs

Lines 818 to 824 in c48fe7a

	assert_eq!(
	verify_apply(Replacement {
	mu_out: vec![new_out_edge.clone()],
	..r.clone()
	}),
	Err(ReplaceError::NoRemovedEdge(new_out_edge))
	);

The errors do not match the expected ones. I am having a hard time figuring out whether this is an actual regression or just that the expected error needs updating. Any pointers are appreciated.

Other tests are fortunately happy.

Neither is obvious, but the error in verify_apply definitely suggests a bug - the two methods are reporting different errors, albeit, the same kind of error but at different node indices. I guess one may not be translating indices (as happens when the replacement subgraph is inserted) but the other is. That both are giving a different error than the expected one, I'm less sure - I can have a look when I'm back in a couple of days

[acl-cqc]

Ok, the failure in verify_apply is fixed by branch fix/replace-bad-edge-nums. Of course, really we should have a test of that case, i.e. returning such a BadEdgeKind.

Your change to Case indeed makes the current failure such a test (e.g. if we changed test_invalid to look for BadEdgeKind). However we are also missing a test of what that bit was supposed to test, i.e. a Replace that specifies a new out-edge in mu_out that does not correspond to an existing edge. That'll take a bit more test-writing....

[acl-cqc]

General thought on PR: inner_signature is good, but should this be a method specific to Case, does the same not apply to FuncDefn and also BasicBlock::DFG i.e. any DataflowContainer? (Even DFG - just in that case only, inner_signature is the same as dataflow_signature)

[acl-cqc]

Well a bunch of problems from my commits ;-) that clippy --fix should handle! But otherwise looks good, we should raise at least the issue that inner_signature is providing a case missing from HugrView's get_function_type, and possibly also that we've lost coverage of a Replace error (I mean, if we are surveying test coverage thoroughly+regularly enough then maybe that's redundant but)

[codecov]

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (056ea17) 84.48% compared to head (63758ba) 84.40%.

Files	Patch %	Lines
src/hugr/rewrite/replace.rs	63.63%	2 Missing and 2 partials ⚠️
src/extension/infer/test.rs	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #749      +/-   ##
==========================================
- Coverage   84.48%   84.40%   -0.09%     
==========================================
  Files          67       67              
  Lines       13222    13218       -4     
  Branches    13222    13218       -4     
==========================================
- Hits        11171    11157      -14     
- Misses       1270     1278       +8     
- Partials      781      783       +2     

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