`BundleInstance.__setattr__` (and maybe a few others?) should probably fail

[dan-fritchman]

Some new-user intuition about how to use Bundles:

import hdl21 as h 

@h.module
class Inner:
    d = h.Diff(port=True)

@h.module
class Outer:
    # Make an `Inner` instance, and a `Diff` connected to it
    d = h.Diff()
    i = Inner(d=d)

    # This is the problem part here: 
    x, y = 2 * h.Signal()
    d.p = x 
    d.n = y

    # And connect Signals `x` and `y` to other stuff 
    v = h.Vdc()(p=x, n=y)

Which, I get why that might be your intuition. We allow LHS-assignment/ __setattr__ of Instance-ports; bundles probably seem kinda similar.

But they're not. And that produces a netlist like so:

subckt Outer 
  + // No ports 
  + // No parameters 

  i
  + // Ports: 
  + ( d_p d_n )
  +  Inner 
  + // No parameters 

  v
  + // Ports: 
  + ( x y )
  +  vsource 
  +  dc=0 type=dc mag=0 

Which is bad in that:

• (a) x and y don't connect to the d signals at all, and
• (b) We didn't offer any indication this looked like a problem

All that's happening: BundleInstance.__setattr__ is just the "regular" object.__setattr__. It stores fields named p and n, which are Signals, which never get looked at again.

This should probably fail at __setattr__. There may be some other "struct-like" types on which we want to add similar behavior; Slice and Concat come to mind.

[dan-fritchman]

FTR - the version of Outer that does work is:

@h.module
class Outer:
    d = h.Diff()
    i = Inner(d=d)
    v = h.Vdc()(p=d.p, n=d.n)

[dan-fritchman]

So poring through all the types that Hdl21 defines, I think they fall in a few categories:

• "Containers" - the semi-arbitrary data-stores, where we apply __setattr__ magic and the like to allow users to gather up semi-arbitrary piles of stuff. Most notably Module.
• "Structs" - built-in parts of the HDL data model. We would prefer against allowing any fields other than the declared ones.
• Enums
• Private/ internal types, e.g. Elaborators. Don't really care on these.

A quick grep-spin through which are which:

# Removing: 
# * `examples/`
# * Instances of the actual built-in types, e.g. `Diff` and `Pair`
# * Enums - no super-easy way to block attr-additions

# "Containers"
# Stuff where we apply our magic to allow arbitrary `__setattr__` etc. 
hdl21/bundle.py:class Bundle:
hdl21/module.py:class Module:
hdl21/pdk/installation.py:class PdkInstallation:
hdl21/sim/data.py:class Sim:
# Instances are kinda a sub-version of this, since they do "connect by setattr"
hdl21/instance.py:class _Instance:
hdl21/instance.py:class Instance(_Instance):
hdl21/instance.py:class InstanceBundle(_Instance):
hdl21/instance.py:class InstanceArray(_Instance):

# Struct-Like 
hdl21/bundle.py:class BundleInstance:
hdl21/bundle.py:class AnonymousBundle:
hdl21/bundle.py:class BundleRef:
hdl21/concat.py:class Concat:
hdl21/default.py:class Default:
hdl21/external_module.py:class ExternalModule:
hdl21/external_module.py:class ExternalModuleCall:
hdl21/generator.py:class Generator:
hdl21/generator.py:class GeneratorCall:
hdl21/literal.py:class Literal:
hdl21/noconn.py:class NoConn:
hdl21/params.py:class Param:
hdl21/params.py:class HasNoParams:
hdl21/instance.py:class Refs:
hdl21/pdk/corner.py:class CmosCornerPair:
hdl21/portref.py:class PortRef:
hdl21/prefix.py:class Prefixed(BaseModel):
hdl21/prefix.py:class Exponent:
hdl21/primitives.py:class Primitive:
hdl21/primitives.py:class PrimitiveCall:
hdl21/primitives.py:class PrimLibEntry:
hdl21/props.py:class Properties:
hdl21/role.py:class Role:
hdl21/role.py:class RoleSet:
hdl21/scalar.py:class Scalar(BaseModel):
hdl21/signal.py:class Signal:
hdl21/sim/data.py:class Param:
hdl21/sim/data.py:class LinearSweep:
hdl21/sim/data.py:class LogSweep:
hdl21/sim/data.py:class PointSweep:
hdl21/sim/data.py:class Op:
hdl21/sim/data.py:class Dc:
hdl21/sim/data.py:class Ac:
hdl21/sim/data.py:class Tran:
hdl21/sim/data.py:class Noise:
hdl21/sim/data.py:class SweepAnalysis:
hdl21/sim/data.py:class MonteCarlo:
hdl21/sim/data.py:class CustomAnalysis:
hdl21/sim/data.py:class Save:
hdl21/sim/data.py:class Meas:
hdl21/sim/data.py:class Include:
hdl21/sim/data.py:class Lib:
hdl21/sim/data.py:class Options:
hdl21/slice.py:class Slice:
hdl21/slice.py:class SliceInner:

# Internal, Don't Care 
hdl21/elab/context.py:class ContextUsage:
hdl21/elab/context.py:class ElabSession:
hdl21/elab/elaborators/arrays.py:class ArrayFlattener(Elaborator):
hdl21/elab/elaborators/base.py:class Elaborator:
hdl21/elab/elaborators/conntypes.py:class Valid:
hdl21/elab/elaborators/conntypes.py:class NoPort:
hdl21/elab/elaborators/conntypes.py:class Unconnected:
hdl21/elab/elaborators/conntypes.py:class InvalidType:
hdl21/elab/elaborators/conntypes.py:class ConnTypes(Elaborator):
hdl21/elab/elaborators/flatten_bundles.py:class Path:
hdl21/elab/elaborators/flatten_bundles.py:class BundleScope:
hdl21/elab/elaborators/flatten_bundles.py:class BundlePortEntry:
hdl21/elab/elaborators/flatten_bundles.py:class Cache:
hdl21/elab/elaborators/flatten_bundles.py:class BundleFlattener(Elaborator):
hdl21/elab/elaborators/generators.py:class GeneratorElaborator(Elaborator):
hdl21/elab/elaborators/inst_bundles.py:class InstBundleElaborator(Elaborator):
hdl21/elab/elaborators/mark_modules.py:class MarkModules(Elaborator):
hdl21/elab/elaborators/orphanage.py:class Orphanage(Elaborator):
hdl21/elab/elaborators/portrefs.py:class ResolvePortRefs(Elaborator):
hdl21/elab/elaborators/portrefs.py:class SetList:
hdl21/elab/elaborators/resolve_ref_types.py:class Inner:
hdl21/elab/elaborators/resolve_ref_types.py:class Outer:
hdl21/elab/elaborators/slices.py:class SliceResolver(Elaborator):
hdl21/pdk/pdk.py:class _PdkManager:
hdl21/proto/from_proto.py:class ProtoImporter:
hdl21/proto/to_proto.py:class ModuleMapping:
hdl21/proto/to_proto.py:class ProtoExporter:
hdl21/sim/to_proto.py:class SimProtoExporter:
hdl21/source_info.py:class SourceInfo:
pdks/Sky130/sky130/pdk_data.py:class Cache:
pdks/Sky130/sky130/pdk_logic.py:class Install(PdkInstallation):
pdks/Sky130/sky130/pdk_logic.py:class Sky130Walker(h.HierarchyWalker):

# Public, but not really real yet 
hdl21/elab/context.py:class Context:

So, kinda a lot of the struct-like ones.
There are probably a few which cause the most confusion - e.g. the namesake BundleInstance which generated this issue.

[dan-fritchman]

OK so of that long list of "struct-like" types:

• All but four are Pydantic things - either dataclass, BaseModel, or our datatype wrapper therefore

  • It's... not totally clear how to make the dataclass ones do what we want here?
  • See https://github.com/pydantic/pydantic/discussions/6232
  • These goals do seem to be readily doable on BaseModel. Maybe we've gotta swap all those data-types to that.

• The other four are BundleInstance, BundleRef, AnonymousBundle, and Concat

  • 148 adds attrmagic decorators for "disallow all setattr" and "only allow setting known attrs"

[dan-fritchman]

So -

• v4 / PR #148 adds this to the non-Pydantic types (Bundle-things, Concat)
• Applying similar __setattr__ magic to the Pydantic types has, probably understandably, not worked
• I'll leave this open until we have a "right way" to do this for the Pydantic types.
  • Leading candidate: convert all the internal dataclasses to BaseModels