Support WeakReferences in NodeConnectingVisitor #1057
Conversation
|
I think in CloningVisitor we don't need a weak-ref because we clone the elements, right? |
| if ($this->weakReferences) { | ||
| $parent = \WeakReference::create($parent); | ||
| } | ||
| $node->setAttribute('parent', $parent); |
There was a problem hiding this comment.
One thing I'm wondering about here is whether we should reuse the same attribute names, or use a different one like 'weak_parent', to avoid having two possible types for these attributes.
There was a problem hiding this comment.
We could use separate attribute names, but then I would go with a separate WeakNodeConnectingVisitor class and leave NodeConnectingVisitor untouched ..?
There was a problem hiding this comment.
Given that the implementation themselves are pretty trivial, I'd be fine with adding separate classes as well.
There was a problem hiding this comment.
I am fine with the PR as is, in case you are also ok with it
closes #1055
we observed perf improvements on a similar visitor in staabm/phpstan-dba#667 (comment)