objects_finalized template#437
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an objects_finalized lifecycle hook/template for DML 1.4 devices that runs after successful configuration, updates code generation to invoke it, and documents the behavior alongside immediate-after semantics.
Changes:
- Introduce
objects_finalizedtemplate in DML 1.4 builtins and wire it into thedevicetemplate. - Update C backend generation to invoke
_objects_finalizedbefore executing immediate-after callbacks. - Add a DML+Python test plus documentation/release note updates describing the new hook.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/1.4/lib/T_objects_finalized.py | Adds a test that asserts objects_finalized() ran for device and subdevice instances. |
| test/1.4/lib/T_objects_finalized.dml | Defines a test device/subdevice using the new objects_finalized template and verifies immediate-after ordering. |
| py/dml/c_backend.py | Emits a call to _objects_finalized in the generated Simics objects_finalized hook (for non-1.2). |
| lib/1.4/dml-builtins.dml | Adds the objects_finalized template and integrates it into the built-in device template. |
| doc/1.4/language.md | Documents that immediate-after callbacks during configuration run during the Simics objects_finalized hook after objects_finalized() calls. |
| RELEASENOTES-1.4.md | Notes the addition of the objects_finalized template/hook. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| `init` and `post_init`, representing the stage after the device has been | ||
| created. Once Simics has successfully completed configuring the device and | ||
| all other objects created at the same time as it, `objects_finalized()` will | ||
| be called of every object that instantiates the `objects_finalized` template. |
There was a problem hiding this comment.
Grammar fix: 'called of every object' should be 'called on every object'.
| be called of every object that instantiates the `objects_finalized` template. | |
| be called on every object that instantiates the `objects_finalized` template. |
There was a problem hiding this comment.
Hm. No, I don't think I agree with that, each objects_finalized method called are of the object that instantiates the template. @mandolaerik your opinion?
There was a problem hiding this comment.
In that case you should spell out the objects_finalized() method.
Maybe it's smoother to read if you avoid passive form (will be called) and instead write smth like Simics will call or Simics calls. Also consider "all other objects created at the same time as it" => "all other simultaneously created objects"
lwaern-intel
force-pushed
the
lw/objects_finalized
branch
from
22a583f to
a24eee2
Compare
|
PR Verification: ✅ success |
|
PR Verification: ✅ success |
| [`post_init()`](dml-builtins.html#post_init), or | ||
| [`set()`](dml-builtins.html#attribute-objects) of a configured or checkpointed | ||
| attribute), then the callback will be executed as part of the | ||
| `objects_finalized` hook of `SIM_create_class`, just after the automatic calls |
There was a problem hiding this comment.
Maybe replace hook with some other word: hook is a thing in DML, and it's not obvious to the reader that we did not define a DML hook for objects_finalized.
Something like "will be executed when SIM_create_class calls the objects_finalized function of class_info_t"?
| Provides an abstract method `objects_finalized`, which is called once both the | ||
| device and all other objects created at the same time as it have been | ||
| successfully configured, such that they are ready to be communicated with. | ||
|
|
There was a problem hiding this comment.
The sentence is correct but somewhat heavy, which is OK for a technical definition I guess. Maybe s/be communicated with/communicate [with other devices]/?
It would be good to also have a separate section somewhere that discusses the different instantiation primitives and how you partition code between them. Not part of this PR though.
| assert SIM_object_is_configured(dev.obj); | ||
| assert SIM_object_is_configured(partner.obj); |
There was a problem hiding this comment.
move to s.objects_finalized(), which is called before?
| subdevice s is objects_finalized { | ||
| saved bool objects_finalized_done; | ||
| method objects_finalized() { | ||
| objects_finalized_done = true; |
There was a problem hiding this comment.
maybe also assert dev.objects_finalized_done == false;? as a stability test that post-order traversal is used.
|
Looks good overall |
No description provided.