From 48add82534038171b5ce651580f000d617361841 Mon Sep 17 00:00:00 2001 From: Richard Lin Date: Fri, 28 Nov 2025 16:58:32 -0800 Subject: [PATCH 1/3] remove constraintexpr copyfrom performance gains are marginal =( --- edg/core/Array.py | 11 +-- edg/core/ArrayExpr.py | 2 +- edg/core/Binding.py | 142 +++++++++++++++---------------------- edg/core/Blocks.py | 12 ++-- edg/core/ConstraintExpr.py | 4 +- edg/core/HierarchyBlock.py | 2 +- 6 files changed, 71 insertions(+), 102 deletions(-) diff --git a/edg/core/Array.py b/edg/core/Array.py index 88179bc21..44da69d5f 100644 --- a/edg/core/Array.py +++ b/edg/core/Array.py @@ -29,13 +29,10 @@ def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: return [self.container] @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: contained_map = self.container._get_elt_sample()._create_ref_map(edgir.LocalPath()) - - pb = edgir.ValueExpr() pb.map_extract.container.ref.CopyFrom(ref_map[self.container]) # TODO support arbitrary refs pb.map_extract.path.CopyFrom(contained_map[self.elt]) - return pb class FlattenBinding(Binding): @@ -48,11 +45,9 @@ def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: return [self.elts] @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: pb.unary_set.op = edgir.UnarySetExpr.Op.FLATTEN - pb.unary_set.vals.CopyFrom(self.elts._expr_to_proto(ref_map)) - return pb + self.elts._populate_expr_proto(pb.unary_set.vals, ref_map) @non_library diff --git a/edg/core/ArrayExpr.py b/edg/core/ArrayExpr.py index d51d4855e..a3fdbe163 100644 --- a/edg/core/ArrayExpr.py +++ b/edg/core/ArrayExpr.py @@ -23,7 +23,7 @@ def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: # element return [] @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: raise ValueError # can't be used directly diff --git a/edg/core/Binding.py b/edg/core/Binding.py index d3e53927a..18b410bbd 100644 --- a/edg/core/Binding.py +++ b/edg/core/Binding.py @@ -75,7 +75,7 @@ def is_bound(self) -> bool: return True @abstractmethod - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: ... + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: ... @abstractmethod def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: ... @@ -106,10 +106,8 @@ def is_bound(self) -> bool: return True @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: pb.ref.CopyFrom(ref_map[expr]) - return pb class InitParamBinding(ParamBinding): @@ -126,6 +124,14 @@ class LiteralBinding(Binding): def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: return [] + @abstractmethod + def populate_literal_proto(self, pb: edgir.ValueLit) -> None: + raise NotImplementedError + + @override + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: + self.populate_literal_proto(pb.literal) + class BoolLiteralBinding(LiteralBinding): @override @@ -137,10 +143,8 @@ def __init__(self, value: bool): self.value = value @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() - pb.literal.boolean.val = self.value - return pb + def populate_literal_proto(self, pb: edgir.ValueLit) -> None: + pb.boolean.val = self.value class IntLiteralBinding(LiteralBinding): @@ -152,10 +156,8 @@ def __init__(self, value: int): self.value = value @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() - pb.literal.integer.val = self.value - return pb + def populate_literal_proto(self, pb: edgir.ValueLit) -> None: + pb.integer.val = self.value class FloatLiteralBinding(LiteralBinding): @@ -167,10 +169,8 @@ def __init__(self, value: Union[float, int]): self.value: float = float(value) @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() - pb.literal.floating.val = self.value - return pb + def populate_literal_proto(self, pb: edgir.ValueLit) -> None: + pb.floating.val = self.value class RangeLiteralBinding(LiteralBinding): @@ -182,11 +182,9 @@ def __init__(self, value: Range): self.value = value @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() - pb.literal.range.minimum.floating.val = self.value.lower - pb.literal.range.maximum.floating.val = self.value.upper - return pb + def populate_literal_proto(self, pb: edgir.ValueLit) -> None: + pb.range.minimum.floating.val = self.value.lower + pb.range.maximum.floating.val = self.value.upper class StringLiteralBinding(LiteralBinding): @@ -199,10 +197,8 @@ def __init__(self, value: str): self.value = value @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() - pb.literal.text.val = self.value - return pb + def populate_literal_proto(self, pb: edgir.ValueLit) -> None: + pb.text.val = self.value class ArrayLiteralBinding(LiteralBinding): @@ -215,14 +211,10 @@ def __init__(self, values: Sequence[LiteralBinding]): self.values = values @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() - pb.literal.array.SetInParent() + def populate_literal_proto(self, pb: edgir.ValueLit) -> None: + pb.array.SetInParent() for value in self.values: - elt_value = value.expr_to_proto(expr, ref_map) - assert elt_value.HasField('literal') - pb.literal.array.elts.add().CopyFrom(elt_value.literal) - return pb + value.populate_literal_proto(pb.array.elts.add()) class RangeBuilderBinding(Binding): @@ -240,15 +232,13 @@ def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: return chain(self.lower._get_exprs(), self.lower._get_exprs()) @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: pb.binary.op = edgir.BinaryExpr.RANGE - pb.binary.lhs.CopyFrom(self.lower._expr_to_proto(ref_map)) - pb.binary.rhs.CopyFrom(self.upper._expr_to_proto(ref_map)) - return pb + self.lower._populate_expr_proto(pb.binary.lhs, ref_map) + self.upper._populate_expr_proto(pb.binary.rhs, ref_map) -class ArrayBinding(LiteralBinding): +class ArrayBinding(Binding): @override def __repr__(self) -> str: return f"Array({self.values})" @@ -258,12 +248,14 @@ def __init__(self, values: Sequence[ConstraintExpr]): self.values = values @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() + def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: + return self.values + + @override + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: pb.array.SetInParent() for value in self.values: - pb.array.vals.add().CopyFrom(value._expr_to_proto(ref_map)) - return pb + value._populate_expr_proto(pb.array.vals.add(), ref_map) class UnaryOpBinding(Binding): @@ -293,12 +285,9 @@ def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: return chain(self.src._get_exprs()) @override - def expr_to_proto(self, expr: ConstraintExpr, - ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: pb.unary.op = self.op_map[self.op] - pb.unary.val.CopyFrom(self.src._expr_to_proto(ref_map)) - return pb + self.src._populate_expr_proto(pb.unary.val, ref_map) class UnarySetOpBinding(Binding): @override @@ -332,12 +321,9 @@ def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: return chain(self.src._get_exprs()) @override - def expr_to_proto(self, expr: ConstraintExpr, - ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: pb.unary_set.op = self.op_map[self.op] - pb.unary_set.vals.CopyFrom(self.src._expr_to_proto(ref_map)) - return pb + self.src._populate_expr_proto(pb.unary_set.vals, ref_map) class BinaryOpBinding(Binding): @override @@ -385,12 +371,11 @@ def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: return chain(self.lhs._get_exprs(), self.rhs._get_exprs()) @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: pb.binary.op = self.op_map[self.op] - pb.binary.lhs.CopyFrom(self.lhs._expr_to_proto(ref_map)) - pb.binary.rhs.CopyFrom(self.rhs._expr_to_proto(ref_map)) - return pb + self.lhs._populate_expr_proto(pb.binary.lhs, ref_map) + self.rhs._populate_expr_proto(pb.binary.rhs, ref_map) + class BinarySetOpBinding(Binding): @override @@ -417,12 +402,10 @@ def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: return chain(self.lhset._get_exprs(), self.rhs._get_exprs()) @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: pb.binary_set.op = self.op_map[self.op] - pb.binary_set.lhset.CopyFrom(self.lhset._expr_to_proto(ref_map)) - pb.binary_set.rhs.CopyFrom(self.rhs._expr_to_proto(ref_map)) - return pb + self.lhset._populate_expr_proto(pb.binary_set.lhset, ref_map) + self.rhs._populate_expr_proto(pb.binary_set.rhs, ref_map) class IfThenElseBinding(Binding): @@ -441,12 +424,10 @@ def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: return chain(self.cond._get_exprs(), self.then_val._get_exprs(), self.else_val._get_exprs()) @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() - pb.ifThenElse.cond.CopyFrom(self.cond._expr_to_proto(ref_map)) - pb.ifThenElse.tru.CopyFrom(self.then_val._expr_to_proto(ref_map)) - pb.ifThenElse.fal.CopyFrom(self.else_val._expr_to_proto(ref_map)) - return pb + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: + self.cond._populate_expr_proto(pb.ifThenElse.cond, ref_map) + self.then_val._populate_expr_proto(pb.ifThenElse.tru, ref_map) + self.else_val._populate_expr_proto(pb.ifThenElse.fal, ref_map) class IsConnectedBinding(Binding): @@ -463,11 +444,10 @@ def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: return [self.src] @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: pb.ref.CopyFrom(ref_map[self.src]) pb.ref.steps.add().reserved_param = edgir.IS_CONNECTED - return pb + class NameBinding(Binding): @override @@ -483,11 +463,9 @@ def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: return [] @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: pb.ref.CopyFrom(ref_map[self.src]) pb.ref.steps.add().reserved_param = edgir.NAME - return pb class LengthBinding(Binding): @@ -504,11 +482,9 @@ def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: return [self.src] @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: pb.ref.CopyFrom(ref_map[self.src]) pb.ref.steps.add().reserved_param = edgir.LENGTH - return pb class AllocatedBinding(Binding): @@ -525,20 +501,19 @@ def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: return [self.src] @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - pb = edgir.ValueExpr() + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: pb.ref.CopyFrom(ref_map[self.src]) pb.ref.steps.add().reserved_param = edgir.ALLOCATED - return pb class AssignBinding(Binding): # Convenience method to make an assign expr without the rest of this proto infrastructure @staticmethod def make_assign(target: ConstraintExpr, value: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: + # TODO DEDUP w/ _POPULATE_EXPR_PROTO pb = edgir.ValueExpr() pb.assign.dst.CopyFrom(ref_map[target]) - pb.assign.src.CopyFrom(value._expr_to_proto(ref_map)) + value._populate_expr_proto(pb.assign.src, ref_map) return pb @override @@ -555,5 +530,6 @@ def get_subexprs(self) -> Iterable[Union[ConstraintExpr, BasePort]]: return [self.value] @override - def expr_to_proto(self, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - return self.make_assign(self.target, self.value, ref_map) + def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map: Refable.RefMapType) -> None: + pb.assign.dst.CopyFrom(ref_map[self.target]) + self.value._populate_expr_proto(pb.assign.src, ref_map) diff --git a/edg/core/Blocks.py b/edg/core/Blocks.py index d44571e1f..96caa705c 100644 --- a/edg/core/Blocks.py +++ b/edg/core/Blocks.py @@ -352,13 +352,11 @@ def _populate_def_proto_block_base(self, pb: edgir.BlockLikeTypes) -> None: for (name, port) in self._ports.items(): if port in self._required_ports: if isinstance(port, Port): - edgir.add_pair(pb.constraints, f'(reqd){name}').CopyFrom( - port.is_connected()._expr_to_proto(ref_map) - ) + port.is_connected()._populate_expr_proto( + edgir.add_pair(pb.constraints, f'(reqd){name}'), ref_map) elif isinstance(port, Vector): - edgir.add_pair(pb.constraints, f'(reqd){name}').CopyFrom( - (port.length() > 0)._expr_to_proto(ref_map) - ) + (port.length() > 0)._populate_expr_proto( + edgir.add_pair(pb.constraints, f'(reqd){name}'), ref_map) else: raise ValueError(f"unknown non-optional port type {port}") @@ -388,7 +386,7 @@ def _populate_def_proto_block_contents(self, pb: edgir.BlockLikeTypes, ref_map: self._constraints.finalize() for (name, constraint) in self._constraints.items(): - edgir.add_pair(pb.constraints, name).CopyFrom(constraint._expr_to_proto(ref_map)) + constraint._populate_expr_proto(edgir.add_pair(pb.constraints, name), ref_map) def _populate_def_proto_description(self, pb: edgir.BlockLikeTypes, ref_map: Refable.RefMapType) -> None: description = self.description diff --git a/edg/core/ConstraintExpr.py b/edg/core/ConstraintExpr.py index 59b229ed5..be45a6952 100644 --- a/edg/core/ConstraintExpr.py +++ b/edg/core/ConstraintExpr.py @@ -93,9 +93,9 @@ def _bind(self: SelfType, binding: Binding) -> SelfType: def _is_bound(self) -> bool: return self.binding is not None and self.binding.is_bound() - def _expr_to_proto(self, ref_map: Refable.RefMapType) -> edgir.ValueExpr: + def _populate_expr_proto(self, pb: edgir.ValueExpr, ref_map: Refable.RefMapType) -> None: assert self.binding is not None - return self.binding.expr_to_proto(self, ref_map) + self.binding.populate_expr_proto(pb, self, ref_map) # for now we define that all constraints can be checked for equivalence @override diff --git a/edg/core/HierarchyBlock.py b/edg/core/HierarchyBlock.py index 132f62c1f..ac335af58 100644 --- a/edg/core/HierarchyBlock.py +++ b/edg/core/HierarchyBlock.py @@ -335,7 +335,7 @@ def _populate_def_proto_block_base(self, pb: edgir.BlockLikeTypes) -> None: if isinstance(param.binding, InitParamBinding) and param.binding.value is not None: # default values can't depend on anything so the ref_map is empty param_typed_value = param._to_expr_type(param.binding.value) - pb.param_defaults[param_name].CopyFrom(param_typed_value._expr_to_proto(IdentityDict())) + param_typed_value._populate_expr_proto(pb.param_defaults[param_name], IdentityDict()) def _populate_def_proto_hierarchy(self, pb: edgir.HierarchyBlock, ref_map: Refable.RefMapType) -> None: self._blocks.finalize() From 13b7700a4db120d960e43c659a328ee939a57f6b Mon Sep 17 00:00:00 2001 From: Richard Lin Date: Fri, 28 Nov 2025 18:17:51 -0800 Subject: [PATCH 2/3] refactor decl to proto --- edg/core/Array.py | 10 ++++------ edg/core/ArrayExpr.py | 6 ++---- edg/core/Blocks.py | 4 ++-- edg/core/ConstraintExpr.py | 34 ++++++++++++---------------------- edg/core/Ports.py | 14 ++++++-------- 5 files changed, 26 insertions(+), 42 deletions(-) diff --git a/edg/core/Array.py b/edg/core/Array.py index 44da69d5f..0ab555ed1 100644 --- a/edg/core/Array.py +++ b/edg/core/Array.py @@ -86,8 +86,8 @@ def _get_elt_sample(self) -> BasePort: return self.target @override - def _instance_to_proto(self) -> edgir.PortLike: - raise RuntimeError() # this doesn't generate into a library element + def _populate_portlike_proto(self, pb: edgir.PortLike) -> None: + raise RuntimeError() # this can't be a block's port @override def _def_to_proto(self) -> edgir.PortTypes: @@ -179,14 +179,12 @@ def _get_def_name(self) -> str: raise RuntimeError() # this doesn't generate into a library element @override - def _instance_to_proto(self) -> edgir.PortLike: - pb = edgir.PortLike() + def _populate_portlike_proto(self, pb: edgir.PortLike) -> None: pb.array.self_class.target.name = self._elt_sample._get_def_name() if self._elts is not None: pb.array.ports.SetInParent() # mark as defined, even if empty for name, elt in self._elts.items(): - edgir.add_pair(pb.array.ports.ports, name).CopyFrom(elt._instance_to_proto()) - return pb + elt._populate_portlike_proto(edgir.add_pair(pb.array.ports.ports, name)) @override def _def_to_proto(self) -> edgir.PortTypes: diff --git a/edg/core/ArrayExpr.py b/edg/core/ArrayExpr.py index a3fdbe163..fabcde16a 100644 --- a/edg/core/ArrayExpr.py +++ b/edg/core/ArrayExpr.py @@ -61,10 +61,8 @@ def _to_expr_type(cls: Type[SelfType], input: ArrayCastableType) -> SelfType: @classmethod @override - def _decl_to_proto(cls) -> edgir.ValInit: - pb = edgir.ValInit() - pb.array.CopyFrom(cls._elt_type._decl_to_proto()) - return pb + def _populate_decl_proto(cls, pb: edgir.ValInit) -> None: + cls._elt_type._populate_decl_proto(pb.array) @staticmethod def array_of_elt(elt: ConstraintExpr) -> ArrayExpr: diff --git a/edg/core/Blocks.py b/edg/core/Blocks.py index 96caa705c..8dab281a3 100644 --- a/edg/core/Blocks.py +++ b/edg/core/Blocks.py @@ -343,10 +343,10 @@ def _populate_def_proto_block_base(self, pb: edgir.BlockLikeTypes) -> None: for (name, param) in self._parameters.items(): assert isinstance(param.binding, ParamBinding) - edgir.add_pair(pb.params, name).CopyFrom(param._decl_to_proto()) + param._populate_decl_proto(edgir.add_pair(pb.params, name)) for (name, port) in self._ports.items(): - edgir.add_pair(pb.ports, name).CopyFrom(port._instance_to_proto()) + port._populate_portlike_proto(edgir.add_pair(pb.ports, name)) ref_map = self._create_ref_map() for (name, port) in self._ports.items(): diff --git a/edg/core/ConstraintExpr.py b/edg/core/ConstraintExpr.py index be45a6952..52f033b09 100644 --- a/edg/core/ConstraintExpr.py +++ b/edg/core/ConstraintExpr.py @@ -49,7 +49,7 @@ def _to_expr_type(cls: Type[SelfType], input: CastableType) -> SelfType: @classmethod @abstractmethod - def _decl_to_proto(cls) -> edgir.ValInit: + def _populate_decl_proto(cls, pb: edgir.ValInit) -> None: """Returns the protobuf for the definition of this parameter. Must have ParamBinding / ParamVariableBinding""" raise NotImplementedError @@ -123,10 +123,8 @@ def _to_expr_type(cls, input: BoolLike) -> BoolExpr: @classmethod @override - def _decl_to_proto(self) -> edgir.ValInit: - pb = edgir.ValInit() - pb.boolean.CopyFrom(edgir.Empty()) - return pb + def _populate_decl_proto(cls, pb: edgir.ValInit) -> None: + pb.boolean.SetInParent() @classmethod @override @@ -328,10 +326,8 @@ def _to_expr_type(cls, input: IntLike) -> IntExpr: @classmethod @override - def _decl_to_proto(cls) -> edgir.ValInit: - pb = edgir.ValInit() - pb.integer.CopyFrom(edgir.Empty()) - return pb + def _populate_decl_proto(cls, pb: edgir.ValInit) -> None: + pb.integer.SetInParent() @classmethod @override @@ -361,10 +357,8 @@ def _to_expr_type(cls, input: Union[FloatLike, IntExpr]) -> FloatExpr: @classmethod @override - def _decl_to_proto(cls) -> edgir.ValInit: - pb = edgir.ValInit() - pb.floating.CopyFrom(edgir.Empty()) - return pb + def _populate_decl_proto(cls, pb: edgir.ValInit) -> None: + pb.floating.SetInParent() @classmethod @override @@ -417,10 +411,8 @@ def _to_expr_type(cls, input: Union[RangeLike, FloatLike, IntLike]) -> RangeExpr @classmethod @override - def _decl_to_proto(cls) -> edgir.ValInit: - pb = edgir.ValInit() - pb.range.CopyFrom(edgir.Empty()) - return pb + def _populate_decl_proto(cls, pb: edgir.ValInit) -> None: + pb.range.SetInParent() @classmethod @override @@ -527,10 +519,8 @@ def _to_expr_type(cls, input: StringLike) -> StringExpr: @classmethod @override - def _decl_to_proto(cls) -> edgir.ValInit: - pb = edgir.ValInit() - pb.text.CopyFrom(edgir.Empty()) - return pb + def _populate_decl_proto(cls, pb: edgir.ValInit) -> None: + pb.text.SetInParent() @classmethod @override @@ -562,7 +552,7 @@ def _to_expr_type(cls, input: Any) -> NoReturn: @classmethod @override - def _decl_to_proto(self) -> NoReturn: + def _populate_decl_proto(cls, pb: edgir.ValInit) -> None: raise ValueError("can't create parameter from AssignExpr") @classmethod diff --git a/edg/core/Ports.py b/edg/core/Ports.py index 7795077a9..e582e7d31 100644 --- a/edg/core/Ports.py +++ b/edg/core/Ports.py @@ -64,8 +64,8 @@ def _def_to_proto(self) -> edgir.PortTypes: # TODO: this might not be valid for def _type_of(self) -> Hashable: ... @abstractmethod - def _instance_to_proto(self) -> edgir.PortLike: - """Returns the proto of an instance of this object""" + def _populate_portlike_proto(self, pb: edgir.PortLike) -> None: + """Populates the proto of an instance of this object""" raise NotImplementedError def _bind_in_place(self, parent: PortParentTypes) -> None: @@ -209,10 +209,8 @@ def _convert(self, adapter: PortAdapter[ConvertTargetType]) -> ConvertTargetType return adapter_inst.dst @override - def _instance_to_proto(self) -> edgir.PortLike: - pb = edgir.PortLike() + def _populate_portlike_proto(self, pb: edgir.PortLike) -> None: pb.lib_elem.target.name = self._get_def_name() - return pb @override def _def_to_proto(self) -> edgir.PortTypes: @@ -229,7 +227,7 @@ def _def_to_proto(self) -> edgir.PortTypes: pb.super_superclasses.add().target.name = cls._static_def_name() for (name, param) in self._parameters.items(): - edgir.add_pair(pb.params, name).CopyFrom(param._decl_to_proto()) + param._populate_decl_proto(edgir.add_pair(pb.params, name)) self._populate_metadata(pb.meta, self._metadata, IdentityDict()) # TODO use ref map @@ -333,9 +331,9 @@ def _def_to_proto(self) -> edgir.Bundle: pb.super_superclasses.add().target.name = cls._static_def_name() for (name, param) in self._parameters.items(): - edgir.add_pair(pb.params, name).CopyFrom(param._decl_to_proto()) + param._populate_decl_proto(edgir.add_pair(pb.params, name)) for (name, port) in self._ports.items(): - edgir.add_pair(pb.ports, name).CopyFrom(port._instance_to_proto()) + port._populate_portlike_proto(edgir.add_pair(pb.ports, name)) self._populate_metadata(pb.meta, self._metadata, IdentityDict()) # TODO use ref map From c1457c86b060152adec45b5fe815df67510e3b79 Mon Sep 17 00:00:00 2001 From: Richard Lin Date: Fri, 28 Nov 2025 18:28:20 -0800 Subject: [PATCH 3/3] refactor out assign; cleaning --- edg/core/Binding.py | 7 ++----- edg/core/Blocks.py | 10 ++++------ edg/core/Core.py | 13 ++----------- edg/core/HierarchyBlock.py | 6 +++--- 4 files changed, 11 insertions(+), 25 deletions(-) diff --git a/edg/core/Binding.py b/edg/core/Binding.py index 18b410bbd..6d1efa9d3 100644 --- a/edg/core/Binding.py +++ b/edg/core/Binding.py @@ -507,14 +507,11 @@ def populate_expr_proto(self, pb: edgir.ValueExpr, expr: ConstraintExpr, ref_map class AssignBinding(Binding): - # Convenience method to make an assign expr without the rest of this proto infrastructure @staticmethod - def make_assign(target: ConstraintExpr, value: ConstraintExpr, ref_map: Refable.RefMapType) -> edgir.ValueExpr: - # TODO DEDUP w/ _POPULATE_EXPR_PROTO - pb = edgir.ValueExpr() + def populate_assign_proto(pb: edgir.ValueExpr, target: ConstraintExpr, value: ConstraintExpr, ref_map: Refable.RefMapType) -> None: + # Convenience method to make an assign expr without the rest of this proto infrastructure pb.assign.dst.CopyFrom(ref_map[target]) value._populate_expr_proto(pb.assign.src, ref_map) - return pb @override def __repr__(self) -> str: diff --git a/edg/core/Blocks.py b/edg/core/Blocks.py index 8dab281a3..8a63b01b1 100644 --- a/edg/core/Blocks.py +++ b/edg/core/Blocks.py @@ -367,16 +367,14 @@ def _populate_def_proto_block_base(self, pb: edgir.BlockLikeTypes) -> None: def _populate_def_proto_port_init(self, pb: edgir.BlockLikeTypes, ref_map: Refable.RefMapType) -> None: for (name, port) in self._ports.items(): for (param, path, initializer) in port._get_initializers([name]): - edgir.add_pair(pb.constraints, f"(init){'.'.join(path)}").CopyFrom( - AssignBinding.make_assign(param, param._to_expr_type(initializer), ref_map) - ) + AssignBinding.populate_assign_proto(edgir.add_pair(pb.constraints, f"(init){'.'.join(path)}"), + param, param._to_expr_type(initializer), ref_map) def _populate_def_proto_param_init(self, pb: edgir.BlockLikeTypes, ref_map: Refable.RefMapType) -> None: for (name, param) in self._parameters.items(): if param.initializer is not None: - edgir.add_pair(pb.constraints, f'(init){name}').CopyFrom( - AssignBinding.make_assign(param, param.initializer, ref_map) - ) + AssignBinding.populate_assign_proto(edgir.add_pair(pb.constraints, f'(init){name}'), + param, param.initializer, ref_map) def _populate_def_proto_block_contents(self, pb: edgir.BlockLikeTypes, ref_map: Refable.RefMapType) -> None: """Populates the contents of a block proto: constraints""" diff --git a/edg/core/Core.py b/edg/core/Core.py index dfec52055..c8f6d05e0 100644 --- a/edg/core/Core.py +++ b/edg/core/Core.py @@ -250,13 +250,6 @@ def _get_def_name(self) -> str: def _def_to_proto(self) -> Union[edgir.PortTypes, edgir.BlockLikeTypes]: ... -class StructuredMetadata(): - """Base class for metadata that is structured (as a class in Python)""" - @abstractmethod - def _to_proto(self, ref_map: Refable.RefMapType) -> edgir.Metadata: - raise NotImplementedError - - @non_library class HasMetadata(LibraryElement): """A library element with the metadata dict-like field""" @@ -264,7 +257,7 @@ def __init__(self) -> None: super().__init__() self._metadata: SubElementDict[Any] = self.manager.new_dict(Any) - MetadataType = TypeVar('MetadataType', bound=Union[StructuredMetadata, str, Mapping[str, Any], SubElementDict[Any], IdentityDict[Any, Any]]) + MetadataType = TypeVar('MetadataType', bound=Union[str, Mapping[str, Any], SubElementDict[Any], IdentityDict[Any, Any]]) def Metadata(self, value: MetadataType) -> MetadataType: """Adds a metadata field to this object. Reference to the value must not change, and reassignment will error. Value may be changed until proto generation. @@ -305,9 +298,7 @@ def process_direct_base(bcls: Type[HasMetadata.BaseType]) -> None: def _populate_metadata(self, pb: edgir.Metadata, src: Any, ref_map: Refable.RefMapType) -> None: """Generate metadata from a given object.""" - if isinstance(src, StructuredMetadata): - pb.CopyFrom(src._to_proto(ref_map)) - elif isinstance(src, str): + if isinstance(src, str): pb.text_leaf = src elif isinstance(src, bytes): pb.bin_leaf = src diff --git a/edg/core/HierarchyBlock.py b/edg/core/HierarchyBlock.py index ac335af58..15a3f40ba 100644 --- a/edg/core/HierarchyBlock.py +++ b/edg/core/HierarchyBlock.py @@ -438,9 +438,9 @@ def _populate_def_proto_hierarchy(self, pb: edgir.HierarchyBlock, ref_map: Refab for (block_param_name, block_param) in all_block_params.items(): if isinstance(block_param.binding, InitParamBinding) and block_param.binding.value is not None: param_typed_value = block_param._to_expr_type(block_param.binding.value) - edgir.add_pair(pb.constraints, f'(init){block_name}.{block_param_name}').CopyFrom( # TODO better name - AssignBinding.make_assign(block_param, param_typed_value, ref_map) - ) + AssignBinding.populate_assign_proto(edgir.add_pair(pb.constraints, f'(init){block_name}.{block_param_name}'), + block_param, param_typed_value, ref_map) + # TODO make this non-overriding? @override