[Vm-dev] VMMaker small problem for pharo developper

Henrik Johansen henrik.s.johansen at veloxit.no
Fri Mar 19 09:44:47 UTC 2010

On Mar 19, 2010, at 4:48 27AM, David T. Lewis wrote:

> On Thu, Mar 18, 2010 at 02:08:36PM +0100, Henrik Johansen wrote:
>> On Mar 18, 2010, at 1:26 10PM, Jean Baptiste Arnaud wrote:
>>> Then what is the solution ? We could have a Pharo immutability bit VM, but that it can be better to share with squeak. (which should be easy, if these differences are fixed ).
>> I agree.
>> One solution for the case I mentioned is to not hardcode where the primitives in such cases as MiscPrimitivePlugin are defined, but instead discover them in the image VMMaker is loaded into.
>> Thus you remove the link between VMMaker, and the image it's loaded into defining primitives in certain places, so the same VMMaker can work for both Squeak and Pharo as described.
>> F.ex. like this:
>> MiscPrimitivePlugin class>>translatedPrimitives
>> oc := OrderedCollection new.
>> CompiledMethod allInstancesDo: [:each | 
>> 	(each pragmaAt: #primitive:module:) ifNotNil: [:prag | 
>> 		((prag  argumentAt: 2) = 'MiscPrimitivePlugin'  and: [each literals includes: #var:declareC:]) 
>> 			ifTrue: [ oc add: (Array with: each methodClass name with: each selector)]]].
>> ^oc asArray
>> This assumes the method defining the primitive will include the primitive pragma, which should always be the case.
>> The "each literals includes: #var:declareC:" is a bit of a hack for disambiguating the CompiledMethod defining the primitive from other callers.
>> AFAIK, hints like that are required if the method is to be translated, and you are unlikely to find it in other callers. If anyone has a better idea, I'd appreciate it.
> Henry,
> Thank you for opening a mantis report on this at
>  http://bugs.squeak.org/view.php?id=7479
> I think your pragma idea may work, but IMO this is getting too
> complicated.  Perhaps the change in Pharo was done without realizing
> the problem it would cause for C code generation.
Well sure, I doubt that was a consideration.
However, imo it is the *right* way to fix the issue, by keeping the contract of the public method, but factoring out the act of calling the primitive (which may not fail, but return erroneous results), and implementing the guard in the "old" primitive method.
What do you think will happen in Squeak if an external package uses findSubstring: directly? I sincerely doubt they will be aware of its limitations, and have the proper guards in place.

> So that means
> that the primitive is already too clever - it is easy for someone to
> look at the Smalltalk method and not realize that it is designed to
> be translated to C. I am afraid that adding pragmas will only make
> it harder for people to understand how this works.

I suppose you refer to adding pragma usage when resolving locations, since the pragmas themselves are already there. 

Using them for resolving was merely the simplest way I could think of which would resolve the primitive definition location for VMMaker, while not hardcoding them.

In sentences describing the functionality of translatedPrimitives, it changes from "These are the hardcoded locations where my methods are defined" to "My methods are defined in places where my primitives are called, and there also exists c-declaration hints". 
Personally, I don't really see how that is much harder to understand.

> So my opinion FWIW is that if we really need different methods in
> the Squeak and Pharo images, then maybe it is time to reimplement
> this primitive as a normal primitive, rather than using the magic
> of MiscPrimitivePlugin translating methods directly from the image.

That'd probably also be a good change in the sense you don't need to know which image VMMaker was loaded into when the plugin was built to know how it works.
How do you redirect images where the MiscPrimitivePlugin method is called, and not the primitive though?
> But to be honest I am not convinced that the Pharo method really needs
> to be renamed to #findSubstringViaPrimitive:in:startingAt:matchTable:.
> Perhaps there is some other way to accomplish this refactoring without
> causing compatibility problems?

If you mess with findSubstring:in:startingAt:matchTable: to take care the WideString case is handled correctly, AFAICT you inevitably end up with a solution which would either mean:
- It does not end up using the primitive it defines.
- It breaks building the primitive with VMMaker. 

So the only other option, is to put  guard clauses in all senders...
Squeak does it. As I noted, in fact  Pharo does it as well, but shouldn't, as it already have them centralized in the old findSubstring: method. 
To me, it does not seem like the right way though, unless your image is static ,and you can ascertain noone else will ever call findSubstring:in:startingAt:matchTable: .


FWIW, as discussed in an earlier thread, the method will probably be renamed again in Pharo, to #primFindSubstring:in:startingAt:matchTable: 

More information about the Vm-dev mailing list