Conversation
…nd PackageInstantiation context items - ModelEntity.GetAncestor(): raise VHDLModelException instead of an unguarded AttributeError when no ancestor of the requested type exists on the parent chain (i.e. the root of the model was reached). - AllowBlackboxMixin.AllowBlackbox: raise VHDLModelException instead of an unguarded AttributeError when neither a local value is set nor a parent is available to inherit from. - Design.TopLevel: check HierarchyGraph.VertexCount instead of EdgeCount to detect an uncomputed hierarchy graph. VertexCount is populated unconditionally by the first step of CreateHierarchyGraph(), while EdgeCount can legitimately be 0 for a valid design (e.g. a single entity with no yet-analyzed architecture), which was incorrectly reported as 'Hierarchy is not yet computed'. - PackageInstantiation: accept and forward a contextItems parameter to Package.__init__, matching every other primary design unit. Previously, library/use/context clauses preceding a package instantiation design unit had no way to be represented at all. - Regions.IndexDeclaredItems: replace a stray print() with WarningCollector.Raise(NotImplementedWarning(...)), matching the existing warning idiom already used 9x elsewhere in this codebase (pyTooling.Warning based), and consistent with pyGHDL.dom's WarningCollector usage. Added tests/unit/Regression.py covering all fixes.
- GetAncestor: shorten docstring summary to one line, rewrite using a
while/else loop instead of a post-loop None check. Keep the deferred
'from pyVHDLModel.Exception import VHDLModelException' import: moving it
to the top of Base.py creates a genuine circular import
(Base -> Exception -> Symbol -> Base), reproduced and documented in a code
comment.
- AllowBlackbox: drop the docstring example and the defensive
getattr(self, '_parent', None) - AllowBlackboxMixin is always combined with
ModelEntity (verified: every one of its 9 use sites inherits ModelEntity
directly or transitively), so self._parent is guaranteed to exist; only its
value can legitimately be None.
- PackageInstantiation.__init__: apply suggested parameter alignment/
formatting and pass contextItems positionally to super().__init__(), as
requested.
- Regions.IndexDeclaredItems: format the variable identifier list as
single-quoted, comma-separated values via the public Identifiers property
(was using the private _identifiers tuple's repr).
- Test suite restructuring, per review: removed tests/unit/Regression.py and
distributed its tests by subject instead of by 'bug fixed':
* PackageInstantiation contextItems tests -> tests/unit/Instantiate.py
(SimpleInstance), matching the existing test_Package/test_PackageBody
pattern.
* GetAncestor / AllowBlackbox tests -> new tests/unit/Hierarchy.py, since
they traverse the parent chain across multiple classes/levels of the
model rather than testing a single construct in isolation.
* TopLevel / IndexDeclaredItems(Variable-warning) tests ->
tests/unit/Analyze.py, alongside the existing CreateHierarchyGraph/
IndexArchitectures tests they exercise.
Full suite: 73 passed (was 70), no regressions.
Co-authored-by: Patrick Lehmann <Paebbels@gmail.com>
…nd PackageInstantiation context items
Adds a genericAssociations parameter to PackageInstantiation.__init__,
matching the same pattern already used by ComponentInstantiation and
EntityInstantiation in Concurrent.py (self._genericAssociations = [],
appended if not None - no separate mixin storage yet, hence the pre-existing
'TODO: extract to mixin' comment, left as-is).
This is needed by a companion pyGHDL.dom change that reads the generic map
aspect ('generic map (WIDTH => 16)') of a package instantiation and forwards
it here; previously PackageInstantiation always had an empty, hardcoded
GenericAssociations with no way to populate it at all.
Extended tests/unit/Instantiate.py::SimpleInstance::test_PackageInstantiation
to cover genericAssociations alongside the existing contextItems coverage.
Full suite: 73 passed, no regressions.
- Apply suggested parameter/import alignment formatting. - Removed a redundant duplicate 'from pyVHDLModel.Expression import IntegerLiteral' import introduced by mistake (IntegerLiteral was already imported earlier in the same file alongside FloatingPointLiteral). - test_PackageInstantiation: use keyword arguments (GenericAssociationItem(actual=..., formal=...)) instead of positional, and reordered the Formal/Actual assertions to read Formal before Actual (matching 'WIDTH => 16' reading order), per review. Investigated whether AssociationItem.__init__'s (actual, formal=None) parameter order itself should be swapped instead: it's the exact order already used at every production call site of GenericAssociationItem / PortAssociationItem / ParameterAssociationItem in pyGHDL.dom's GetMapAspect() (pyGHDL/dom/_Translate.py), which is exercised for every entity/component instantiation's generic and port maps. Swapping the base class parameter order would ripple through all of those call sites for no functional benefit, so kept it as-is and used keyword arguments at the test call site instead, which fully resolves the readability concern without touching that shared, already-exercised API. Left a comment explaining this in the test. Full suite: 73 passed, no regressions.
Per review feedback: a keyword-argument workaround at the test call site
wasn't the right fix - the constructor's parameter order itself was
misleading and should read the same way VHDL does ('formal => actual').
AssociationItem.__init__ now takes (formal, actual), both required positional
(formal's previous default of None is dropped: every real call site already
passed both arguments explicitly, so nothing relied on the default).
Companion pyGHDL.dom change updates the 3 call sites this touches:
- pyGHDL/dom/Concurrent.py: GenericAssociationItem/PortAssociationItem/
ParameterAssociationItem constructors, now (node, formal, actual).
- pyGHDL/dom/_Translate.py: GetMapAspect(), now yields cls(item, formal,
actual) instead of cls(item, actual, formal).
Verified: full pyVHDLModel suite (73 passed) and, against real GHDL analysis
(nightly libghdl build), the full testsuite/pyunit/dom suite (18 passed,
5 skipped due to missing fixtures in this sparse checkout) - including
StopWatch's real component instantiations with generic/port maps, which
exercise this exact code path and confirm the swap didn't break anything.
…ociationItems
Per discussion: the class held is GenericAssociationItem/PortAssociationItem/
ParameterAssociationItem in every case, so the parameter/property name should
be the plural of the class name, matching genericItems (plural of the
declared-generics class) and mirroring PackageInstantiation's already-fixed
genericAssociationItems.
genericAssociations -> genericAssociationItems (Concurrent.py: Instantiation,
ComponentInstantiation,
EntityInstantiation,
ConfigurationInstantiation;
Instantiation.py: PackageInstantiation)
portAssociations -> portAssociationItems (same four Concurrent.py classes)
parameterMappings -> parameterAssociationItems (Common.py: ProcedureCallMixin;
Concurrent.py: ConcurrentProcedureCall;
Sequential.py: ProcedureCallStatement)
parameterMappings (not '...Associations') was the odd one out - checked
whether it might be intentionally different (VHDL's LRM never calls a
procedure call's actual parameter part a 'map', unlike generic/port map
aspects) but was asked to rename it too for full consistency, so done.
Updated tests/unit/Instantiate.py accordingly. Full suite: 73 passed, no
regressions.
- Applied alignment suggestions (Common.py, Concurrent.py x4) - types and default values aligned at consistent columns. - Fixed PackageInstantiation.__init__ not setting .Parent on genericAssociationItems entries. Concurrent.py's Instantiation base class already does this correctly for the same kind of items (genericAssociationItems/portAssociationItems) - PackageInstantiation was the odd one out. Full suite: 73 passed, no regressions.
…ociationItems for consistency (#128)
…enericItems/parameterItems/IsPure
Function.__init__ never set self._returnType (declared as a type annotation only) despite
Function.ReturnType reading it unconditionally - every Function instance crashed on
.ReturnType access:
>>> Function('f').ReturnType
AttributeError: 'Function' object has no attribute '_returnType'
Subprogram.__init__ (the shared base of Function/Procedure) didn't accept genericItems/
parameterItems/declaredItems/statements as constructor parameters at all - it unconditionally
initialized all four to []. pyGHDL.dom's Function/Procedure had to reach directly into
self._genericItems/self._parameterItems/self._returnType after calling super().__init__(),
bypassing the constructor entirely (marked '# TODO: move to model' in pyGHDL.dom, i.e. the
maintainer of that layer already knew this was a workaround for a gap here).
Subprogram.__init__ now accepts genericItems/parameterItems/declaredItems/statements and wires
Parent correctly on each (matching the pattern already used by WithGenericsMixin/WithPortsMixin
elsewhere - note: Subprogram could plausibly be refactored to compose those mixins instead of
reimplementing generics/parameters storage itself; left as-is for this fix since that's a larger
architectural change, flagging it as worth considering separately).
Function.__init__ now accepts and stores returnType (required - every function has one) and
Procedure/ProcedureMethod/FunctionMethod all forward the new parameters through.
Also fixed ReturnType's type hint: it was declared as pyVHDLModel.Type.Subtype, but every real
caller (pyGHDL.dom) actually passes a Symbol (SimpleSubtypeSymbol) - a reference to the return
type, not an owned type definition. Retyped as SubtypeSymbol to match actual usage.
Companion pyGHDL.dom change updates Function/Procedure to use the fixed constructor properly
instead of the private-field workaround, and additionally reads IsPure from Get_Pure_Flag (was
never read before - every function silently defaulted to IsPure=True regardless of 'impure').
Full suite: 73 passed, no regressions.
ReturnStatement misused ConditionalMixin (which sets self._condition) while ReturnValue reads
self._returnValue - the field was never actually assigned:
>>> ReturnStatement(IntegerLiteral(5)).ReturnValue
AttributeError: 'ReturnStatement' object has no attribute '_returnValue'
Also, ReturnStatement.__init__ didn't accept a label parameter at all, and passed its own parent
argument into Statement.__init__'s label slot (super().__init__(parent), where
Statement.__init__(label=None, parent=None) - so parent ended up in the label position).
Now stores returnValue directly (no mixin misuse), accepts label properly, and forwards both to
Statement.__init__ correctly.
Full suite: 73 passed, no regressions.
VHDLVersion overrides __eq__/__lt__/__le__/__gt__/__ge__/__ne__ without defining __hash__, which
Python implicitly sets to None whenever __eq__ is overridden - making every VHDLVersion member
unhashable and unusable as a dict key or set member:
>>> hash(VHDLVersion.VHDL2008)
TypeError: unhashable type: 'VHDLVersion'
Found while adding VHDL version selection to pyGHDL.dom's Design class (wanted a
{VHDLVersion: '--std option'} mapping).
Hashes by self.value. Documented one caveat: Any compares equal to every other member (see
__eq__), which no hash value can satisfy simultaneously for every member without collapsing all
members to the same hash - this implementation is internally consistent for all comparisons except
those involving Any (avoid using Any as a dict key/set member).
Full suite: 73 passed, no regressions.
- Rebased onto current dev (post #128 merge) - picked up the .Parent wiring fix for PackageInstantiation.genericAssociationItems automatically (already fixed there via #128). - Re-aligned PackageInstantiation's field declarations and __init__ parameters: the genericAssociations -> genericAssociationItems rename made that name/type the longest, breaking the existing column alignment for the whole block. - Aligned all 5 new multi-line constructors in Subprogram.py (Subprogram, Procedure, Function, ProcedureMethod, FunctionMethod) - these were never aligned at all when originally written. - Aligned ReturnStatement.__init__ in Sequential.py, same reason. Full suite: 73 passed, no regressions.
Co-authored-by: Patrick Lehmann <Paebbels@gmail.com>
…ems constructor gap (#129)
…face items
Design discussed and agreed in chat first, then implemented:
- ModeViewSymbol (Symbol.py): reference to a mode view declaration, mirroring
PackageReferenceSymbol exactly. Uses PossibleReference.View, which - along
with .ViewAttribute - was already present in the enum, apparently
anticipating this.
- ModeViewDeclaration, ModeViewElement, SimpleModeViewElement,
CompositeModeViewElement (Interface.py, per 'mode views are used closely to
interfaces'):
- ModeViewElement uses MultipleNamedEntityMixin (mode view elements
support comma-separated identifiers sharing one mode, e.g. 'a, b : out;',
same pattern as Constant/Signal/Variable).
- CompositeModeViewElement merges what GHDL's IIR splits into
Array_Mode_View_Element/Record_Mode_View_Element - verified both are
structurally identical (just a reference to another named view); the
distinction requires resolving the target field's type, which needs
semantic analysis this project doesn't perform.
- ModeViewDeclaration.Subtype is a plain SubtypeSymbol, so 'of
MyArrayType(0 to 7)' (LRM19 constrained array-of-views) is already
supported for free via the existing ConstrainedArraySubtypeSymbol -
no new work needed there.
- Split PortSignalInterfaceItem and ParameterSignalInterfaceItem into
abstract bases with two concrete forms each (Simple/View), since VHDL-2019
mode views apply to signal-class ports and signal-class subprogram
parameters, but not to generics (generics are restricted to
constants/types/subprograms/packages - confirmed against the VHDL grammar
and by the model owner directly). GenericSignalInterfaceItem does not exist
and is out of scope here.
- Obj.__init__ (Object.py) now tolerates subtype=None: a view-typed port has
no separate subtype indication of its own at the parse-only level this
project operates at (verified against real GHDL: Get_Subtype_Indication on
Interface_View_Declaration is Null_Iir before semantic analysis) - the type
is only implied by the referenced mode view. This is additive
(Nullable[Symbol]), not breaking for any existing caller.
Design cross-checked against the VHDL-2017/2019 draft grammar (sigasi.com's
browsable EBNF) before implementation - interface_object_declaration is one
production shared across generic/port/parameter lists; the generics-can't-
be-signals restriction is semantic (LRM prose), not encoded in the grammar
itself, which is why GHDL's parser (grammar-level only, no full semantic
pass) accepts view-typed generics/parameters syntactically without them
being legal VHDL.
Added tests/unit/Interface.py. Full suite: 82 passed (was 73), no
regressions.
- Object.py: reverted the Nullable[Symbol] workaround entirely - you were right, an object's subtype can never be None, there's no VHDL syntax that omits it. Found the actual root cause instead: Obj._subtype is typed as the general Symbol base class, not SubtypeSymbol specifically, and ModeViewSymbol is also a Symbol. VHDL's own grammar treats a mode view indication as occupying the same structural position as an ordinary subtype indication (mode_indication ::= simple_mode_indication | mode_view_indication). So PortViewSignalInterfaceItem/ ParameterViewSignalInterfaceItem now pass the ModeViewSymbol itself as the real, non-None subtype value - Subtype is never None, and ModeViewIndication is just an aliased, more specific name for the same underlying value, not a separate (possibly-None) field. Also removed dead leftover code from the previous version of these two constructors. - Interface.py: fixed 4 docstrings missing a blank line before '.. admonition:: Example' (found by checking all occurrences in the file, not just the one flagged). - doc/LanguageModel/InterfaceItems.rst: this document was indeed stale - checked the whole repo for other PortSignalInterfaceItem/ParameterSignalInterfaceItem construction sites (none found; the split is otherwise fully contained in the class hierarchy itself) but this doc's 'condensed definition' blocks and inheritance-diagram still described the old, now-abstract classes as if concrete. Updated the diagram and added proper sections for Port/ParameterSimpleSignalInterfaceItem and Port/ParameterViewSignalInterfaceItem, matching this file's existing (if still work-in-progress, per its own '.. todo:: Write documentation.' markers) conventions. Did not attempt to fix this file's other pre-existing staleness (e.g. the 'SyntaxModel' module path references) - out of scope here. Updated tests/unit/Interface.py: two assertions were testing the now-corrected (previously broken) behavior - Subtype is the ModeViewSymbol itself, not None. Full suite: 82 passed, no regressions.
alias b is s; previously lost the fact that 'b' aliases 's' entirely - Alias only ever stored its own identifier and documentation, nothing else. Alias.Name (a plain Name, not a *ReferenceSymbol - an alias can refer to almost anything nameable: objects, types, subprograms, literals, so none of the narrower ReferenceSymbol classes fit) and Alias.Subtype (Nullable[SubtypeSymbol] - genuinely optional per the grammar: 'alias_designator [ : subtype_indication ] IS name [ signature ]', unlike Obj.Subtype which is never optional). Added tests/unit/Declaration.py. Full suite: 84 passed (was 82), no regressions.
FunctionInstantiation/ProcedureInstantiation were bare 'pass' stub classes, and SubprogramInstantiationMixin's __init__ took no parameters at all - _subprogramReference was always set to None unconditionally, with no way to populate it (flagged in the code itself: '# FIXME: is this a subprogram symbol?'). - New SubprogramReferenceSymbol (Symbol.py), mirroring PackageReferenceSymbol, using the existing PossibleReference.SubProgram flag. Fixes the exact mismatch flagged in that FIXME: _subprogramReference was typed as the resolved Subprogram entity directly rather than a Symbol wrapper - same class of bug PackageInstantiation had before its own fix. - SubprogramInstantiationMixin now properly accepts and stores subprogramReference and genericAssociationItems (mirroring PackageInstantiation's pattern). - ProcedureInstantiation gets a real constructor. - FunctionInstantiation gets a real constructor too, but deliberately does NOT call Function.__init__ (which would require a mandatory, non-None returnType) - it calls Subprogram.__init__ directly instead and sets _returnType = None. This is NOT a repeat of the Obj.Subtype workaround: verified against real GHDL that Get_Return_Type on a Function_Instantiation_Declaration is Null_Iir - and unlike a plain function, the LRM grammar never lets a subprogram instantiation write its own return type at all; it can only ever be known by resolving the referenced uninstantiated subprogram, which needs semantic analysis this project doesn't perform. None here reflects a not-yet-resolved reference, the same status as any other unresolved Symbol.Reference, not an omission the source syntax would never allow (which is what made the Obj.Subtype case wrong). Added tests to tests/unit/Instantiate.py. Full suite: 86 passed (was 84), no regressions.
…re Name Good catch - Alias.Name should participate in the same resolve-later mechanism (Symbol.Reference/IsResolved) as every other cross-reference in this model, rather than being a bare Name with no resolution slot at all. Also confirmed: with an explicit subtype indication, the LRM restricts an alias to referencing an object (constant/variable/signal/file) specifically - PossibleReference.Object already exists for exactly this. Without a subtype, the target can be almost anything nameable, so the caller should choose a broader PossibleReference value in that case (e.g. PackageMember | EnumLiteral). Updated tests/unit/Declaration.py accordingly. Full suite: 86 passed, no regressions.
…DL.dom but had nowhere to go 'signal s : integer range 0 to 15;' parses fine, but ConstrainedScalarSubtypeSymbol was a bare stub (class ConstrainedScalarSubtypeSymbol(SubtypeSymbol): pass) - no fields, no constructor parameter for a range at all. pyGHDL.dom's ConstrainedScalarSubtypeSymbol.__init__ took an rng: Range parameter but never forwarded it (super().__init__(subtypeName) # , rng) # XXX: hacked), and its .parse() classmethod was a bare 'pass'. Added ScalarConstraint (mirroring ArrayConstraint/RecordConstraint's established pattern for consistency, even though - like those two - it's not reused elsewhere) and gave ConstrainedScalarSubtypeSymbol a real constructor, storing the range via ScalarConstraint.Constraint. Constraint is Nullable - not because the source syntax ever omits a range constraint for a constrained scalar subtype (it never does), but because pyGHDL.dom doesn't yet extract a range from an AttributeName-based constraint (e.g. 'subtype s is t'range;') - a pre-existing, separately- tracked TODO in that function, not something addressed here. Added tests to tests/unit/Instantiate.py. Full suite: 88 passed (was 86), no regressions.
…he ExtendedType metaclass Constraint (and ArrayConstraint/RecordConstraint, pre-existing, plus the new ScalarConstraint) had no metaclass specified at all - inconsistent with every other mixin in this codebase, which all use class XMixin(metaclass=ExtendedType, mixin=True). Fixed all four to follow that exact convention (child mixins just add mixin=True, inheriting the metaclass from Constraint). This was silently 'working' before only because Python resolves the metaclass conflict automatically when one base (SubtypeSymbol, via ExtendedType) is more derived than the other's default type metaclass - not because the declaration was actually correct. Full suite: 88 passed, no regressions. Manually re-verified all three constrained-subtype symbol classes (scalar/array/record) still construct and expose their constraints correctly.
`IndexedObjectOrFunctionCallSymbol` declared `Object | Function`, but the syntax it represents has a third reading: `arr(0)`, `f(0)` and `integer(0)` are written identically, so a parser cannot tell an indexed name from a function call from a type conversion. `Type` and `Subtype` join its possible references, and the doc-string names all three readings. This came out of investigating why `TypeConversion` is never constructed by pyGHDL: GHDL's parser emits `Parenthesis_Name` for all three, and only semantic analysis rewrites one into `Type_Conversion`. `pyGHDL.dom` parses without that pass, so this symbol is what a type conversion actually becomes today - and it was under-declaring that. Co-Authored-By: Patrick Lehmann <Paebbels@gmail.com>
`formatInterfaceItem()` is now `InterfaceItemMixin._FormatInterfaceItem()`, so the shared logic travels with the class hierarchy that already ties these items together instead of sitting beside it as a free function. That is consistent with the convention from #156: a mixin may contribute methods and algorithms; what it must not own is the class's *representation*. `__str__` still lives on each concrete interface item, and the check still reports 0 mixins defining `__str__`/`__repr__`. Co-Authored-By: Patrick Lehmann <Paebbels@gmail.com>
Your wording was better - "is either an ..." states the exclusivity that "is an ..." leaves open. Restored in both the class and the initializer doc-string. The explanation of why the three readings are indistinguishable is now an `.. attention::`, so it stands out from the summary rather than reading as ordinary prose. Co-Authored-By: Patrick Lehmann <Paebbels@gmail.com>
`RecordTypeElement(ModelEntity, MultipleNamedEntityMixin)` had no `DocumentedEntityMixin`, so its initializer was `(identifiers, subtype, parent)` with no `documentation` slot - a doc comment on a record element had nowhere to go, while every other declaration-like class in the model accepts one. The mixin and a `documentation` parameter are added, and the class doc-string's example gains the comment line it can now model. Found via pyGHDL, whose mirror was passing a bare `None` into what is actually the `parent` slot (Paebbels/ghdl#19). The companion change there extracts the comment. Positional compatibility: `documentation` goes before `parent`, matching every other class. The only construction sites are this repo's tests and pyGHDL, and neither passed `parent` positionally. Co-Authored-By: Patrick Lehmann <Paebbels@gmail.com>
Completes the sweep started for `RecordTypeElement`: of the nine named classes without `DocumentedEntityMixin`, four were genuine declarations that could not record a doc comment. * `ModeViewElement` gains the mixin, so `SimpleModeViewElement` and `CompositeModeViewElement` both inherit it; each threads `documentation` through its own initializer. * `DefaultClock` gains it too. The remaining four - `Library`, `Ieee`, `Std`, `PredefinedLibrary` - keep none deliberately: a library is not a source declaration and has no comment to carry. `documentation` goes before `parent` in every signature, matching the rest of the model. No caller passed `parent` positionally. `DefaultClock` had no test at all; it has two now. Co-Authored-By: Patrick Lehmann <Paebbels@gmail.com>
VHDL has no library declaration, so there is nowhere in the source to hang a comment - but a caller of this API may well have one from elsewhere, e.g. a compile-order file or a project description. `Library` gains `DocumentedEntityMixin` and a `documentation` parameter, and **overrides `Documentation` as a settable property**. That is deliberate: every other documented entity gets its text from the source and keeps it read-only, whereas a library's can only ever come from outside, and may not be known when the library is constructed. `PredefinedLibrary` accepts and forwards one too. `Std` and `Ieee` pass none - VHDL defines no documentation for the predefined libraries - but a caller can still set it afterwards. `documentation` sits before `allowBlackbox`, matching `Package` and the rest of the model. The only positional callers were tests passing `None`, which now reads as the documentation slot; those are cleaned up to pass nothing rather than a positional that no longer means what it did. Co-Authored-By: Patrick Lehmann <Paebbels@gmail.com>
pyTooling v8.18.0 rejects a slot that a class member shadows, with `DuplicateFieldInSlotsError`. `UnaryExpression`, `BinaryExpression`, `TernaryExpression` and `RangeExpression` declare `_FORMAT`/`_direction` as annotated instance fields - so they become slots - and 48 derived classes then assign them as class constants in the class body. That is a real defect, not a new rule: the assignment creates a class attribute that hides the slot's descriptor, so the field could be read but never assigned on an instance. Nothing assigns either field per instance; both are constants of the derived class. They are now annotated `ClassVar[...]` at the declaration and at all 48 assignments, which is the resolution the `# FIXME: needs ClassVar[...] when pyTooling gets fixed.` on `TernaryExpression._FORMAT` was waiting for. The dependency floor moves to `pyTooling ~= 8.18`, and it has to: under 8.17.0 a `ClassVar` *without* an initial value still became a slot, so the same declaration fails there with `Slot '_FORMAT' declared in class 'NegationExpression' already exists in base-class`. 8.18.0 is the release that stopped treating a valueless `ClassVar` as a slot. `requirements.txt`, `pyproject.toml` and the three `doc/Dependency.rst` tables are updated together; the grid tables keep their column widths. Verified against pyTooling 8.18+: 461 passed, 69 subtests, and a fresh import of every module raises no `UnannotatedFieldWarning`. `_FORMAT` and `_direction` no longer appear in any `__slots__`, and `__str__` renders unchanged for unary, binary, range and ternary expressions. Co-Authored-By: Patrick Lehmann <Paebbels@gmail.com>
Paebbels
added
Bug: fixed on dev
documentation
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Documentation | 86 minor |
| ErrorProne | 14 high |
🟢 Metrics 562 complexity · 4 duplication
Metric Results Complexity 562 Duplication 4
🟢 Coverage 92.77% diff coverage · +18.51% coverage variation
Metric Results Coverage variation ✅ +18.51% coverage variation Diff coverage ✅ 92.77% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (244c24f) 4407 3103 70.41% Head commit (d82822e) 5219 (+812) 4641 (+1538) 88.93% (+18.51%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#167) 1272 1180 92.77% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Member
Author
|
Do not merge yet — this will fail exactly as the last attempt did.
The description above is the one from #133, unchanged as requested. Two things it does not yet cover, both coming from #166 — say the word and I will fold them in before you merge:
Merge order: #166 → |
Three tests in a new `ClassVariables` class, covering what the shadowed slots actually broke rather than only that construction succeeds: * `test_NotASlot` walks the MRO of one class per arity and asserts neither name appears in any `__slots__`. * `test_Picklable` round-trips an expression of every arity through `pickle`. This is the consequence that made the defect more than cosmetic: `pickle`'s default `__setstate__` writes every slot back, so any model containing an expression was unpicklable. * `test_DirectionIsPerClass` asserts the two range expressions carry their direction as a class constant, readable without an instance. 461 -> 464 passed, 69 -> 82 subtests. Co-Authored-By: Patrick Lehmann <Paebbels@gmail.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #167 +/- ##
===========================================
+ Coverage 68.70% 87.14% +18.43%
===========================================
Files 22 24 +2
Lines 4407 5219 +812
Branches 403 446 +43
===========================================
+ Hits 3028 4548 +1520
+ Misses 1304 578 -726
- Partials 75 93 +18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Paebbels
marked this pull request as draft
July 31, 2026 14:38
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
This release accumulated 32 pull-requests over three months, so the notes below are grouped by theme
rather than enumerating individual classes. The linked pull-requests carry the per-class detail.
The model grew from 384 to 433 classes; the package is +15,743 / -985 lines.
New Features
ModeViewDeclaration,ModeViewElementand the symbols addressing them, plus the split of the port and parameter signal interface items into a simple and a mode view variant, so a port declaredview vis no longer forced through the subtype position. (Add VHDL-2019 mode view classes and split Port/Parameter signal interface items #134)Aliasmodels alias declarations — with and without a subtype indication, which is what decides whether the alias may name anything or only an object. (Subprogram instantiation (VHDL-2008 generic subprograms) and Alias #135)pyVHDLModel.Configurationmodule: configuration declarations, block/component configurations, binding indications and the entity/configuration aspects. This was the last design unit with no model at all. (Add configuration/binding model #137)Rangeis split intoSimpleRange(a to b) andRangeFromName(a range named by an object or subtype), withRangeAttributeSymbolfor'range/'reverse_range. The single class could not represent a range that isn't written as two bounds. (Split Range into SimpleRange and RangeFromName #145)__str__, and an interface item shows its mode (signal p : in bit) — an object's own rendering cannot, since a mode is not part of an object declaration. (Give the primary base classes a__str__#157, Show the interface mode when rendering an interface item #162)Changes
~= 8.17→~= 8.18), inrequirements.txt,pyproject.tomlanddoc/Dependency.rst. This is a hard floor, not a tidy-up: theClassVardeclarations required by the bug fix below do not work under 8.17.0 either, because aClassVarwithout an initial value still became a slot there. There is no formulation compatible with both releases. (Declare_FORMATand_directionasClassVar, require pyTooling 8.18 #166)genericAssociations,portAssociationsandparameterMappingsare nowgenericAssociationItems,portAssociationItemsandparameterAssociationItems. This is a breaking rename of constructor parameters and properties. (Rename genericAssociations/portAssociations/parameterMappings to *AssociationItems for consistency #128)@readonlythroughout. Every property without a setter is marked@readonlyrather than@property. (Exception pattern, @readonly marking and property doc-strings #149, Bring the remaining property doc-strings onto the pattern, and fix stale ones #150)Is…/Has…), and the duplicate class names inIEEE.pyare de-duplicated. (Fix two__str__defects, unify boolean naming, de-duplicate IEEE class names #155)Bug Fixes
Roughly a dozen fixes, most of them crashes on real VHDL that only surfaced once pyGHDL.dom fed the model actual designs:
Function.ReturnType, theSubprogramgeneric/parameter item gap (Fix Function.ReturnType crash and Subprogram genericItems/parameterItems constructor gap #129), andGenericProcedureInterfaceItem/GenericFunctionInterfaceItemon real VHDL-2008 generic subprogram clauses — fixed in Fix GenericFunctionInterfaceItem crash on real VHDL-2008 generic function clauses #143 and again in Fix regression: GenericProcedureInterfaceItem/GenericFunctionInterfaceItem crash again #144 after a regression.ConstrainedScalarSubtypeSymbolhad nowhere to put its range constraint, so the constraint was silently dropped. (Fix ConstrainedScalarSubtypeSymbol: range constraint had nowhere to go #136)NameErroron Python < 3.14 from an annotation left stale by theRangesplit. Python 3.14 defers annotation evaluation (PEP 649), so this was invisible locally and failed on three of the four CI Python versions. (Fix NameError on Python < 3.14 from a stale Range annotation #146)__str__defects producing malformed VHDL. (Fix two__str__defects, unify boolean naming, de-duplicate IEEE class names #155)_FORMATand_directionwere slots, not class variables.UnaryExpression,BinaryExpression,TernaryExpressionandRangeExpressiondeclared them as annotated instance fields, soExtendedTypemade them slots, and the 48 concrete subclasses then assigned them in the class body — which hides the slot descriptor. The field could be read but never assigned on an instance, and sincepickle's default__setstate__writes every slot back, any model containing an expression was unpicklable. Both are nowClassVar[...]. (Declare_FORMATand_directionasClassVar, require pyTooling 8.18 #166)Documentation
This is the largest part of the release.
pyVHDLModelwent from partially documented to fully documented:^^^markers naming the parts that map onto the class's fields. (Document every class, with VHDL examples #151)#:comment — 204 added, taking fields from 78/282 (27.7 %) to 282/282 (100 %). interrogate does not count fields at all, so this closed a gap that was invisible to the coverage number. (Document every class field with a#:comment #152)__init__methods are documented, with the:param:descriptions derived from those field comments so the two stay consistent by construction. Doing the fields first is what made this pass mechanical. (Document all 246__init__methods, exclude nested functions from interrogate #153).. seealso::cross-references link each base class to its derived classes.fail-underis raised 59 → 90 so the gain cannot silently regress. Nested helper functions are excluded (ignore-nested-functions); they are implementation-internal and never rendered by Sphinx.docstr_coverage: 33.71 % → 96.77 %, undocumented items 706 → 40.Three pre-existing documentation bugs were found by checking the generated text against the code rather than by reading it:
InterfaceGroup.__init__was titled for the wrong class and documented none of its parameters,BaseType.__init__was missing a parameter, andDesign.__init__listed its parameters in the wrong order.Unit Tests
Parentverified. This sweep is what found most of the bugs listed above. (Add level-1 construction/property/parent-wiring tests across the model #139)Instantiationtest package. (Namespace tests, ExtendedKeyError chaining, and the Instantiation test package #147)ClassVariablestest class asserts what the shadowed slots actually broke: neither name appears in any__slots__along the hierarchy, an expression of every arity round-trips throughpickle, and the two range expressions carry their direction as a class constant. (Declare_FORMATand_directionasClassVar, require pyTooling 8.18 #166)Related Issues and Pull-Requests
#:comment #152__init__methods, exclude nested functions from interrogate #153__str__defects, unify boolean naming, de-duplicate IEEE class names #155__str__#157_FORMATand_directionasClassVar, require pyTooling 8.18 #166Companion work in pyGHDL.dom (
Paebbels/GHDL, branchpaebbels/dom-updates) translates GHDL's IIR tree into the classes added here; it depends on this release being published to PyPI.