Infer extension constraints on FuncDefn; remove many ExtensionSets from builder #739
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…hugr_classical/build_3not_hugr
acl-cqc
commented
Dec 13, 2023
| @@ -154,36 +155,25 @@ fn plus() -> Result<(), InferExtensionError> { | |||
| } | |||
|
|
|||
| #[test] | |||
| // This generates a solution that causes validation to fail | |||
There was a problem hiding this comment.
This comment was incorrect (it doesn't generate a solution - the error is CantInfer). But the body/subject of this test has largely switched with the identically-named test in validation.
acl-cqc
commented
Dec 13, 2023
| @@ -996,6 +986,7 @@ fn funcdefn_signature_mismatch() -> Result<(), Box<dyn Error>> { | |||
| result, | |||
| Err(ValidationError::CantInfer( | |||
| InferExtensionError::MismatchedConcreteWithLocations { .. } | |||
| | InferExtensionError::EdgeMismatch(ExtensionError::SrcExceedsTgtExtensions { .. }) | |||
There was a problem hiding this comment.
This one is a bit nondeterministic as to which error is reported
acl-cqc
commented
Dec 13, 2023
| let func = mod_builder.declare("test", FunctionType::new_endo(type_row![BOOL_T]).into())?; | ||
| let func = mod_builder.declare( | ||
| "test", | ||
| FunctionType::new_endo(type_row![BOOL_T]) |
There was a problem hiding this comment.
That this delta wasn't needed immediately following #734 is a kind of "bug" fixed by this PR
acl-cqc
commented
Dec 13, 2023
| @@ -781,7 +787,9 @@ mod tests { | |||
| let mut mod_builder = ModuleBuilder::new(); | |||
| let func = mod_builder.declare( | |||
| "test", | |||
| FunctionType::new(type_row![BOOL_T], type_row![BOOL_T]).into(), | |||
| FunctionType::new_endo(type_row![BOOL_T]) | |||
acl-cqc
commented
Dec 13, 2023
| input_wires: impl IntoIterator<Item = Wire>, | ||
| ) -> Result<DFGBuilder<&mut Hugr>, BuildError> { | ||
| let op = ops::DFG { | ||
| signature: signature.clone(), | ||
| }; | ||
| let nodetype = NodeType::new(op, input_extensions.clone()); | ||
| let nodetype = NodeType::new_auto(op); |
There was a problem hiding this comment.
The choice of new_auto here is about the only non-obvious thing in the PR. ("pure" for FuncDefn but "open" for any other DFG.) I think the next PR might even remove new_auto and that could be a good point to say #702 is finished
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follows #734. Big step towards #702 but maybe not the last word?
create_with_ioanddfg_builderno longer take an ExtensionSet, nor set extensions of the Input/Output node