Skip to content

Commit

Permalink
Add error message for obscure case with parameters. (#214)
Browse files Browse the repository at this point in the history
Prior to this change, if a user omitted the length of an integer
parameter *and* used that parameter as an argument to an operator or
function, the compiler with throw an `AttributeError` instead of
providing a useful message.

This is because the check that integer runtime parameters have an
explicit size happened later than the function in `expression_bounds.py`
that tried to use the size.

This change moves the check earlier (incidentally spliting
`check_constraints` into `check_constraints` and
`check_early_constraints`), and also gives it a better error message.
  • Loading branch information
reventlov authored Dec 18, 2024
1 parent 35220b7 commit 75bdc4c
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 26 deletions.
71 changes: 55 additions & 16 deletions compiler/front_end/constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,49 @@ def _check_type_requirements_for_field(
)


def _check_early_type_requirements_for_parameter_type(
runtime_parameter, ir, source_file_name, errors
):
"""Checks that the type of a parameter is valid."""
physical_type = runtime_parameter.physical_type_alias
logical_type = runtime_parameter.type
size = ir_util.constant_value(physical_type.size_in_bits)
if logical_type.which_type == "integer":
# This seems a little weird: for `UInt`, `Int`, etc., the explicit size
# is required, but for enums it is banned. This is because enums have
# a "natural" size (the size specified in the `enum` definition, or 64
# bits by default) in expressions, so the physical size would just be
# ignored. Integer types do not have "natural" sizes, so the width is
# required.
if not physical_type.HasField("size_in_bits"):
errors.extend(
[
[
error.error(
source_file_name,
physical_type.source_location,
f"Parameters with integer type must have explicit size (e.g., `{physical_type.atomic_type.reference.source_name[-1].text}:32`).",
)
]
]
)
elif logical_type.which_type == "enumeration":
if physical_type.HasField("size_in_bits"):
errors.extend(
[
[
error.error(
source_file_name,
physical_type.size_in_bits.source_location,
"Parameters with enum type may not have explicit size.",
)
]
]
)
else:
assert False, "Non-integer/enum parameters should have been caught earlier."


def _check_type_requirements_for_parameter_type(
runtime_parameter, ir, source_file_name, errors
):
Expand Down Expand Up @@ -346,22 +389,7 @@ def _check_type_requirements_for_parameter_type(
)
)
elif logical_type.which_type == "enumeration":
if physical_type.HasField("size_in_bits"):
# This seems a little weird: for `UInt`, `Int`, etc., the explicit size is
# required, but for enums it is banned. This is because enums have a
# "native" 64-bit size in expressions, so the physical size is just
# ignored.
errors.extend(
[
[
error.error(
source_file_name,
physical_type.size_in_bits.source_location,
"Parameters with enum type may not have explicit size.",
)
]
]
)
pass
else:
assert False, "Non-integer/enum parameters should have been caught earlier."

Expand Down Expand Up @@ -686,6 +714,17 @@ def _attribute_in_attribute_action(a):
return {"in_attribute": a}


def check_early_constraints(ir):
errors = []
traverse_ir.fast_traverse_ir_top_down(
ir,
[ir_data.RuntimeParameter],
_check_early_type_requirements_for_parameter_type,
parameters={"errors": errors},
)
return errors


def check_constraints(ir):
"""Checks miscellaneous validity constraints in ir.
Expand Down
48 changes: 39 additions & 9 deletions compiler/front_end/constraints_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@
from compiler.util import test_util


def _make_ir_from_emb(emb_text, name="m.emb"):
def _make_ir_from_emb(emb_text, name="m.emb", stop_before_step="check_constraints"):
ir, unused_debug_info, errors = glue.parse_emboss_file(
name,
test_util.dict_file_reader({name: emb_text}),
stop_before_step="check_constraints",
stop_before_step=stop_before_step,
)
assert not errors, repr(errors)
return ir
Expand Down Expand Up @@ -1252,8 +1252,12 @@ def test_checks_constancy_of_constant_references(self):
error.filter_errors(constraints.check_constraints(ir)),
)

def test_checks_for_explicit_size_on_parameters(self):
ir = _make_ir_from_emb("struct Foo(y: UInt):\n" " 0 [+1] UInt x\n")
def test_checks_for_explicit_integer_size_on_parameters(self):
ir = _make_ir_from_emb(
"struct Foo(y: UInt):\n" #
" 0 [+1] UInt x\n",
stop_before_step="check_early_constraints",
)
error_parameter = ir.module[0].type[0].runtime_parameter[0]
error_location = error_parameter.physical_type_alias.source_location
self.assertEqual(
Expand All @@ -1262,12 +1266,34 @@ def test_checks_for_explicit_size_on_parameters(self):
error.error(
"m.emb",
error_location,
"Integer range of parameter must not be unbounded; it "
"must fit in a 64-bit signed or unsigned integer.",
"Parameters with integer type must have explicit size (e.g., `UInt:32`).",
)
]
],
error.filter_errors(constraints.check_constraints(ir)),
error.filter_errors(constraints.check_early_constraints(ir)),
)

def test_checks_for_explicit_integer_size_on_parameters_and_uses_type_in_error(
self,
):
ir = _make_ir_from_emb(
"struct Foo(y: Int):\n" #
" 0 [+1] UInt x\n",
stop_before_step="check_early_constraints",
)
error_parameter = ir.module[0].type[0].runtime_parameter[0]
error_location = error_parameter.physical_type_alias.source_location
self.assertEqual(
[
[
error.error(
"m.emb",
error_location,
"Parameters with integer type must have explicit size (e.g., `Int:32`).",
)
]
],
error.filter_errors(constraints.check_early_constraints(ir)),
)

def test_checks_for_correct_explicit_size_on_parameters(self):
Expand All @@ -1292,7 +1318,11 @@ def test_checks_for_correct_explicit_size_on_parameters(self):

def test_checks_for_explicit_enum_size_on_parameters(self):
ir = _make_ir_from_emb(
"struct Foo(y: Bar:8):\n" " 0 [+1] UInt x\n" "enum Bar:\n" " QUX = 1\n"
"struct Foo(y: Bar:8):\n" #
" 0 [+1] UInt x\n"
"enum Bar:\n"
" QUX = 1\n",
stop_before_step="check_early_constraints",
)
error_parameter = ir.module[0].type[0].runtime_parameter[0]
error_size = error_parameter.physical_type_alias.size_in_bits
Expand All @@ -1307,7 +1337,7 @@ def test_checks_for_explicit_enum_size_on_parameters(self):
)
]
],
error.filter_errors(constraints.check_constraints(ir)),
error.filter_errors(constraints.check_early_constraints(ir)),
)


Expand Down
5 changes: 4 additions & 1 deletion compiler/front_end/expression_bounds.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,18 +144,21 @@ def _compute_constraints_of_field_reference(expression, ir):
expression.type.integer.modulus = "1"
expression.type.integer.modular_value = "0"
type_definition = ir_util.find_parent_object(field_path, ir)
type_size = None
if isinstance(field, ir_data.Field):
referrent_type = field.type
else:
referrent_type = field.physical_type_alias
if referrent_type.HasField("size_in_bits"):
type_size = ir_util.constant_value(referrent_type.size_in_bits)
else:
elif isinstance(field, ir_data.Field):
field_size = ir_util.constant_value(field.location.size)
if field_size is None:
type_size = None
else:
type_size = field_size * type_definition.addressable_unit
else:
type_size = None
assert referrent_type.HasField("atomic_type"), field
assert not referrent_type.atomic_type.reference.canonical_name.module_file
_set_integer_constraints_from_physical_type(
Expand Down
1 change: 1 addition & 0 deletions compiler/front_end/glue.py
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ def process_ir(ir, stop_before_step):
symbol_resolver.resolve_field_references,
type_check.annotate_types,
type_check.check_types,
constraints.check_early_constraints,
expression_bounds.compute_constants,
attribute_checker.normalize_and_verify,
constraints.check_constraints,
Expand Down

0 comments on commit 75bdc4c

Please sign in to comment.