Skip to content

Commit

Permalink
Add style check for unsigned integers specified as range integers
Browse files Browse the repository at this point in the history
Ref. eng/recordflux/RecordFlux#1775
  • Loading branch information
andrestt committed Nov 6, 2024
1 parent ccf0857 commit 60dc679
Show file tree
Hide file tree
Showing 19 changed files with 240 additions and 236 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Added

- Style check for unsigned integer syntax (eng/recordflux/RecordFlux#1775)

## [0.25.0] - 2024-11-05

### Added
Expand Down Expand Up @@ -612,6 +618,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [0.1.0] - 2019-05-14

[Unreleased]: https://github.com/AdaCore/RecordFlux/compare/v0.25.0...HEAD
[0.25.0]: https://github.com/AdaCore/RecordFlux/compare/v0.24.0...v0.25.0
[0.24.0]: https://github.com/AdaCore/RecordFlux/compare/v0.23.0...v0.24.0
[0.23.0]: https://github.com/AdaCore/RecordFlux/compare/v0.22.0...v0.23.0
Expand Down
38 changes: 35 additions & 3 deletions rflx/model/type_decl.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,14 @@ def constraints(


class Integer(Scalar):
def __init__(
def __init__( # noqa: PLR0913
self,
identifier: StrID,
first: expr.Expr,
last: expr.Expr,
size: expr.Expr,
location: Location = NO_LOCATION,
allow_full_unsigned: bool = False,
) -> None:
super().__init__(identifier, size, location)

Expand Down Expand Up @@ -204,6 +205,29 @@ def simplify(expression: expr.Expr) -> expr.Expr:
),
)

if (
not allow_full_unsigned
and int(first_num) == 0
and int(last_num) == 2 ** int(size_num) - 1
):
assert self.location is not None
self.error.push(
ErrorEntry(
"unsigned integer syntax preferable",
Severity.ERROR,
self.location,
annotations=(
[
Annotation(
f'use "type {self.name} is unsigned {int(size_num)}" instead',
Severity.HELP,
self.location,
),
]
),
),
)

self.error.propagate()

self._first_expr = first
Expand Down Expand Up @@ -318,7 +342,8 @@ def __init__(
location=size.location,
),
size,
location,
location=location,
allow_full_unsigned=True,
)

def __str__(self) -> str:
Expand Down Expand Up @@ -695,7 +720,14 @@ def checked(
skip_verification: bool = False, # noqa: ARG002
workers: int = 1, # noqa: ARG002
) -> Integer:
return Integer(self.identifier, self.first, self.last, self.size, self.location)
return Integer(
self.identifier,
self.first,
self.last,
self.size,
location=self.location,
allow_full_unsigned=False,
)


@dataclass
Expand Down
29 changes: 15 additions & 14 deletions tests/compilation/integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def test_type_name_equals_package_name(definition: str, tmp_path: Path) -> None:
spec = """\
package Test is
type T is range 0 .. 2 ** 8 - 1 with Size => 8;
type T is unsigned 8;
type Test is {};
Expand Down Expand Up @@ -181,7 +181,7 @@ def test_sequence_with_imported_element_type_scalar(tmp_path: Path) -> None:
p.parse_string(
"""\
package Test is
type T is range 0 .. 255 with Size => 8;
type T is unsigned 8;
end Test;
""",
)
Expand Down Expand Up @@ -225,7 +225,8 @@ def test_sequence_with_imported_element_type_message(tmp_path: Path) -> None:
@pytest.mark.parametrize(
"type_definition",
[
"range 0 .. 2 ** 63 - 1 with Size => 63",
"unsigned 63",
"range 1 .. 2 ** 63 - 1 with Size => 63",
"range 2 ** 8 .. 2 ** 48 with Size => 63",
"(A, B, C) with Size => 63",
"(A, B, C) with Always_Valid, Size => 63",
Expand All @@ -249,7 +250,7 @@ def test_message_fixed_size_sequence(tmp_path: Path) -> None:
"""\
package Test is
type E is range 0 .. 2 ** 8 - 1 with Size => 8;
type E is unsigned 8;
type S is sequence of E;
Expand Down Expand Up @@ -291,7 +292,7 @@ def test_message_with_optional_field_based_on_message_size(tmp_path: Path) -> No
"""\
package Test is
type T is range 0 .. 2 ** 8 - 1 with Size => 8;
type T is unsigned 8;
type M is
message
Expand Down Expand Up @@ -321,7 +322,7 @@ def test_size_attribute(tmp_path: Path, aspects: str) -> None:
f"""\
package Test is
type T is range 0 .. 2 ** 8 - 1 with Size => 8;
type T is unsigned 8;
type M is
message
Expand All @@ -341,7 +342,7 @@ def test_message_size_calculation(tmp_path: Path) -> None:
"""\
package Test is
type T is range 0 .. 2 ** 16 - 1 with Size => 16;
type T is unsigned 16;
type Message is
message
Expand All @@ -364,7 +365,7 @@ def test_transitive_type_use(tmp_path: Path) -> None:
"""\
package Test is
type U8 is range 0 .. 2 ** 8 - 1 with Size => 8;
type U8 is unsigned 8;
type M1 is
message
Expand Down Expand Up @@ -436,7 +437,7 @@ def test_refinement_with_self(tmp_path: Path) -> None:
type Tag is (T1 => 1, T2 => 2) with Size => 8;
type Length is range 0 .. 2 ** 8 - 1 with Size => 8;
type Length is unsigned 8;
type Message is
message
Expand All @@ -463,7 +464,7 @@ def test_message_expression_value_outside_type_range(tmp_path: Path) -> None:
spec = """\
package Test is
type Length is range 0 .. 2 ** 8 - 1 with Size => 8;
type Length is unsigned 8;
type Packet is
message
Expand All @@ -484,7 +485,7 @@ def test_message_field_conditions_on_corresponding_fields(tmp_path: Path) -> Non
spec = """\
package Test is
type T is range 0 .. 2 ** 8 - 1 with Size => 8;
type T is unsigned 8;
type M is
message
Expand All @@ -507,7 +508,7 @@ def test_message_field_conditions_on_subsequent_fields(tmp_path: Path) -> None:
spec = """\
package Test is
type T is range 0 .. 2 ** 8 - 1 with Size => 8;
type T is unsigned 8;
type M is
message
Expand Down Expand Up @@ -601,7 +602,7 @@ def test_state_machine_type_conversion_in_message_size_calculation(tmp_path: Pat
spec = """\
package Test is
type Length is range 0 .. 2 ** 8 - 1 with Size => 8;
type Length is unsigned 8;
type Elems is range 0 .. 1 with Size => 8;
type Data is
Expand Down Expand Up @@ -646,7 +647,7 @@ def test_state_machine_move_content_of_opaque_field(tmp_path: Path) -> None:
spec = """\
package Test is
type Payload_Size is range 0 .. 2 ** 16 - 1 with Size => 16;
type Payload_Size is unsigned 16;
type M1 is
message
Expand Down
39 changes: 12 additions & 27 deletions tests/data/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
State,
StateMachine,
Transition,
UnsignedInteger,
)


Expand Down Expand Up @@ -77,11 +78,9 @@ def tlv_tag() -> Enumeration:


@lru_cache
def tlv_length() -> Integer:
return Integer(
def tlv_length() -> UnsignedInteger:
return UnsignedInteger(
ID("TLV::Length", location=Location((1, 1))),
Number(0),
Sub(Pow(Number(2), Number(16)), Number(1)),
Number(16),
location=Location((1, 1)),
)
Expand Down Expand Up @@ -160,21 +159,17 @@ def tlv_with_checksum_tag() -> Enumeration:


@lru_cache
def tlv_with_checksum_length() -> Integer:
return Integer(
def tlv_with_checksum_length() -> UnsignedInteger:
return UnsignedInteger(
"TLV_With_Checksum::Length",
Number(0),
Sub(Pow(Number(2), Number(16)), Number(1)),
Number(16),
)


@lru_cache
def tlv_with_checksum_checksum() -> Integer:
return Integer(
def tlv_with_checksum_checksum() -> UnsignedInteger:
return UnsignedInteger(
"TLV_With_Checksum::Checksum",
Number(0),
Sub(Pow(Number(2), Number(16)), Number(1)),
Number(16),
)

Expand Down Expand Up @@ -261,10 +256,8 @@ def null_message_in_tlv_message_model() -> Model:

@lru_cache
def ethernet_address() -> Integer:
return Integer(
return UnsignedInteger(
"Ethernet::Address",
Number(0),
Sub(Pow(Number(2), Number(48)), Number(1)),
Number(48),
)

Expand All @@ -286,10 +279,8 @@ def ethernet_tpid() -> Integer:

@lru_cache
def ethernet_tci() -> Integer:
return Integer(
return UnsignedInteger(
"Ethernet::TCI",
Number(0),
Sub(Pow(Number(2), Number(16)), Number(1)),
Number(16),
)

Expand Down Expand Up @@ -446,10 +437,8 @@ def enumeration_model() -> Model:

@lru_cache
def sequence_length() -> Integer:
return Integer(
return UnsignedInteger(
"Sequence::Length",
Number(0),
Sub(Pow(Number(2), Number(8)), Number(1)),
Number(8),
)

Expand Down Expand Up @@ -752,20 +741,16 @@ def universal_message_type() -> Enumeration:

@lru_cache
def universal_length() -> Integer:
return Integer(
return UnsignedInteger(
"Universal::Length",
Number(0),
Sub(Pow(Number(2), Number(16)), Number(1)),
Number(16),
)


@lru_cache
def universal_value() -> Integer:
return Integer(
return UnsignedInteger(
"Universal::Value",
Number(0),
Sub(Pow(Number(2), Number(8)), Number(1)),
Number(8),
)

Expand Down
2 changes: 1 addition & 1 deletion tests/data/specs.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
DEFINITE_MESSAGE_WITH_BUILTIN_TYPE_SPEC = """\
package Test is
type Length is range 0 .. 2 ** 7 - 1 with Size => 7;
type Length is unsigned 7;
type Message is
message
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/cli_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def test_parse_no_subsequent_errors_caused_by_style_errors(tmp_path: Path) -> No
"""\
package B is
type T is range 0 .. 255 with Size => 8;
type T is unsigned 8;
type M is
message
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/pyrflx_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ def test_simplification_of_div_expressions() -> None:
"""\
package Test is
type T is range 0 .. 2 ** 8 - 1 with Size => 8;
type T is unsigned 8;
type Test is
message
Expand Down
Loading

0 comments on commit 60dc679

Please sign in to comment.