Levente Uzonyi uploaded a new version of Compiler to project The Inbox:
http://source.squeak.org/inbox/Compiler-ul.401.mcz ==================== Summary ==================== Name: Compiler-ul.401 Author: ul Time: 16 March 2019, 11:56:29.899053 pm UUID: 395adc27-f48e-484c-9097-5197ea3a0155 Ancestors: Compiler-eem.400 - fixed typo =============== Diff against Compiler-eem.400 =============== Item was changed: ----- Method: ParseNodeWithPrecedingStatementEnumerator>>ofBlock: (in category 'initialize-release') ----- ofBlock: aBlock "N.B. This enumerator visits a node before any of the node's children. Hence, when enumewrating statements in a block, we can ensure that the second argument to the block, the preceeding statement, is non-nil only for top-level statements in the block by nilling out preceedingStatement once the block is evaluated. Perhaps stronger would be to capture its value in a temporary and nil it before evaluating, but this is good enough." theBlock := [:node| aBlock value: node value: precedingStatement. + precedingStatement := nil]! - preceedingStatement := nil]! |
Hi Eliot,
I pushed this to the Inbox, because I noticed there was an undeclared variable used by this method. Given how the code was intended to work, this typo meant that the code didn't work. I also noticed that the Compiler package has failing tests. I didn't investigate if those failures are related to this method in any way, but it seemed safer not to push this to the Trunk yet. I'm sure this fix is an improvement, but there may be further changes needed. Levente On Sat, 16 Mar 2019, [hidden email] wrote: > Levente Uzonyi uploaded a new version of Compiler to project The Inbox: > http://source.squeak.org/inbox/Compiler-ul.401.mcz > > ==================== Summary ==================== > > Name: Compiler-ul.401 > Author: ul > Time: 16 March 2019, 11:56:29.899053 pm > UUID: 395adc27-f48e-484c-9097-5197ea3a0155 > Ancestors: Compiler-eem.400 > > - fixed typo > > =============== Diff against Compiler-eem.400 =============== > > Item was changed: > ----- Method: ParseNodeWithPrecedingStatementEnumerator>>ofBlock: (in category 'initialize-release') ----- > ofBlock: aBlock > "N.B. This enumerator visits a node before any of the node's children. > Hence, when enumewrating statements in a block, we can ensure that > the second argument to the block, the preceeding statement, is non-nil > only for top-level statements in the block by nilling out preceedingStatement > once the block is evaluated. Perhaps stronger would be to capture its value > in a temporary and nil it before evaluating, but this is good enough." > theBlock := [:node| > aBlock value: node value: precedingStatement. > + precedingStatement := nil]! > - preceedingStatement := nil]! |
>> =============== Diff against Compiler-eem.400 ===============
>> >> Item was changed: >> ----- Method: ParseNodeWithPrecedingStatementEnumerator>>ofBlock: (in >> category 'initialize-release') ----- >> ofBlock: aBlock >> "N.B. This enumerator visits a node before any of the node's >> children. >> Hence, when enumewrating statements in a block, we can ensure that typo s/enumewrat/enumerate/ >> the second argument to the block, the preceeding statement, is >> non-nil >> only for top-level statements in the block by nilling out >> preceedingStatement typo in 2 places. s/preceeding/preceding/ There are multiple places in Compiler-Kernel and Compiler-ParseNodes where preceding is written as preceeding, largely in comments. It would be better to fix these too while merging this patch into trunk so that it is not misread for 'proceeding' which reverses the meaning ;-). Regards .. Subbu |
Free forum by Nabble | Edit this page |