On Thu, Sep 15, 2011 at 1:00 PM, Nicolas Cellier < nicolas.cellier.aka.nice@gmail.com> wrote:

2011/9/15 Eliot Miranda <eliot.miranda@gmail.com>:

...

On Thu, Sep 15, 2011 at 9:45 AM, Mariano Martinez Peck <marianopeck@gmail.com> wrote:

...

Ok, I understand. But

On Thu, Sep 15, 2011 at 6:05 PM, Henrik Sperre Johansen <henrik.s.johansen@veloxit.no> wrote:

...

On 15.09.2011 17:27, Mariano Martinez Peck wrote:

Hi guys. I am having a problem with the integration of issue 4538: http://code.google.com/p/pharo/issues/detail?id=4538

I am serializing CompiledMethods with Fuel and then I materialize them. To test they are correct, I use #= So far it was working correctly, but now for the materialized method it says they are not equal. The problem is that #= is failing in

(self sameLiteralsAs: aCompiledMethod)

And inside that method, it is failing because (literal1 == literal2 or: [ literal1 literalEqual: literal2 ]) answers false.

So... why literal1 literalEqual: literal2 answers false? from what I

can

...

...

...

say, having that selector: "literalEqual", then both MUST be equal, because they are. The problem is that it was added:

Association >> literalEqual: otherLiteral "Answer true if the receiver and otherLiteral represent the same literal. Variable bindings are literally equals only if identical. This is how variable sharing works, by preserving identity and changing only the value." ^self == otherLiteral

So....I am not sure I agree with this change.

Any idea how can we deal with this problem?

Cheers

-- Mariano http://marianopeck.wordpress.com

The change is correct, here's an example:

Create a global: Global := 1

Create a method: foo ^Global

Serialize method, deserialize

Change value of global: Global := 5.

foo should return 5, not 1. Unless it's actually the same association as in the SystemDictionary, this will not be true.

I understand that. In fact, in Fuel we use exactly the same association of SystemDictionary for globals. Look this test example:

testGlobalVariableMethod

| materializedCompiledMethod | Smalltalk globals at: #TestGlobalVariableMethod2 put: false. (self class compileSilently: 'globalVariableForTestingMethod Transcript name. ^ GlobalVariableForTesting.').

materializedCompiledMethod := self materializedCompiledMethod: (self class >> #globalVariableForTestingMethod). Smalltalk globals at: #GlobalVariableForTesting put: true. self assert: (materializedCompiledMethod valueWithReceiver: self arguments: #()).

BUT, it doesn't mean that Association is always used for globals. CompiledMethod equality is failing because of the last literal, the one that maps class name (symbol) and point to the real class. So...when I materialize, both CMs have non-identical associations for the last literal, but equal.

As Henrik says the last literals are ideally #== to each other. However, no Squeak dialect makes any attempt to keep the class0side associations equal. Look at a class-side method and you'll see it's last literal is nil->SomeClass class. Now since this association doesn't exist in Smalltalk (unlike last literals on the instance side) the compiler merely creates distinct ones for each class-side method. Personally I don't think one can defend the position where method equality is different for instance-side or class-side methods so there must be some solutions:

Hmm, good catch. A metaclass is never accessed by dictionary lookup, but only by sending #class to a class, so there is no point in maintaining a unique Association. (otherwise, we could maintain such Association in inst. var. thisClass). Having a nil key is a clear indication that lookup is pointless.

The question is why having an association at all in the CompiledMethod ? For handling super sends ?

Yes, but two reasons. One is super sends and the other is being able to answer the methodClass (e.g. for CompiledMethod>>printOn: and for the debugger). One doesn't need to use an association, but one can't change that without changing al the VMs /and/ changing the ClassBuilder so that when it becomes a class on class redefinition the methodsa are updated. Indeed VisualWorks does exactly this. In any case, the super send implementation in the VM means we can't change this overnight :)

I think a simple reference to the class would be enough. IMHO, the purpose was to simplify implementation.

And I don't think the example of Henrik is worth : Lets just change it a bit:

Object subclass: #SuperFoo.! Object subclass: #Bar.! SuperFoo subclass: #Foo.!

SuperFoo compile: 'bar ^1'. Foo compile: 'bar ^super bar *2'. foo := Foo new. Smalltalk at: #Foo put: Bar. ^foo bar

Could you predict the result (will it try to invoke super Bar bar) ? Yes, since the last association is shared, we just broke (foo class>>bar) for no reason...

Right. But Rube Goldberg machines work like Rube Goldberg machines. Tis the nature of the beast. Doctors say "don't do that", and in Smalltalk we should say "what did you expect"? :) Being able to change the system is worth the cost of it behaving contrary to common-sense. Just like the physical world :)

Nicolas

...

1. special case comparison of the last literal (the methodClass literal), comparing keys and insisting that the keys be #== and the values be #== (can't just define it as keys #== since all similar class-side methods will be equal irrespective of their actual class). 2. special case comparison of the last literal (the methodClass literal), insisting only that the class of the literal be the same if it isVariableBinding. 3. make the compile unique class-side methodClass literals. i.e. if a class already has a class-side method then the compiler or method dictionary insertion code must find that existing association and reuse it Other ideas?

...

...

From my point of view, that literal, the last one does not need to be identical to assume 2 CMs are equal. They just need to be equal.

-- Mariano http://marianopeck.wordpress.com

-- best, Eliot

-- best, Eliot