[Pharo-dev] [Pharo 8.0] Build #273: 3338-Slots-isSpecial-cleanup-part-2

[ci-pharo-ci-jenkins2]

There is a new Pharo build available!
  
The status of the build #273 was: SUCCESS.

The Pull Request #3339 was integrated: "3338-Slots-isSpecial-cleanup-part-2"
Pull request url: https://github.com/pharo-project/pharo/pull/3339

Issue Url: https://github.com/pharo-project/pharo/issues/3338
Build Url: 
https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/Pharo8.0/273/

Re: [Pharo-dev] Some more effort to make Slang and VMMaker work on Pharo [for review]

[David T. Lewis]

Guille,

This is excellent work. Thank you also for your careful explanations.

Dave

On Wed, May 15, 2019 at 10:33:38AM +0200, Guillermo Polito wrote:
> Hi Eliot,
> 
> > El 15 may 2019, a las 3:20, Eliot Miranda  
> > escribi??:
> > 
> > Hi Guille,
> > 
> > On Tue, May 14, 2019 at 7:30 AM Guillermo Polito  > > wrote:
> > 
> >  - I had to review the AST-to-AST transformation, checking the output, 
> > comparing it to what squeak does and so on...
> > 
> > Can you show a little of th differences in output between Squeak compiler 
> > derived sources and RB parser derived sources?  I'm not going to demand 
> > changes or fixes or that they be exactly the same.  But I am curious to 
> > know what kind of changes there are.
> 
> From what I have now, I can see three main kind of differences (I actually 
> compared with source code I generated myself from squeak and the existing 
> sources in the git repo).
> I did not work on them because the VM seemed to work, so I left them for 
> later:
> 
> 1) some comments are placed in different places in the output source code. 
> This is entirely cosmetic, but it brings unnecessary noise to C source diffs.
> 
> 2) labels in the gnu???ed code are numbered differently. I did not spot any 
> ???bug??? in that yet, I did not dig enough into the gnuification to see why 
> the difference, and I???m not entirely sure that this difference is caused by 
> that either :)???
> Anyhow, this is mostly annoying because it also brings lots of noise to diffs.
> 
> 3) some for loops inline the condition while they probably shouldn't. For 
> example:
> 
> 0 to: (self numSlotsOf: array1) - 1 do: [ ??? ]
> 
> gets in my output translated to 
> 
> for (i = 0; i < (numSlotsOf(array1)); i += 1) { ...
> 
> while in the version generated from squeak
> 
> for (i = 0, iLimiT = ((numSlotsOf(array1)) - 1); i <= iLimiT; i += 1) { ???
> 
> This seems the most ???dangerous" difference I have so far specially if the 
> limit condition has some side effect.
> We will probably agree in that this third point should be fixed ^^.
> 
> >  
> >  - I???ve written several unit tests to ensure that future migrations are 
> > easier to do
> >  - I???ve introduced several compatibility classes/methods related to 
> > PackageInfo, Time and so on???
> > 
> > This is great1  Thank you.  Perhaps we can move some of the source 
> > generation tests into the core.  It would be good for there to be tests 
> > that apply to the Squeak compiler side too.  I never took the time to run 
> > tests (I look at the generated source every time I make a change Slang), 
> > and writing a really good set of tests always seemed like a lot of work.  
> > But I might be tempted to add to the ones you've written. 
> 
> Maybe in the next weeks I can try to massage the tests to make them less 
> dependent on the source (wether squeak's or pharo???s) AST.
> The thing is that Pharo???s AST is closer to the source code, while 
> Squeak???s has already some pre-cooked code transformations (like #ifNil: & 
> co), and my tests assume a specific AST structure to traverse the tree and 
> compare???
> 
> Cheers,
> Guille

Re: [Pharo-dev] Some more effort to make Slang and VMMaker work on Pharo [for review]

[Guillermo Polito]

Hi Subbu,

I do not believe this deserves a paper :P. At most a blogpost. What I can do 
quickly here is to draft the “methodology” I followed by instinct, but that can 
be reproduced by anyone.

1) generate sources from pharo
2) try to compile (and run)
3) in case of failure, make a diff with the latest version of the same file in 
the git repository of opensmalltalk vm
4) spot an interesting difference (most common cases were related to loops and 
conditions, missing or extra inlinings) => get ONE method with the problem
  4.1) Check 1: verify that the generating that method from squeak does **not** 
have the problem
  4.2) Check 2: verify that the generating that method from pharo **does have** 
the problem
 For these checks I used some of the already given VMMaker scripts
5) generate a small artificial but representative case for testing. For example:

methodWithIfNil
self something
ifNil: [ 1 ]
ifNotNil: [ 2 ]

Then I have two level of tests: AST-to-AST transformation, and generation tests.

testIfNilIfNotNilBecomesIfTrueIfFalse

| translation thisAST |
thisAST := (self class >> #methodWithIfNil) ast.
translation := thisAST asTranslationMethodOfClass: TMethod.

self assert: translation statements first selector equals: 
#ifTrue:ifFalse:

testSimpleIfNilAssignment

| translation thisAST codeGenerator result |
thisAST := (self class >> #methodWithIfNil) ast.
translation := thisAST asTranslationMethodOfClass: TMethod.
codeGenerator := CCodeGeneratorGlobalStructure new.
codeGenerator generateDeadCode: false.
codeGenerator addMethod: translation.
codeGenerator doInlining: true.

result := String streamContents: [ :stream |
translation parseTree statements first emitCCodeOn: stream 
level: 0 generator: codeGenerator.
].

self assert: result equals: 'if ((something()) == null) {
}
else {
}’

6) Fix it

* Most of the “bugs” were related to making the different #asTranslatorNodeIn: 
methods in the RB AST compliant to what it was doing in squeak.
* Sometimes I ended up doing parallel stepping in Pharo and Squeak to spot 
where the translation diverged
* Something important that I’ve learnt in the process is that Squeak’s AST 
contain already some transformations/expansions like

what you see/write:  self x ifNil: [  … ]
what you actually have:   self x == nil ifTrue: [ … ]

BUT, the most confusing point is that even if the AST internally is structured 
as in the version in the right, the printString of the AST in the inspector 
showed the left version.


Nothing new under the sun ^^.

Guille

> El 15 may 2019, a las 9:34, ducasse  escribió:
> 
> Hi Subbu
> 
>>> To do this work
>>> - I had to review the AST-to-AST transformation, checking the output,
>>> comparing it to what squeak does and so on...
>> 
>> Guillermo,
>> 
>> This is a superb effort! Thanks. Are you planning to write a paper on this 
>> part of your work? A blog? It can open new vistas for research students in 
>> Pharo.
> 
> We will see if we can produce a tutorial in how to extend the VM. Igor did it 
> in 2011 and we will redo it. Life is a cycle.
> 
> Then we would love to see if we can have a VMMaker repackaged (it should not 
> be difficult) with less code so that we can do lectures on the Pharo VM 
> and exercises for students with it. It is not really pedagogical to tell them 
> to jump over many not used classes. 
> And more important since we want to attract smart guys they will not take us 
> seriously if we present a system that is monolithic
> - we got several times students asking why VMMaker could not be cut into 
> different packages and well they are right. 
> 
> What I was thinking is that we should have soon a travis to build it 
> automatically with Pharo. 
> If you are interested to help please join the effort. 
> What guille showed us is that it was not difficult to do it in fact. 
> 
> Stef
> 
>> 
>> Regards .. Subbu
>> 
> 
> 

Re: [Pharo-dev] Some more effort to make Slang and VMMaker work on Pharo [for review]

[Guillermo Polito]

Hi Eliot,

> El 15 may 2019, a las 3:20, Eliot Miranda  escribió:
> 
> Hi Guille,
> 
> On Tue, May 14, 2019 at 7:30 AM Guillermo Polito  > wrote:
> 
>  - I had to review the AST-to-AST transformation, checking the output, 
> comparing it to what squeak does and so on...
> 
> Can you show a little of th differences in output between Squeak compiler 
> derived sources and RB parser derived sources?  I'm not going to demand 
> changes or fixes or that they be exactly the same.  But I am curious to know 
> what kind of changes there are.

From what I have now, I can see three main kind of differences (I actually 
compared with source code I generated myself from squeak and the existing 
sources in the git repo).
I did not work on them because the VM seemed to work, so I left them for later:

1) some comments are placed in different places in the output source code. This 
is entirely cosmetic, but it brings unnecessary noise to C source diffs.

2) labels in the gnu’ed code are numbered differently. I did not spot any “bug” 
in that yet, I did not dig enough into the gnuification to see why the 
difference, and I’m not entirely sure that this difference is caused by that 
either :)…
Anyhow, this is mostly annoying because it also brings lots of noise to diffs.

3) some for loops inline the condition while they probably shouldn't. For 
example:

0 to: (self numSlotsOf: array1) - 1 do: [ … ]

gets in my output translated to 

for (i = 0; i < (numSlotsOf(array1)); i += 1) { ...

while in the version generated from squeak

for (i = 0, iLimiT = ((numSlotsOf(array1)) - 1); i <= iLimiT; i += 1) { …

This seems the most “dangerous" difference I have so far specially if the limit 
condition has some side effect.
We will probably agree in that this third point should be fixed ^^.

>  
>  - I’ve written several unit tests to ensure that future migrations are 
> easier to do
>  - I’ve introduced several compatibility classes/methods related to 
> PackageInfo, Time and so on…
> 
> This is great1  Thank you.  Perhaps we can move some of the source generation 
> tests into the core.  It would be good for there to be tests that apply to 
> the Squeak compiler side too.  I never took the time to run tests (I look at 
> the generated source every time I make a change Slang), and writing a really 
> good set of tests always seemed like a lot of work.  But I might be tempted 
> to add to the ones you've written. 

Maybe in the next weeks I can try to massage the tests to make them less 
dependent on the source (wether squeak's or pharo’s) AST.
The thing is that Pharo’s AST is closer to the source code, while Squeak’s has 
already some pre-cooked code transformations (like #ifNil: & co), and my tests 
assume a specific AST structure to traverse the tree and compare…

Cheers,
Guille

Re: [Pharo-dev] Some more effort to make Slang and VMMaker work on Pharo [for review]

[ducasse]

Hi Subbu

>> To do this work
>> - I had to review the AST-to-AST transformation, checking the output,
>> comparing it to what squeak does and so on...
> 
> Guillermo,
> 
> This is a superb effort! Thanks. Are you planning to write a paper on this 
> part of your work? A blog? It can open new vistas for research students in 
> Pharo.

We will see if we can produce a tutorial in how to extend the VM. Igor did it 
in 2011 and we will redo it. Life is a cycle.

Then we would love to see if we can have a VMMaker repackaged (it should not be 
difficult) with less code so that we can do lectures on the Pharo VM 
and exercises for students with it. It is not really pedagogical to tell them 
to jump over many not used classes. 
And more important since we want to attract smart guys they will not take us 
seriously if we present a system that is monolithic
- we got several times students asking why VMMaker could not be cut into 
different packages and well they are right. 

What I was thinking is that we should have soon a travis to build it 
automatically with Pharo. 
If you are interested to help please join the effort. 
What guille showed us is that it was not difficult to do it in fact. 

Stef

> 
> Regards .. Subbu
>