[zeek/spicy] 682692: Make the visitors' `replaceNode()` safe.

0 views
Skip to first unread message

Robin Sommer

unread,
Feb 25, 2026, 6:55:28 AMFeb 25
to spicy-...@zeek.org
Branch: refs/heads/topic/robin/replace-node
Home: https://github.com/zeek/spicy
Commit: 682692fc635b55e54bd2666ebbcdcc2ccabe1d59
https://github.com/zeek/spicy/commit/682692fc635b55e54bd2666ebbcdcc2ccabe1d59
Author: Robin Sommer <ro...@corelight.com>
Date: 2026-02-25 (Wed, 25 Feb 2026)

Changed paths:
M hilti/toolchain/include/ast/visitor.h
M hilti/toolchain/include/compiler/detail/optimizer/pass.h
M hilti/toolchain/src/ast/visitor.cc
M hilti/toolchain/src/compiler/optimizer/pass.cc
M hilti/toolchain/src/compiler/optimizer/passes/constant-propagation.cc
M hilti/toolchain/src/compiler/optimizer/passes/dead-code-static.cc
M hilti/toolchain/src/compiler/optimizer/passes/peephole.cc
M hilti/toolchain/src/compiler/optimizer/passes/propagate-function-returns.cc
M hilti/toolchain/src/compiler/optimizer/passes/remove-unused-fields.cc
M hilti/toolchain/src/compiler/resolver.cc
M spicy/toolchain/src/compiler/codegen/codegen.cc
M tests/Baseline/spicy.optimization.default-parser-functions/log
M tests/Baseline/spicy.optimization.feature_requirements/log
M tests/Baseline/spicy.optimization.unused-functions/log
M tests/Baseline/spicy.optimization.unused-types/log

Log Message:
-----------
Make the visitors' `replaceNode()` safe.

The existing `replaceNode()` method was impossible to use safely: if
it was passed a replacement node that was already part of the AST
somewhere, the method would disconnect that node from its original
position first. That, however, is generally not safe because AST nodes
do not give any guarantees about their internal child layout, so that
simply removing one can lead to trouble. Plus, one always had to keep
mind that the replacement node would now disappear from its original
place, which is error-prone even in safe cases.

This changes `replaceNode()` to instead use our standard semantics
when making AST modifications: If a node is being inserted that
already has a parent, we deep-copy it first. That way, the caller
doesn't need to worry about safe memory management. In addition, we
add a new method `replaceNodeWithChild()` that optimizes the operation
for a special case: If an existing child is taking the position of a
parent of itself, then we can always safely move it over into its new
place without copying. We now use that for cases across the code base
that match this special case.



To unsubscribe from these emails, change your notification settings at https://github.com/zeek/spicy/settings/notifications
Reply all
Reply to author
Forward
0 new messages