Record which of the user's own calls reached each body (idToCallSite) - #180
Merged
Merged
Conversation
idToNode names the node that PRODUCED a body, which for anything a library
builds is a node inside that library -- a plain cube(10) maps into BOSL2's
builtins.scad the moment BOSL2 is included, since BOSL2 overrides the
primitives with its own modules. A picker cannot point the user at their
own source with it, and a consumer that forgets to check origin splices a
library's byte offsets into the user's buffer (BelfrySCAD #450).
idToCallSite maps each originalID to the call in the USER's own file that
reached it, or nullptr for geometry written at top level, where idToNode
is already the user's node.
Nothing new had to be computed. CSGNode::warnEntry already captures
callStack_.front().callPosition at resolve time, precisely so a warning
raised during generate -- after the stack has unwound -- can still name
the user's line. This records it per ID as well as per node.
A cache hit restamps to the site REUSING the geometry rather than the one
that first produced it: a module called twice genuinely is two call sites,
and that is the line the user would be shown for either copy.
It resolves to the call, not the body. `module bracket() { cuboid(10); }
bracket();` attributes to `bracket();`, because wrapping the body would
move every instance.
Exposed as node.call_site on each id_to_node entry. The id-span tuple the
binding returns grows from 5 fields to 11; the facade in this package is
its only consumer.
Also ignores build-tbb/, a build directory the .gitignore did not cover.
…ment
Review of the first commit: selecting the statement that entered the chain
is the wrong end of it. `translate([20,0,0]) cuboid(8, rounding=1);`
attributed to the whole statement, so clicking the cuboid highlighted the
translate too -- and, because the span then started BEFORE that translate,
a gizmo's backwards-looking merge could not see it and added a second
wrapper instead of updating the first.
currentUserCallEntry() takes the innermost frame still in the user's own
file. currentWarnEntry() keeps the outermost, unchanged: a warning wants
the top-level statement to look at, a click wants the line that placed
that object. CSGNode carries both, captured at the same five resolve
sites, and generate republishes both.
translate([20,0,0]) cuboid(8, rounding=1); -> 'cuboid(8, rounding=1);'
module inner() { boxy(8); } ... outer(); -> 'boxy(8);'
The merge regex sees ('20','0','0') again with the span where it now is.
No stored script path was needed to tell the user's file from a library:
callStack_.front() is by construction the call made from top level, so its
origin is the file being run.
Known consequence, documented rather than worked around: geometry from a
module called twice attributes to the same line in that module's body both
times, so an edit there moves every instance. Distinguishing instances is
what walking the selection outwards is for (BelfrySCAD #455).
…epping into
Filtering to the user's own file was wrong. A single cuboid() call is 24
frames -- cuboid -> attachable -> _attach_transform -> _find_anchor, most
of them inside BOSL2 -- and someone writing BOSL2 wants to step into
exactly those. Which frames a front end can show depends on what it has
open, which the evaluator has no business guessing, so it now records all
of them and the consumer picks its level.
That also removes the origin comparison currentUserCallEntry() was doing:
the evaluator no longer needs any notion of "the user's file".
Stored as a cactus stack rather than a list per node. Chains nest, so the
distinct chains over a run form a tree: callChains_ holds {site, parent,
isModule} and a CSGNode holds one uint32 into it. Memory tracks distinct
call PATHS -- tens on a real model, where Dalek's 139 bodies share 13
innermost sites -- not CSG nodes, and nothing allocates per node. That is
what makes this affordable where csg_node.hpp had ruled out "the full
frame list a TRACE would need".
isModule separates a module frame, which has geometry behind it, from a
function frame like _find_anchor, which does not: both are worth showing,
only the former is worth dragging.
Exposed as node.call_sites, innermost-first, with node.call_site kept as
the innermost for a caller that does not care about the chain.
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
1 participant
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.
Groundwork for BelfrySCAD #451. No behaviour changes here; this adds the attribution the picker needs.
Problem
idToNodenames the node that produced a body. For anything a library builds that is a node inside the library — and a plaincube(10)maps into BOSL2'sbuiltins.scadthe moment BOSL2 is included, because BOSL2 overrides the primitives with its own modules:A picker cannot point the user at their own source with that, and a consumer that forgets to check
originsplices a library's byte offsets into the user's buffer — which is how BelfrySCAD #450 appended atranslate()to the end of a 59-character script.Change
idToCallSitemaps eachoriginalIDto the call in the user's own file that reached it, ornullptrfor geometry written at top level, whereidToNodeis already the user's node.Nothing new is computed.
CSGNode::warnEntryalready capturescallStack_.front().callPositionat resolve time, precisely so a warning raised during generate — after the stack has unwound — can still name the user's line. This records it per ID as well as per node.Behaviour
Two decisions worth reviewing:
module bracket() { cuboid(10); } bracket();attributes tobracket();, because wrapping the body in a transform would move every instance.idToNodeis restamped, where keeping the original producer is the honest answer.API
Exposed as
node.call_siteon eachid_to_nodeentry — a_PositionorNone.The id-span tuple the binding returns grows from 5 fields to 11. The facade in this package is its only consumer, and it is updated here.
Tests
Two new binding tests: one builds a private library so the producing node is provably inside it and asserts every body attributes back to
wrapped(10);in the user's file; one asserts top-level geometry has no call site at all.C++ suite 1263 passed, bindings 44 passed.
Also adds
build-tbb/to.gitignore, a build directory it did not cover.🤖 Generated with Claude Code