Skip to content

Commit

Permalink
Fix rejection of variable declarations with type Opaque
Browse files Browse the repository at this point in the history
Ref. eng/recordflux/RecordFlux#633
  • Loading branch information
treiher committed Oct 8, 2024
1 parent 6153b63 commit 94b6399
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 226 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Rejection of invalid parameter types and return types in function declarations (eng/recordflux/RecordFlux#977)
- Consequential errors caused by undefined variables in binary expressions (eng/recordflux/RecordFlux#1672)
- Rejection of variable declarations with type `Opaque` (eng/recordflux/RecordFlux#633)

## [0.24.0] - 2024-09-12

Expand Down
44 changes: 2 additions & 42 deletions rflx/generator/state_machine.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
And,
AndThen,
Annotate,
Aspect,
Assignment,
Call,
CallStatement,
Expand Down Expand Up @@ -80,7 +79,6 @@
RecordType,
Selected,
Size,
SizeAspect,
Slice,
Statement,
String,
Expand Down Expand Up @@ -2918,7 +2916,7 @@ def always_true(_: ID) -> bool:
is_global,
declaration.location,
declaration.expression,
state_machine_global=state_machine_global,
state_machine_global,
)
if isinstance(declaration.type_, (ty.Message, ty.Sequence)):
has_composite_declarations |= True
Expand Down Expand Up @@ -3010,7 +3008,6 @@ def _declare( # noqa: PLR0912, PLR0913
is_global: Callable[[ID], bool],
alloc_id: Location | None,
expression: ir.ComplexExpr | None = None,
constant: bool = False,
state_machine_global: bool = False,
) -> EvaluatedDeclaration:
result = EvaluatedDeclaration()
Expand All @@ -3021,44 +3018,7 @@ def _declare( # noqa: PLR0912, PLR0913
location=expression.expr.location,
)

if type_ == ty.OPAQUE:
initialization = None
object_type: Expr = Variable(const.TYPES_BYTES)
aspects: list[Aspect] = []

if expression:
if expression.is_expr() and isinstance(expression.expr, ir.Agg):
e = expression.expr
if len(e.elements) == 0:
object_type = Slice(
Variable(const.TYPES_BYTES),
Last(const.TYPES_INDEX),
First(const.TYPES_INDEX),
)
initialization = None
if len(e.elements) > 0:
aspects.append(
SizeAspect(Mul(Number(len(e.elements)), Size(const.TYPES_BYTE))),
)
initialization = expression.expr
else:
fail(
"initialization not yet supported",
location=expression.expr.location,
)

result.global_declarations.append(
ObjectDeclaration(
[identifier],
object_type,
self._to_ada_expr(initialization, is_global) if initialization else None,
constant=constant if initialization else False,
aliased=False,
aspects=aspects,
),
)

elif isinstance(type_, (ty.UniversalInteger, ty.Integer, ty.Enumeration)):
if isinstance(type_, (ty.UniversalInteger, ty.Integer, ty.Enumeration)):
result.global_declarations.append(
ObjectDeclaration(
[identifier],
Expand Down
20 changes: 20 additions & 0 deletions rflx/model/state_machine.py
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,26 @@ def undefined_type(type_identifier: StrID, location: Location | None) -> None:
undefined_type(d.type_identifier, d.location)
d.type_ = ty.Any()

if isinstance(d, decl.VariableDeclaration) and d.type_ == ty.OPAQUE:
assert d.type_identifier.location is not None
self.error.push(
ErrorEntry(
"invalid variable type",
Severity.ERROR,
d.type_identifier.location,
annotations=(
[
Annotation(
"use a message with an opaque field instead",
Severity.HELP,
d.type_identifier.location,
),
]
),
generate_default_annotation=False,
),
)

if isinstance(d, decl.FunctionDeclaration):
for p in d.parameters:
parameter_id = type_decl.internal_type_identifier(
Expand Down
30 changes: 25 additions & 5 deletions tests/compilation/integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -930,17 +930,37 @@ def test_state_machine_message_field_access_in_transition(tmp_path: Path) -> Non


@pytest.mark.parametrize(
("global_decl", "local_decl", "value"),
("global_decl", "local_decl", "value", "error"),
[
("Key : Opaque := [0, 1, 0];", "", "Key"),
("", "Key : Opaque := [0, 1, 0];", "Key"),
("", "", "[0, 1, 0]"),
(
"Key : Opaque := [0, 1, 0];",
"",
"Key",
r"<stdin>:13:13: error: invalid variable type\n"
r"<stdin>:13:13: help: use a message with an opaque field instead\n"
r"<stdin>:22:17: error: comparisons of opaque fields not yet supported",
),
(
"",
"Key : Opaque := [0, 1, 0];",
"Key",
r"<stdin>:18:15: error: invalid variable type\n"
r"<stdin>:18:15: help: use a message with an opaque field instead\n"
r"<stdin>:22:17: error: comparisons of opaque fields not yet supported",
),
(
"",
"",
"[0, 1, 0]",
r"<stdin>:22:17: error: comparisons of opaque fields not yet supported",
),
],
)
def test_state_machine_comparing_opaque_values_in_comprehension(
global_decl: str,
local_decl: str,
value: str,
error: str,
tmp_path: Path,
) -> None:
spec = f"""\
Expand Down Expand Up @@ -980,6 +1000,6 @@ def test_state_machine_comparing_opaque_values_in_comprehension(
# TODO(eng/recordflux/RecordFlux#1497): Support comparisons of opaque fields
with pytest.raises(
RecordFluxError,
match=r"^<stdin>:22:17: error: comparisons of opaque fields not yet supported$",
match=rf"^{error}$",
):
utils.assert_compilable_code_string(spec, tmp_path)
174 changes: 0 additions & 174 deletions tests/unit/generator/state_machine_test.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import annotations

import typing
from dataclasses import dataclass
from functools import lru_cache
from typing import Callable, Sequence

Expand Down Expand Up @@ -482,166 +481,6 @@ def test_state_machine_evaluate_declarations(
)


@dataclass
class EvaluatedDeclarationStr:
global_declarations: str = ""
initialization_declarations: str = ""
initialization: str = ""
finalization: str = ""


@pytest.mark.parametrize(
("type_", "expression", "constant", "state_machine_global", "expected"),
[
(
ty.OPAQUE,
ir.ComplexExpr([], ir.Agg([])),
False,
False,
EvaluatedDeclarationStr(
global_declarations=(
"X : RFLX_Types.Bytes (RFLX_Types.Index'Last .. RFLX_Types.Index'First);"
),
),
),
(
ty.OPAQUE,
ir.ComplexExpr([], ir.Agg([])),
True,
False,
EvaluatedDeclarationStr(
global_declarations=(
"X : RFLX_Types.Bytes (RFLX_Types.Index'Last .. RFLX_Types.Index'First);"
),
),
),
(
ty.OPAQUE,
ir.ComplexExpr([], ir.Agg([ir.IntVal(1)])),
False,
False,
EvaluatedDeclarationStr(
global_declarations=(
"X : RFLX_Types.Bytes := (RFLX_Types.Index'First => RFLX_Types.Byte'Val (1))"
" with\n Size =>\n 1 * RFLX_Types.Byte'Size;"
),
),
),
(
ty.OPAQUE,
ir.ComplexExpr([], ir.Agg([ir.IntVal(1)])),
True,
False,
EvaluatedDeclarationStr(
global_declarations=(
"X : constant RFLX_Types.Bytes :="
" (RFLX_Types.Index'First => RFLX_Types.Byte'Val (1))"
" with\n Size =>\n 1 * RFLX_Types.Byte'Size;"
),
),
),
(
ty.OPAQUE,
ir.ComplexExpr([], ir.Agg([ir.IntVal(1), ir.IntVal(2)])),
False,
False,
EvaluatedDeclarationStr(
global_declarations=(
"X : RFLX_Types.Bytes := (RFLX_Types.Byte'Val (1), RFLX_Types.Byte'Val (2))"
" with\n Size =>\n 2 * RFLX_Types.Byte'Size;"
),
),
),
(
ty.OPAQUE,
ir.ComplexExpr([], ir.Agg([ir.IntVal(1), ir.IntVal(2)])),
False,
True,
EvaluatedDeclarationStr(
global_declarations=(
"X : RFLX_Types.Bytes := (RFLX_Types.Byte'Val (1), RFLX_Types.Byte'Val (2))"
" with\n Size =>\n 2 * RFLX_Types.Byte'Size;"
),
),
),
(
ty.OPAQUE,
ir.ComplexExpr([], ir.Agg([ir.IntVal(1), ir.IntVal(2)])),
True,
False,
EvaluatedDeclarationStr(
global_declarations=(
"X : constant RFLX_Types.Bytes :="
" (RFLX_Types.Byte'Val (1), RFLX_Types.Byte'Val (2))"
" with\n Size =>\n 2 * RFLX_Types.Byte'Size;"
),
),
),
(
ty.OPAQUE,
None,
False,
False,
EvaluatedDeclarationStr(
global_declarations=("X : RFLX_Types.Bytes;"),
),
),
(
ty.OPAQUE,
None,
False,
True,
EvaluatedDeclarationStr(
global_declarations=("X : RFLX_Types.Bytes;"),
),
),
(
ty.OPAQUE,
None,
True,
False,
EvaluatedDeclarationStr(
global_declarations=("X : RFLX_Types.Bytes;"),
),
),
],
)
def test_state_machine_declare(
type_: ty.Type,
expression: ir.ComplexExpr | None,
constant: bool,
state_machine_global: bool,
expected: EvaluatedDeclarationStr,
) -> None:
loc: Location = Location((1, 1))
allocator = AllocatorGenerator(dummy_state_machine(), Integration())

allocator._allocation_slots[loc] = 1 # noqa: SLF001
fsm_generator = FSMGenerator(
dummy_state_machine(),
Integration(),
allocator,
debug=Debug.BUILTIN,
)

result = fsm_generator._declare( # noqa: SLF001
ID("X"),
type_,
lambda _: False,
loc,
expression,
constant,
state_machine_global,
)
assert "\n".join(str(d) for d in result.global_declarations) == expected.global_declarations
assert (
"\n".join(str(d) for d in result.initialization_declarations)
== expected.initialization_declarations
)
assert "\n".join(str(s) for s in result.initialization) == expected.initialization
assert "\n".join(str(s) for s in result.finalization) == expected.finalization


@pytest.mark.parametrize(
("type_", "expression", "error_type", "error_msg"),
[
Expand All @@ -660,19 +499,6 @@ def test_state_machine_declare(
RecordFluxError,
r"initialization using function call not yet supported",
),
(
ty.OPAQUE,
ir.ComplexExpr(
[ir.Assign("X", ir.IntVal(0), INT_TY)],
ir.IntVar(
"X",
INT_TY,
origin=ir.ConstructedOrigin("X", Location((10, 20))),
),
),
RecordFluxError,
r"initialization not yet supported",
),
(
ty.Message("T"),
ir.ComplexExpr(
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/graph_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
BOOLEAN,
FINAL,
INITIAL,
OPAQUE,
Field,
Integer,
Link,
Expand All @@ -22,6 +21,7 @@
statement as stmt,
)
from rflx.rapidflux import Location, RecordFluxError
from tests.data import models


def assert_graph(graph: Dot, expected: str, tmp_path: Path) -> None:
Expand Down Expand Up @@ -222,7 +222,7 @@ def test_state_machine_graph(tmp_path: Path) -> None:
"STATE",
transitions=[Transition(target=ID("IGNORED_1")), Transition(target=ID("null"))],
actions=[stmt.VariableAssignment("Global", FALSE), stmt.Reset("Local")],
declarations=[decl.VariableDeclaration("Local", "Opaque")],
declarations=[decl.VariableDeclaration("Local", "TLV::Message")],
),
State(
"IGNORED_1",
Expand All @@ -231,7 +231,7 @@ def test_state_machine_graph(tmp_path: Path) -> None:
],
declarations=[decl.VariableDeclaration("Global", "Boolean")],
parameters=[],
types=[BOOLEAN, OPAQUE],
types=[BOOLEAN, models.tlv_message()],
)

expected_full = r"""
Expand Down
Loading

0 comments on commit 94b6399

Please sign in to comment.