[prev in list] [next in list] [prev in thread] [next in thread] 

List:       squeak-dev
Subject:    Re: [squeak-dev] The Trunk: Compiler-eem.380.mcz
From:       Tobias Pape <Das.Linux () gmx ! de>
Date:       2018-03-31 12:06:55
Message-ID: C128136D-E461-490B-BF5C-290DA96B912C () gmx ! de
[Download RAW message or body]

Hi Nicolas,

> On 23.03.2018, at 18:23, Nicolas Cellier <nicolas.cellier.aka.nice@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@gmail.com>:
> Hi Tobias,
> 
> On Mar 23, 2018, at 7:56 AM, Tobias Pape <Das.Linux@gmx.de> wrote:
> 
> > Heho
> > 
> > > On 23.03.2018, at 15:26, Eliot Miranda <eliot.miranda@gmail.com> wrote:
> > > 
> > > Hi Tobias,
> > > 
> > > > On Mar 23, 2018, at 7:03 AM, Tobias Pape <Das.Linux@gmx.de> wrote:
> > > > 
> > > > HI Eliot,
> > > > 
> > > > > On 23.03.2018, at 14:55, Eliot Miranda <eliot.miranda@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@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@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!
> > > > > > > 
> > > > > > > 
> > > > > > 
> > > > > > 
> > > > > 
> > > > 
> > > > 
> > > 
> > 
> > 
> 
> 
> 
> 
> 


[prev in list] [next in list] [prev in thread] [next in thread]