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.
lwaern-intel
force-pushed
the
lw/objects_finalized
branch
from
22a583f to
a24eee2
Compare
|
PR Verification: ✅ success |
|
PR Verification: ✅ success |
| 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?
There was a problem hiding this comment.
Eh. s is really just a test that objects_finalized is recursively invoked using the same mechanism as init, post_init, etc. For readability, I prefer all the "main" tests are centralized in top-level init(). Unless you insist.
There was a problem hiding this comment.
What I meant: s.objects_finalized is called before, so if the intent is to check that it's set early then it is slightly stronger to test that it's configured in s.objects_finalized vs dev.objects_finalized.
I do not insist, the gain is miniscule and if you prefer current for readability then that's probably worth more.
|
Looks good overall |
lwaern-intel
force-pushed
the
lw/objects_finalized
branch
from
a24eee2 to
38301e5
Compare
|
DML Verification : 6: ❌ failure |
|
DML Verification : 7: ❌ failure |
lwaern-intel
force-pushed
the
lw/objects_finalized
branch
2 times, most recently
from
692235d to
2adb363
Compare
|
PR Verification: ❌ failure |
|
PR Verification: ✅ success |
lwaern-intel
force-pushed
the
lw/objects_finalized
branch
from
2adb363 to
d16c5eb
Compare
…zed` This *was* possible, contrary to the guarantee we now explicitly document, if `SIM_process_pending_work()` were to be called as part of object configuration. I don't know why in the world anyone would *ever* do that, but hey, now it wouldn't cause issue.
lwaern-intel
force-pushed
the
lw/objects_finalized
branch
from
d16c5eb to
164dcb3
Compare
|
PR Verification: ✅ success |
|
PR Verification: ✅ success |
No description provided.