Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Direct ranges in fields e.g. foo: 0..255 #174

Merged
merged 4 commits into from
Apr 19, 2023
Merged

Direct ranges in fields e.g. foo: 0..255 #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d5b2f94
Direct ranges in fields e.g. `foo: 0..255`
rooooooooob Apr 14, 2023
c2a2d32
integer range test coverage + fix for >isize::MAX ranges
rooooooooob Apr 17, 2023
cee12b7
run cargo fmt
rooooooooob Apr 17, 2023
e914c6f
clippy fixes
rooooooooob Apr 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 43 additions & 31 deletions src/parsing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,19 @@ fn parse_type_choices(
}
}

fn type2_to_number_literal(type2: &Type2) -> isize {
fn type2_to_number_literal(type2: &Type2) -> i128 {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was to fix an issue noticed when I tried to add u64::MAX as a bound to something and it was rolling over to -1 silently.

match type2 {
Type2::UintValue { value, .. } => *value as isize,
Type2::IntValue { value, .. } => *value,
Type2::FloatValue { value, .. } => *value as isize,
Type2::UintValue { value, .. } => *value as i128,
Type2::IntValue { value, .. } => *value as i128,
Type2::FloatValue { value, .. } => {
// FloatToInt trait still experimental so just directly check
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this was more of something copy-pasted that went unnoticed during the float PR that someone made. I don't think we'd want silent converting to integers as the preferred behavior there.

let as_int = *value as i128;
assert_eq!(
as_int as f64, *value,
"decimal not supported. Issue: https://github.com/dcSpark/cddl-codegen/issues/178"
);
as_int
}
_ => panic!(
"Value specified: {:?} must be a number literal to be used here",
type2
Expand Down Expand Up @@ -216,23 +224,23 @@ fn parse_control_operator(
match operator.operator {
RangeCtlOp::RangeOp { is_inclusive, .. } => {
let range_start = match type2 {
Type2::UintValue { value, .. } => *value as isize,
Type2::IntValue { value, .. } => *value,
Type2::FloatValue { value, .. } => *value as isize,
Type2::UintValue { value, .. } => *value as i128,
Type2::IntValue { value, .. } => *value as i128,
Type2::FloatValue { value, .. } => *value as i128,
_ => panic!("Number expected as range start. Found {:?}", type2),
};
let range_end = match operator.type2 {
Type2::UintValue { value, .. } => value as isize,
Type2::IntValue { value, .. } => value,
Type2::FloatValue { value, .. } => value as isize,
Type2::UintValue { value, .. } => value as i128,
Type2::IntValue { value, .. } => value as i128,
Type2::FloatValue { value, .. } => value as i128,
_ => unimplemented!("unsupported type in range control operator: {:?}", operator),
};
ControlOperator::Range((
Some(range_start as i128),
Some(range_start),
Some(if is_inclusive {
range_end as i128
range_end
} else {
(range_end + 1) as i128
range_end + 1
}),
))
}
Expand All @@ -247,31 +255,29 @@ fn parse_control_operator(
ControlOperator::CBOR(rust_type_from_type2(types, parent_visitor, &operator.type2))
}
token::ControlOperator::EQ => ControlOperator::Range((
Some(type2_to_number_literal(&operator.type2) as i128),
Some(type2_to_number_literal(&operator.type2) as i128),
Some(type2_to_number_literal(&operator.type2)),
Some(type2_to_number_literal(&operator.type2)),
)),
// TODO: this would be MUCH nicer (for error displaying, etc) to handle this in its own dedicated way
// which might be necessary once we support other control operators anyway
token::ControlOperator::NE => ControlOperator::Range((
Some((type2_to_number_literal(&operator.type2) + 1) as i128),
Some((type2_to_number_literal(&operator.type2) - 1) as i128),
Some(type2_to_number_literal(&operator.type2) + 1),
Some(type2_to_number_literal(&operator.type2) - 1),
)),
token::ControlOperator::LE => ControlOperator::Range((
lower_bound,
Some(type2_to_number_literal(&operator.type2) as i128),
Some(type2_to_number_literal(&operator.type2)),
)),
token::ControlOperator::LT => ControlOperator::Range((
lower_bound,
Some((type2_to_number_literal(&operator.type2) - 1) as i128),
)),
token::ControlOperator::GE => ControlOperator::Range((
Some(type2_to_number_literal(&operator.type2) as i128),
None,
)),
token::ControlOperator::GT => ControlOperator::Range((
Some((type2_to_number_literal(&operator.type2) + 1) as i128),
None,
Some(type2_to_number_literal(&operator.type2) - 1),
)),
token::ControlOperator::GE => {
ControlOperator::Range((Some(type2_to_number_literal(&operator.type2)), None))
}
token::ControlOperator::GT => {
ControlOperator::Range((Some(type2_to_number_literal(&operator.type2) + 1), None))
}
token::ControlOperator::SIZE => {
let base_range = match &operator.type2 {
Type2::UintValue { value, .. } => {
Expand Down Expand Up @@ -1121,7 +1127,7 @@ fn rust_type_from_type1(
));
ty.as_bytes()
}
Some(ControlOperator::Range(min_max)) => match &type1.type2 {
Some(ControlOperator::Range((low, high))) => match &type1.type2 {
Type2::Typename { ident, .. }
if ident.to_string() == "uint"
|| ident.to_string() == "int"
Expand All @@ -1130,15 +1136,21 @@ fn rust_type_from_type1(
|| ident.to_string() == "float32"
|| ident.to_string() == "float64" =>
{
match range_to_primitive(min_max.0, min_max.1) {
match range_to_primitive(low, high) {
Some(t) => t.into(),
None => panic!(
"unsupported range for {:?}: {:?}",
"unsupported range for {:?}: {:?}. Issue: https://github.com/dcSpark/cddl-codegen/issues/173",
ident.to_string().as_str(),
control
),
}
}
},
// the base value will be a constant due to incomplete parsing earlier for explicit ranges
// e.g. foo = 0..255
Type2::IntValue { .. } |
Type2::UintValue { .. } => range_to_primitive(low, high)
.map(Into::into)
.expect("unsupported ranges. Only integer ranges mapping to primitives supported. Issue: https://github.com/dcSpark/cddl-codegen/issues/173"),
_ => base_type,
},
Some(ControlOperator::Default(default_value)) => base_type.default(default_value),
Expand Down
12 changes: 6 additions & 6 deletions tests/core/input.cddl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,26 +54,26 @@ u8 = uint .size 1
u16 = uint .le 65535
u32 = 0..4294967295
u64 = uint .size 8 ; 8 bytes
i8 = -128..127
i8 = -128 .. 127
i64 = int .size 8 ; 8 bytes

signed_ints = [
u_8: uint .size 1,
u_8: 0 .. 255,
u_16: uint .size 2,
u_32: uint .size 4,
u_64: uint .size 8,
u_64: uint .le 18446744073709551615,
i_8: int .size 1,
i_16: int .size 2,
i_32: int .size 4,
i_32: -2147483648 .. 2147483647,
i_64: int .size 8,
n_64: nint
n_64: nint,
u64_max: 18446744073709551615,
; this test assumes i64::BITS == isize::BITS (i.e. 64-bit programs) or else the cddl parsing lib would mess up
; if this test fails on your platform we might need to either remove this part
; or make a fix for the cddl library.
; The fix would be ideal as even though the true min in CBOR would be -u64::MAX
; we can't test that since isize::BITS is never > 64 in any normal system and likely never will be
i64_min: -9223372036854775808
i64_min: -9223372036854775808,
]

default_uint = uint .default 1337
Expand Down