Adds data models and builder functions for remaining constraints #238
Conversation
|
When I started self-reviewing it to write the PR tour comments, I hit upon a solution to one of the things that was bothering me. Don't bother reviewing this yet. |
| } | ||
|
|
||
| impl<V: IslVersion> TypeDefinitionBuilder<V> { | ||
| pub fn element(self, type_argument: VersionedTypeArgument<V>) -> Self { |
There was a problem hiding this comment.
🗺️ I chose to make this method available for both ISL versions, so that customers can switch from ISL 1.0 to 2.0 without having to refactor any existing code that does programmatic construction. There's also element_distinct with accepts a bool flag indicating whether the elements should be distinct or not.
| } | ||
|
|
||
| impl TypeDefinitionBuilder<ISL_2_0> { | ||
| pub fn field_names(self, type_argument: VersionedTypeArgument<ISL_2_0>) -> Self { |
There was a problem hiding this comment.
🗺️ Like element, field_names has two builder methods, but in this case, both are only available for ISL 2.0.
| // Sorting the vec allows us to get Bag-like equality semantics. | ||
| // TODO: Replace the vec with a HashBag or come up with a less hacky way to sort the type arguments. |
There was a problem hiding this comment.
Since this is used in a few places, consider making a naive Bag type wrapping a sorted Vec<TypeArgument> that you can optimize later.
| vec![AnyOf::new(vec![ | ||
| TypeArgument::from_type_ref( | ||
| NullabilityModifier::Nothing, | ||
| TypeReference::from("foo_type") | ||
| ), | ||
| TypeArgument::from_type_def( | ||
| NullabilityModifier::Nothing, | ||
| TypeDefinition::new(vec![], vec![]) | ||
| ), | ||
| TypeArgument::from_type_ref( | ||
| NullabilityModifier::Nothing, | ||
| TypeReference::imported("bar.isl", "baz") | ||
| ), | ||
| TypeArgument::from_type_ref( | ||
| NullabilityModifier::Nullable, | ||
| TypeReference::from("quux_type") | ||
| ), | ||
| ],) |
There was a problem hiding this comment.
These explicitly-constructed vec!s would be good candidates for the tuple-of-Into<_>s trick. (Not in this PR.)
There was a problem hiding this comment.
It could be, but I'm intentionally not using it here so that if there's something wrong with the tuple-of-Into<_>s logic, it won't silently pass these tests. In other words, I'm intentionally constructing these the verbose way in order to make the tests more thorough.
| use FloatingPointNumberFormat::*; | ||
| match text { | ||
| Some("binary16") => Ok(Binary16), | ||
| Some("binary32") => Ok(Binary32), | ||
| Some("binary64") => Ok(Binary64), | ||
| _ => invalid_schema_error(format!("Not a valid IEEE754 format symbol: {value}")), | ||
| } | ||
| } |
There was a problem hiding this comment.
I was curious to see if the compiler would do something clever with this, like lifting out the comparison of the string prefix. To my surprise, it did not.
Issue #, if available:
Description of changes:
Adds all the remaining constraints, except for the variants of
annotations(which still needs a little bit of work). Much of this PR is copy/paste from other constraints with some renaming done. It's probably fine to skim over them—just double check that there are no copy/paste mistakes where things should have been renamed but weren't.AnyOfandOneOfare copied fromAllOfNotis copied fromTypeConstraintScale,Precision,Exponent,ContainerLength,CodepointLength, andByteLengthare copied fromUtf8ByteLengthThese constraints are entirely new:
RegexTimestampOffsetContainsElementandFieldNamesIeee754FloatI also fixed the visibility of the fields in the
utf8_byte_lengthandtypeconstraints.Finally,
convert_to_patternwas madepub(crate)so that it could be reused in theRegexconstraint.There are some doc comments, but I plan to fill that out in a later PR to avoid blowing up the cope of this one.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.