Hi Eliot, hi all,<br>
<br>
have a wonderful new year, and thank you for the feedback! Again, I/we can change this of course until the code is perfect. :)<br>
<br>
please continue reading below ...<br>
<br>
On 2021-12-31T09:14:04+01:00, marcel.taeumel@hpi.de wrote:<br>
<br>
> > Instead use this pattern:(self objectClass: meth) isCompiledCodeClass<br>
> <br>
> +1<br>
> <br>
> > But do you *really* need to deal with proxies here? If not, then *please* revert to the simple and rational meth isCompiledCode.<br>
> <br>
> +1<br>
> <br>
> Best,<br>
> Marcel<br>
> Am 30.12.2021 20:01:54 schrieb Eliot Miranda <eliot.miranda at gmail.com>:<br>
> Hi Christoph,<br>
> <br>
> On Tue, Dec 28, 2021 at 12:13 PM <commits at source.squeak.org [mailto:commits at source.squeak.org]> wrote:<br>
> <br>
> Christoph Thiede uploaded a new version of Kernel to project The Trunk:<br>
> http://source.squeak.org/trunk/Kernel-ct.1438.mcz [http://source.squeak.org/trunk/Kernel-ct.1438.mcz]<br>
> <br>
> ==================== Summary ====================<br>
> <br>
> Name: Kernel-ct.1438<br>
> Author: ct<br>
> Time: 28 December 2021, 9:13:10.136249 pm<br>
> UUID: 0477e714-e01d-d34d-b6aa-0939494a4abc<br>
> Ancestors: Kernel-mt.1437, Kernel-ct.1403<br>
> <br>
> Merges Kernel-ct.1403 (fixes simulation of dynamic-forwarding objects as methods).<br>
> <br>
> =============== Diff against Kernel-mt.1437 ===============<br>
> <br>
> Item was changed:<br>
> ----- Method: Context>>send:to:with:lookupIn: (in category 'controlling') -----<br>
> send: selector to: rcvr with: arguments lookupIn: lookupClass<br>
> "Simulate the action of sending a message with selector and arguments to rcvr. The argument, lookupClass, is the class in which to lookup the message. This is the receiver's class for normal messages, but for super messages it will be some specific class related to the source method."<br>
> <br>
> + | meth methClass primIndex val ctxt |<br>
> - | meth primIndex val ctxt |<br>
> (meth := lookupClass lookupSelector: selector) ifNil:<br>
> [selector == #doesNotUnderstand: ifTrue:<br>
> [self error: 'Recursive message not understood!!' translated].<br>
> ^self send: #doesNotUnderstand:<br>
> to: rcvr<br>
> with: {(Message selector: selector arguments: arguments) lookupClass: lookupClass}<br>
> lookupIn: lookupClass].<br>
> <br>
> + ((methClass := self objectClass: meth) == CompiledMethod or: [methClass == CompiledBlock]) ifFalse:<br>
> <br>
> <br>
> Again this cannot be allowed to stand. It prevents adding subclasses of CompiledMethod & CompiledBlock, which is definitely something one would want to allow. Instead use this pattern:<br>
> <br>
> (self objectClass: meth) isCompiledCodeClass<br>
<br>
Ah! Of course. There is no need to assume that primitiveClass would answer a proxy object. :)<br>
<br>
> But do you *really* need to deal with proxies here? If not, then *please* revert to the simple and rational meth isCompiledCode.<br>
<br>
I will respect your eventual decision, but please give me another chance to argue for proxy-safe code in this place: There are not many implementations of the Objects as Methods pattern at all, but within these implementations, transparent proxies do not appear as something very unusual to me. I would even argue that transparent proxies are be the best practice here because otherwise, implementors are likely to break the default code browsing tools and literal search in the entire image [1].<br>
In current Trunk, 2 out of 4 #run:with:in: implementors also override #doesNotUnderstand: and thus will not work during debugging. The pretty common (?) MethodWrappers package [2] does so, too.<br>
If we add support for OaM in the image-side simulator, why not add full support? Is primitiveClass more expensive than I was assuming? :)<br>
<br>
Here is a concrete example that you can evaluate but not debug without the current patch:<br>
<br>
<font color="#000000">Object</font><font color="#000000"> </font><font color="#000080">compile:</font><font color="#000000"> </font><font color="#800080">'foo ^42'</font><font color="#000000">.</font><font color="#000000"><br>
</font> <font color="#000000">cov</font><font color="#000000"> </font><b>:=</b><font color="#000000"> </font><font color="#000000">TestCoverage</font><font color="#000000"> </font><font color="#000080">on:</font><font color="#000000"> </font><font color="#000000">(</font><font color="#000000">Object</font><font color="#000000"> </font><font color="#000080">>></font><font color="#000000"> </font><font color="#000080">#foo</font><font color="#000000">)</font><font color="#000000"> </font><font color="#000080">asCodeReference</font><font color="#000000">.</font><font color="#000000"><br>
</font> <font color="#000000">cov</font><font color="#000000"> </font><font color="#000080">install</font><font color="#000000">.</font><font color="#000000"><br>
</font> <font color="#008080">"[Context runSimulated: [self foo]] ensure: [cov uninstall]."</font><font color="#000000"><br>
</font> <font color="#800000">self</font><font color="#000000"> </font><font color="#000080">foo</font><font color="#000000">.</font><font color="#000000"><br>
</font> <font color="#800000">self</font><font color="#000000"> </font><font color="#000080">assert:</font><font color="#000000"> </font><font color="#000000">cov</font><font color="#000000"> </font><font color="#000080">hasRun</font><font color="#000000">.</font><br>
<br>
> Remember you can always have overrides for your own project, You don't need to inflict these special cases on the community code base (until they're fully justified). For example, in Virtend (which used to be called Terf) we have several packages we load on top of Squeak 5.3, TerfExtensions-Kernel, TerfExtensions-Collections ,etc. By organising like this we keep related changes organised by package and we don't have to disturb 5.3.<br>
<br>
That is a good advice and one I'm already trying to follow, but I still feel the desire to contribute a maximum of conveniences and bugfixes upstream to the Trunk because I want other projects and people benefit from them as well. Provided that I do not disturb anyone, of course. :)<br>
Besides, a concrete problem with too many override method can be that they intermix and shadow each other if I need to add the same override method to multiple packages (for instance, if we decide to revert this patch, I will feel tempted to override #send:to:with:lookupIn: not only in SimulationStudio but also in MethodWrappers, which will make it tricky to load both of them into one image).<br>
<br>
> <br>
> - meth isCompiledMethod ifFalse:<br>
> ["Object as Methods (OaM) protocol: 'The contract is that, when the VM encounters an ordinary object (rather than a compiled method) in the method dictionary during lookup, it sends it the special selector #run:with:in: providing the original selector, arguments, and receiver.'. DOI: 10.1145/2991041.2991062."<br>
> ^self send: #run:with:in:<br>
> to: meth<br>
> with: {selector. arguments. rcvr}].<br>
> <br>
> meth numArgs = arguments size ifFalse:<br>
> [^ self error: ('Wrong number of arguments in simulated message {1}' translated format: {selector})].<br>
> (primIndex := meth primitive) > 0 ifTrue:<br>
> [val := self doPrimitive: primIndex method: meth receiver: rcvr args: arguments.<br>
> (self isPrimFailToken: val) ifFalse:<br>
> [^val]].<br>
> <br>
> ctxt := self activateMethod: meth withArgs: arguments receiver: rcvr.<br>
> (primIndex isInteger and: [primIndex > 0]) ifTrue:<br>
> [ctxt failPrimitiveWith: val].<br>
> <br>
> ^ctxt!<br>
> <br>
> <br>
> <br>
> <br>
> <br>
> --<br>
> <br>
> _,,,^..^,,,_<br>
> <br>
> best, Eliot<br>
> -------------- next part --------------<br>
> An HTML attachment was scrubbed...<br>
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20211231/b1fe9232/attachment.html><br>
<br>
Looking forward to your feedback!<br>
<br>
Best,<br>
Christoph<br>
<br>
[1] http://forum.world.st/BUG-Objects-as-methods-break-method-search-td5112346.html<br>
[2] https://github.com/hpi-swa/MethodWrappers<br>
<br>
<font color="#808080">---<br>
</font><font color="#808080"><i>Sent from </i></font><font color="#808080"><i><a href="https://github.com/hpi-swa-lab/squeak-inbox-talk"><u><font color="#808080">Squeak Inbox Talk</font></u></a></i></font>