-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: use type schemes in extension definitions wherever possible #678
Changes from all commits
fffb696
4ec9c75
7eb7bc8
3f3e5ea
8be903e
f5f994b
6355fa2
76b1995
1508ead
09dbb1b
3ddd6ab
c068c40
aeb7ef6
d93d5e6
b5159e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -331,6 +331,23 @@ impl Extension { | |
SignatureFunc::TypeScheme(type_scheme), | ||
) | ||
} | ||
|
||
/// Create an OpDef with a signature (inputs+outputs) read from e.g. | ||
/// declarative YAML; and no "misc" or "lowering functions" defined. | ||
pub fn add_op_type_scheme_simple( | ||
&mut self, | ||
name: SmolStr, | ||
description: String, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm, how about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All the other methods take String so I think I'd rather leave that for a |
||
type_scheme: PolyFuncType, | ||
) -> Result<&OpDef, ExtensionBuildError> { | ||
self.add_op( | ||
name, | ||
description, | ||
Default::default(), | ||
vec![], | ||
SignatureFunc::TypeScheme(type_scheme), | ||
) | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -100,6 +100,11 @@ mod test { | |
fn test_float_consts() { | ||
let const_f64_1 = ConstF64::new(1.0); | ||
let const_f64_2 = ConstF64::new(2.0); | ||
|
||
assert_eq!(const_f64_1.value(), 1.0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is unrelated extra test coverage - doesn't need your custom consts PR or anything? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, the commits are separate so i could cherry pick them, but its "related" in the sense that the changes in this PR highlight the remaining missing coverage better |
||
assert_eq!(*const_f64_2, 2.0); | ||
assert_eq!(const_f64_1.name(), "f64(1)"); | ||
assert!(const_f64_1.equal_consts(&ConstF64::new(1.0))); | ||
assert_ne!(const_f64_1, const_f64_2); | ||
assert_eq!(const_f64_1, ConstF64::new(1.0)); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍