On Thu, Sep 15, 2011 at 1:00 PM, Nicolas Cellier < [email protected]> wrote:
> 2011/9/15 Eliot Miranda <[email protected]>: > > > > > > On Thu, Sep 15, 2011 at 9:45 AM, Mariano Martinez Peck > > <[email protected]> wrote: > >> > >> Ok, I understand. But > >> > >> On Thu, Sep 15, 2011 at 6:05 PM, Henrik Sperre Johansen > >> <[email protected]> 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
