[ipgen] Strengthen template parameter type checks
The `IpConfig` class holds typed template parameters, which can be
either a string or an integer. Before, the parameter type was only
checked when data was loaded from text form (usually reading a Hjson
file), but not when constructing an `IpConfig` object programmatically.
Strengthen the checks to ensure that all template parameters which are
passed into `IpConfig` can ultimately be serialized to Hjson and survive
a round-trip.
This checking requires passing the list of expected template parameters,
encapsulated in `TemplateParams` around a bit more.
Signed-off-by: Philipp Wagner <phw@lowrisc.org>
diff --git a/util/ipgen/lib.py b/util/ipgen/lib.py
index c90198d..579e628 100644
--- a/util/ipgen/lib.py
+++ b/util/ipgen/lib.py
@@ -3,7 +3,7 @@
# SPDX-License-Identifier: Apache-2.0
from pathlib import Path
-from typing import Dict, Optional, Union, cast
+from typing import Any, Dict, Optional, Union
import hjson # type: ignore
from reggen.lib import check_int, check_keys, check_list, check_name, check_str
@@ -16,8 +16,15 @@
class TemplateParameter(BaseParam):
""" A template parameter. """
+ VALID_PARAM_TYPES = (
+ 'int',
+ 'string',
+ )
+
def __init__(self, name: str, desc: Optional[str], param_type: str,
default: str):
+ assert param_type in self.VALID_PARAM_TYPES
+
super().__init__(name, desc, param_type)
self.default = default
self.value = None
@@ -41,16 +48,21 @@
r_type = rd.get('type')
param_type = check_str(r_type, 'type field of ' + where)
- if not param_type in ('string', 'int'):
+ if param_type not in TemplateParameter.VALID_PARAM_TYPES:
raise ValueError('At {}, the {} param has an invalid type field {!r}. '
- 'Allowed values are: string, int.'.format(
- where, name, param_type))
+ 'Allowed values are: {}.'.format(
+ where, name, param_type,
+ ', '.join(TemplateParameter.VALID_PARAM_TYPES)))
r_default = rd.get('default')
- default = check_str(r_default, 'default field of ' + where)
- if param_type[:3] == 'int':
- check_int(default,
- 'default field of {}, (an integer parameter)'.format(name))
+ if param_type == 'int':
+ default = check_int(
+ r_default,
+ 'default field of {}, (an integer parameter)'.format(name))
+ elif param_type == 'string':
+ default = check_str(r_default, 'default field of ' + where)
+ else:
+ assert False, f"Unknown parameter type found: {param_type!r}"
return TemplateParameter(name, desc, param_type, default)
@@ -82,10 +94,10 @@
"""
name: str
- params: Params
+ params: TemplateParams
template_path: Path
- def __init__(self, name: str, params: Params, template_path: Path):
+ def __init__(self, name: str, params: TemplateParams, template_path: Path):
self.name = name
self.params = params
self.template_path = template_path
@@ -145,35 +157,52 @@
return cls(template_name, params, template_path)
-def check_param_dict(obj: object, what: str) -> Dict[str, Union[int, str]]:
- """ Check that obj is a dict with str keys and int or str values. """
- if not isinstance(obj, dict):
- raise ValueError(
- "{} is expected to be a dict, but was actually a {}.".format(
- what,
- type(obj).__name__))
-
- for key, value in obj.items():
- if not isinstance(key, str):
- raise ValueError(
- '{} has a key {!r}, which is not a string.'.format(what, key))
-
- if not (isinstance(value, str) or isinstance(value, int)):
- raise ValueError(f"key {key} of {what} has a value {value!r}, "
- "which is neither a str, nor an int.")
-
- return cast(Dict[str, Union[int, str]], obj)
-
-
class IpConfig:
def __init__(self,
+ template_params: TemplateParams,
instance_name: str,
param_values: Dict[str, Union[str, int]] = {}):
+ self.template_params = template_params
self.instance_name = instance_name
- self.param_values = param_values
+ self.param_values = IpConfig._check_param_values(
+ template_params, param_values)
+
+ @staticmethod
+ def _check_param_values(template_params: TemplateParams,
+ param_values: Any) -> Dict[str, Union[str, int]]:
+ """Check if parameter values are valid.
+
+ Returns the parameter values in typed form if successful, and throws
+ a ValueError otherwise.
+ """
+ param_values_typed = {}
+ for key, value in param_values.items():
+ if not isinstance(key, str):
+ raise ValueError(
+ f"The IP configuration has a key {key!r} which is not a "
+ "string.")
+
+ if key not in template_params:
+ raise ValueError(
+ f"The IP configuration has a key {key!r} which is a "
+ "valid parameter.")
+
+ if template_params[key].param_type == 'string':
+ param_value_typed = check_str(
+ value, f"the key {key} of the IP configuration")
+ elif template_params[key].param_type == 'int':
+ param_value_typed = check_int(
+ value, f"the key {key} of the IP configuration")
+ else:
+ assert True, "Unexpeced parameter type found, expand this check"
+
+ param_values_typed[key] = param_value_typed
+
+ return param_values_typed
@classmethod
- def from_raw(cls, raw: object, where: str) -> 'IpConfig':
+ def from_raw(cls, template_params: TemplateParams, raw: object,
+ where: str) -> 'IpConfig':
""" Load an IpConfig from a raw object """
rd = check_keys(raw, 'configuration file ' + where, ['instance_name'],
@@ -181,10 +210,15 @@
instance_name = check_name(rd.get('instance_name'),
"the key 'instance_name' of " + where)
- param_values = check_param_dict(rd.get('param_values', []),
- f"the key 'param_values' of {where}")
+ if not isinstance(raw, dict):
+ raise ValueError(
+ "The IP configuration is expected to be a dict, but was "
+ "actually a " + type(raw).__name__)
- return cls(instance_name, param_values)
+ param_values = IpConfig._check_param_values(template_params,
+ rd['param_values'])
+
+ return cls(template_params, instance_name, param_values)
@classmethod
def from_text(cls, txt: str, where: str) -> 'IpConfig':
diff --git a/util/ipgen/renderer.py b/util/ipgen/renderer.py
index f2f5f4f..7607e25 100644
--- a/util/ipgen/renderer.py
+++ b/util/ipgen/renderer.py
@@ -14,7 +14,7 @@
from mako.lookup import TemplateLookup as MakoTemplateLookup # type: ignore
from reggen.ip_block import IpBlock
-from .lib import IpConfig, IpTemplate
+from .lib import IpConfig, IpTemplate, TemplateParameter
_HJSON_LICENSE_HEADER = ("""// Copyright lowRISC contributors.
// Licensed under the Apache License, Version 2.0, see LICENSE for details.
@@ -72,7 +72,7 @@
log.info(f"Using default value for template parameter {name}")
val = template_param.default
- assert template_param.param_type in ('string', 'int')
+ assert template_param.param_type in TemplateParameter.VALID_PARAM_TYPES
try:
if template_param.param_type == 'string':
val_typed = str(val) # type: Union[int, str]
