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 <eliot.mira...@gmail.com> > > escribi??: > > > > Hi Guille, > > > > On Tue, May 14, 2019 at 7:30 AM Guillermo Polito <guillermopol...@gmail.com > > <mailto:guillermopol...@gmail.com>> 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