[squeak-dev] The Trunk: Compiler-eem.380.mcz

Tobias Pape Das.Linux at gmx.de
Sat Mar 31 12:06:55 UTC 2018


Hi Nicolas,

> On 23.03.2018, at 18:23, Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com> wrote:
> 
> Hi Tobias,
> Yes, some months ago I changed Undeclared to automagically clean-up when bindings are no more used.
> This was necessary to workaround some Environment bugs, and it is a nice feature anyway.
> So, if you want some Undeclared binding to persist, then you must have a strong pointer on it.
> IOW, there should be some class with source code refering to this undeclared binding...
> From what I see, there is such reference in http://source.squeak.org/trunk/Compiler-eem.379.diff
> If there is a reference to this version in the update map, then update should work (and it did for me).

In the end, here too...

> 
> 2018-03-23 16:53 GMT+01:00 Eliot Miranda <eliot.miranda at gmail.com>:
> Hi Tobias,
> 
> On Mar 23, 2018, at 7:56 AM, Tobias Pape <Das.Linux at gmx.de> wrote:
> 
>> Heho
>> 
>>> On 23.03.2018, at 15:26, Eliot Miranda <eliot.miranda at gmail.com> wrote:
>>> 
>>> Hi Tobias,
>>> 
>>>> On Mar 23, 2018, at 7:03 AM, Tobias Pape <Das.Linux at gmx.de> wrote:
>>>> 
>>>> HI Eliot,
>>>> 
>>>>> On 23.03.2018, at 14:55, Eliot Miranda <eliot.miranda at gmail.com> wrote:
>>>>> 
>>>>> Hi Tobias,
>>>>> 
>>>>> as the commit comments says, you must first load Compiler-eem.379.  The update steam contains an update for it.  And its commit comment explains the issue:
>>>> 
>>>> Yes, I have 379 loaded,
>>>> no it does not help.
>>>> 
>>>> Reason: The class is reshaped before its methods are recompiled, hence 'addedSelectorAndMethodClassLiterals' is gone and nilled and throws a MustBeBoolean upon next use, wich is any kind of compilation, so the recompilation fails.
>>> 
>>> That's strange.  I tested this multiple times (in 64-bit images) before I committed (as two file-ins), and after I committed (as an update) and saw no problems.  379 adds versions that update both variables, so addedExtraLiterals is in Undeclared and both  it and the addedSelectorAndMethodClassLiterals isn't var get assigned on encoder initialization.  Then, when Encoder gets reshaped, addedSelectorAndMethodClassLiterals is out in Undeclared, addedExtraLiterals becomes the inst var, and both get initialized.  Finally, the references to addedSelectorAndMethodClassLiterals are replaced by references to  addedExtraLiterals, and the putch is complete.
>>> 
>>> So you should find addedExtraLiterals in Undeclared, with the value true before loading 380.  
>> 
>> Yes: 
>> ===
>> Undeclared "a WeakIdentityDictionary(#addedExtraLiterals->false )"
>> (Encoder >> #allLiterals) "first bytecode is pushRcvr: 14, aka addedSelectorAndMethodClassLiterals)
>> ===
>> and "	addedSelectorAndMethodClassLiterals := addedExtraLiterals := false." is present.
>> 
>> The patch also looks good (especially , change allLiterals before the class reshape)
>> <Bildschirmfoto 2018-03-23 um 15.44.47.PNG>
>> 
>> 
>> Yet, MCPackageLoader>>basicLoad changes that and pulls everything except the method defs to the front
>> ""Pass 1: Load everything but the methods,  which are collected in methodAdditions.""
>> 
>> 
>>> Then, after the reshape, but before loading the other defs, you should find addedSelectorAndMethodClassLiterals with the value true.
>> 
>> And, yes, After the step 1, undeclareds are as you expected:
>> 
>> Undeclared  "a WeakIdentityDictionary(#addedExtraLiterals->false #addedSelectorAndMethodClassLiterals->true )"
>> 
>> And Encoder>>allLiterals has the expected literal for the undeclared var.
>> 
>> =-=-=-==
>> And now I was able to step though everything just fine (after it DIDN'T work four times this morning), I feel a bit like a moron -.-'
>> 
>> Sorry Eliot, everything seems fine *sigh*
>> 
>> 
>> CARRY ON FOLKS NOTHNG TO SEE KEEP WALKING … 
> 
> Hang on !! :-). Stepping through isn't a solution.  If you use, for example, Update Squeak from the Squeak menu does it work or not?  I'd still like to understand d why it broke for you...
> 
>> 
>> best regards
>> 	-tobias
>> 
>> 
>>> 
>>> The first thing to determine before we throw away 380 and try again is if anyone else is being bitten by this.  If it works for everyone else (and as I said it has worked for me) then there's something broken in your image.  If it's broken for everyone then I guess the package prologue may need to add addedSelectorAndMethodClassLiterals to Undeclared with value false before the reshape and redefinition. You could test if this would work by manually adding addedSelectorAndMethodClassLiterals to Undeclared with value false before loading 380.
>>> 
>>> So has anyone else had their system broken by Compiler-eem.380?
>>> 
>>> (And I'm really sorry Tobias; I thought I had this one right; and Bert, I think this is an example where update naps are absolutely required so it might be worth adding to the update doc; I'll take a look).
>>> 
>>>> 
>>>> Best regards
>>>>   -Tobias
>>>> 
>>>> 
>>>> 
>>>>> 
>>>>> Name: Compiler-eem.379
>>>>> Author: eem
>>>>> Time: 20 March 2018, 3:27:27.12646 pm
>>>>> UUID: b3856f24-9d98-478a-936f-c6d24d667be4
>>>>> Ancestors: Compiler-eem.378
>>>>> 
>>>>> Add initialization of the Undeclared variable addedExtraLiterals which is soon to be a replacement for Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support.  By adding the initialization of the Undeclared variable the compiler is not broken as the instance variable is renamed and Encoder's methods are recompiled.
>>>>> 
>>>>> _,,,^..^,,,_ (phone)
>>>>> 
>>>>>> On Mar 23, 2018, at 1:20 AM, Tobias Pape <Das.Linux at gmx.de> wrote:
>>>>>> 
>>>>>> Hi Eliot,
>>>>>> 
>>>>>> 
>>>>>> loading .380  produces an error for me in #allLiterals.
>>>>>> 
>>>>>> I think this is because of the Encoder being used for changing the encoder?
>>>>>> we need a few thing here probably:
>>>>>> 1. remove .380
>>>>>> 2. a commit that _Adds_ the new inst var (iirc this is .379 already)
>>>>>> 3. an mcm for that (iirc this is .431.mcm)
>>>>>> 4. remove config .432.mcm
>>>>>> 5. a commit that _uses_ the new inst var
>>>>>> 6. an mcm for that (a _new_ .432.mcm)
>>>>>> 7. a commit that _Removes_ the old inst var.
>>>>>> (8. maybe an mcm for that..)
>>>>>> 
>>>>>> Best regards
>>>>>>   -Tobias
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> <Bildschirmfoto 2018-03-23 um 09.02.37.PNG>
>>>>>>> On 20.03.2018, at 23:30, commits at source.squeak.org wrote:
>>>>>>> 
>>>>>>> Eliot Miranda uploaded a new version of Compiler to project The Trunk:
>>>>>>> http://source.squeak.org/trunk/Compiler-eem.380.mcz
>>>>>>> 
>>>>>>> ==================== Summary ====================
>>>>>>> 
>>>>>>> Name: Compiler-eem.380
>>>>>>> Author: eem
>>>>>>> Time: 20 March 2018, 3:30:10.256928 pm
>>>>>>> UUID: 3133d60f-54b2-410e-92ae-ef5cc782ab9c
>>>>>>> Ancestors: Compiler-eem.379
>>>>>>> 
>>>>>>> Rename Encoder's addedSelectorAndMethodClassLiterals, a name which is now misleading given the new full block support, to addedExtraLiterals.  Requires Compiler-eem.379.
>>>>>>> 
>>>>>>> =============== Diff against Compiler-eem.379 ===============
>>>>>>> 
>>>>>>> Item was changed:
>>>>>>> ----- Method: BytecodeEncoder>>allLiteralsForBlockMethod (in category 'results') -----
>>>>>>> allLiteralsForBlockMethod
>>>>>>> +    addedExtraLiterals ifFalse:
>>>>>>> +        [addedExtraLiterals := true.
>>>>>>> -    "N.B. addedSelectorAndMethodClassLiterals is a misnomer.  It should be something like
>>>>>>> -     addedImplicitLiterals or addedExtraLiterals."
>>>>>>> -    addedSelectorAndMethodClassLiterals ifFalse:
>>>>>>> -        [addedSelectorAndMethodClassLiterals := true.
>>>>>>>       "Put the optimized selectors in literals so as to browse senders more easily"
>>>>>>>       optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
>>>>>>>       optimizedSelectors isEmpty ifFalse: [
>>>>>>>           "Use one entry per literal if enough room, else make anArray"
>>>>>>>           literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
>>>>>>>               ifTrue: [self litIndex: optimizedSelectors asArray]
>>>>>>>               ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
>>>>>>>       "Add a slot for outerCode"
>>>>>>>       self litIndex: nil].
>>>>>>>   ^literalStream contents!
>>>>>>> 
>>>>>>> Item was changed:
>>>>>>> ----- Method: BytecodeEncoder>>resetForFullBlockGeneration (in category 'code generation') -----
>>>>>>> resetForFullBlockGeneration
>>>>>>>   literalStream := WriteStream on: (Array new: 8).
>>>>>>> +    addedExtraLiterals := false.
>>>>>>> -    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
>>>>>>>   optimizedSelectors := Set new!
>>>>>>> 
>>>>>>> Item was changed:
>>>>>>> ----- Method: BytecodeEncoder>>resetLiteralStreamForFullBlock (in category 'code generation') -----
>>>>>>> resetLiteralStreamForFullBlock
>>>>>>>   literalStream := WriteStream on: (Array new: 32).
>>>>>>> +    addedExtraLiterals := false.
>>>>>>> -    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
>>>>>>>   optimizedSelectors := Set new!
>>>>>>> 
>>>>>>> Item was changed:
>>>>>>> ParseNode subclass: #Encoder
>>>>>>> +    instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedExtraLiterals optimizedSelectors cue'
>>>>>>> -    instanceVariableNames: 'scopeTable nTemps supered requestor class selector literalStream selectorSet litIndSet litSet sourceRanges globalSourceRanges addedSelectorAndMethodClassLiterals optimizedSelectors cue'
>>>>>>>   classVariableNames: ''
>>>>>>>   poolDictionaries: ''
>>>>>>>   category: 'Compiler-Kernel'!
>>>>>>> 
>>>>>>> !Encoder commentStamp: 'cwp 12/26/2012 23:29' prior: 0!
>>>>>>> I encode names and literals into tree nodes with byte codes for the compiler. Byte codes for literals are not assigned until the tree-sizing pass of the compiler, because only then is it known which literals are actually needed. I also keep track of sourceCode ranges during parsing and code generation so I can provide an inverse map for the debugger.!
>>>>>>> 
>>>>>>> Item was changed:
>>>>>>> ----- Method: Encoder>>allLiterals (in category 'results') -----
>>>>>>> allLiterals
>>>>>>> +    addedExtraLiterals ifFalse:
>>>>>>> +        [addedExtraLiterals := true.
>>>>>>> -    addedSelectorAndMethodClassLiterals ifFalse:
>>>>>>> -        [addedSelectorAndMethodClassLiterals := true.
>>>>>>>       "Put the optimized selectors in literals so as to browse senders more easily"
>>>>>>>       optimizedSelectors := optimizedSelectors reject: [:e| literalStream originalContents hasLiteral: e].
>>>>>>>       optimizedSelectors isEmpty ifFalse: [
>>>>>>>           "Use one entry per literal if enough room, else make anArray"
>>>>>>>           literalStream position + optimizedSelectors size + 2 >= self maxNumLiterals
>>>>>>>               ifTrue: [self litIndex: optimizedSelectors asArray]
>>>>>>>               ifFalse: [optimizedSelectors do: [:e | self litIndex: e]]].
>>>>>>>       "Add a slot for selector or MethodProperties"
>>>>>>>       self litIndex: nil.
>>>>>>>       self litIndex: self associationForClass].
>>>>>>>   ^literalStream contents!
>>>>>>> 
>>>>>>> Item was changed:
>>>>>>> ----- Method: Encoder>>initScopeAndLiteralTables (in category 'initialize-release') -----
>>>>>>> initScopeAndLiteralTables
>>>>>>> 
>>>>>>>   scopeTable := StdVariables copy.
>>>>>>>   litSet := StdLiterals copy.
>>>>>>>   "comments can be left hanging on nodes from previous compilations.
>>>>>>>    probably better than this hack fix is to create the nodes afresh on each compilation."
>>>>>>>   scopeTable do:
>>>>>>>       [:varNode| varNode comment: nil].
>>>>>>>   litSet do:
>>>>>>>       [:varNode| varNode comment: nil].
>>>>>>>   selectorSet := StdSelectors copy.
>>>>>>>   litIndSet := Dictionary new: 16.
>>>>>>>   literalStream := WriteStream on: (Array new: 32).
>>>>>>> +    addedExtraLiterals := false.
>>>>>>> -    addedSelectorAndMethodClassLiterals := addedExtraLiterals := false.
>>>>>>>   optimizedSelectors := Set new!
>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>>> 
>>> 
>> 
>> 
> 
> 
> 
> 
> 



More information about the Squeak-dev mailing list