Smalltalk › Squeak › Squeak - Dev

Compiler evaluate: '' | 'inner' argument in parser

‹ Previous Topic Next Topic ›
 
classic Classic list List threaded Threaded
♦
♦
 4 messages Options
Christoph Thiede
Reply | Threaded
Open this post in threaded view
♦
♦
|

Compiler evaluate: '' | 'inner' argument in parser

Christoph Thiede

Hi all,


I just evaluated the following line and was surprised by the result:


Compiler evaluate: '' for: 42.


The answer was nil, whereas I rather would have expected the answer to life the universe and everything 42, as it is the case for full methods compiled on an object. What do you think about this, which should be the right output?


I was even more surprised after taking a look into the relevant Parser implementation, which is #statements:innerBlock:blockNode:. Actually, the 'inner' argument's sole purpose is to use 'nil' instead of 'self' as a return value if there are zero statements. If we would not like to have this extra rule for doIt snippets, we would only have to change #method:context:[encoder:] and pass 'false' instead of 'doit' to #statements:innerBlock:.


I'm probably missing some logical consequences of the syntactic concept of blocks, methods, and doits, so I would like to hear your thoughts on this.


Best,

Christoph



Carpe Squeak!
Eliot Miranda-2
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: Compiler evaluate: '' | 'inner' argument in parser

Eliot Miranda-2
Hi Christoph,

On Sep 14, 2020, at 5:17 AM, Thiede, Christoph <[hidden email]> wrote:

Hi all,

I just evaluated the following line and was surprised by the result:

Compiler evaluate: '' for: 42.

The answer was nil, whereas I rather would have expected the answer to life the universe and everything 42, as it is the case for full methods compiled on an object. What do you think about this, which should be the right output?

I was even more surprised after taking a look into the relevant Parser implementation, which is #statements:innerBlock:blockNode:. Actually, the 'inner' argument's sole purpose is to use 'nil' instead of 'self' as a return value if there are zero statements. If we would not like to have this extra rule for doIt snippets, we would only have to change #method:context:[encoder:] and pass 'false' instead of 'doit' to #statements:innerBlock:.

I'm probably missing some logical consequences of the syntactic concept of blocks, methods, and doits, so I would like to hear your thoughts on this.


Smells like my bug.  I agree with you that the null statement should evaluate to self at method level.  IIRC, the ANSI standard says that the null statement evaluated to the last block argument did blocks with arguments (to legalize a quirk in Peter Deutsch’s closure compiler for ObjectWorks 2.5 and subsequent).  And clearly the null statement in a block evaluates to nil.  I hunk that the null statement should evaluate to self at method level and nil at block level, irrespective of the number of block arguments.

To be clear I would have these tests

self assert: (Compiler evaluate: '' for: 42) equals: 42.
self assert: ([:arg|] value: 42) equals: nil.
self assert: [] value equals: nil.

Best,

Christoph
Eliot Miranda-2
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: Compiler evaluate: '' | 'inner' argument in parser

Eliot Miranda-2
In reply to this post by Christoph Thiede
Hi Christoph,

  ignore my first response.  I hadn't looked at the code yet.  It hasn't changed my conclusions, but it alters the proposed solution.


On Mon, Sep 14, 2020 at 5:17 AM Thiede, Christoph <[hidden email]> wrote:

Hi all,

I just evaluated the following line and was surprised by the result:

Compiler evaluate: '' for: 42.

The answer was nil, whereas I rather would have expected the answer to life the universe and everything 42, as it is the case for full methods compiled on an object. What do you think about this, which should be the right output?


I agree with you that the null statement should evaluate to self at method level.  IIRC, the ANSI standard says that the null statement evaluated to the last block argument did blocks with arguments (to legalize a quirk in Peter Deutsch’s closure compiler for ObjectWorks 2.5 and subsequent).  And clearly the null statement in a block evaluates to nil.  I hunk that the null statement should evaluate to self at method level and nil at block level, irrespective of the number of block arguments.

To be clear I would have these tests

self assert: (Compiler evaluate: '' for: 42) equals: 42.
self assert: ([:arg|] value: 42) equals: nil.
self assert: [] value equals: nil.

Looking at the code, when I added closures I took the existing compiler as a given and hence perpetuated the bug that (Compiler evaluate: '' for: 42) isNil.  Looking at the code I don't like the inner: argument, and for the longest time I have felt like the statements:innerBlock: method was unnecessary. So one fix in Parser>>#method:context: is to replace

self statements: #() innerBlock: doit.

with
self statements: #() innerBlock: false blockNode: BlockNode new.

Another improvement would be to simply substitute the variable to be used for empty statements, so we would use

Parser>>#method:context:
...
self statements: #() defaultVariable: 'self' blockNode: BlockNode new.
...

Parser>>#statements: argNodes defaultVariable: selfOrNil blockNode: theBlockNode

...
ifFalse: 
[self addComment.
 stmts size = 0 ifTrue: 
[stmts addLast:  (encoder encodeVariable: selfOrNil)]]].
...

Parser>>#blockExpression
...
self statements: variableNodes defaultVariable: 'nil' blockNode: blockNode.
...


but maybe the following:

Parser>>#method:context:
...
doit ifTrue: [blk returnLast]
ifFalse: [blk returnSelfIfNoOther: encoder].
...

indicates that either
stmts size = 0 ifTrue: 
[stmts addLast:  (encoder encodeVariable: selfOrNil)]
is wrong and the defaulting should happen in the sender (in Parser>>#method:context: and Parser>>#primaryExpression where it sends blockExpression), or that we should pass in a selector which is performed in the method to determine what to do.  So the method would then be e.g.

    Parser>>#statements:blockNode:ifEmpty:

and the senders would look like, e.g.

Parser>>#method:context:
...
   self statements: #() blockNode: BlockNode new ifEmpty: (doit ifTrue: [#returnLast:] ifFalse: [#returnSelfIfNoOther:).
   blk := parseNode.
   hereType == #doIt ifFalse: [^self expected: 'Nothing more']. 
...

Parser>>#blockExpression
...
   self statements: variableNodes blockNode: BlockNode new ifEmpty: #returnNilIfEmpty:.
... 

I'm interested in your reading.  I think you should implement whatever makes most sense to you. I'm happy to rebiew.

I was even more surprised after taking a look into the relevant Parser implementation, which is #statements:innerBlock:blockNode:. Actually, the 'inner' argument's sole purpose is to use 'nil' instead of 'self' as a return value if there are zero statements. If we would not like to have this extra rule for doIt snippets, we would only have to change #method:context:[encoder:] and pass 'false' instead of 'doit' to #statements:innerBlock:.

I'm probably missing some logical consequences of the syntactic concept of blocks, methods, and doits, so I would like to hear your thoughts on this.


This is not so much philosophical as rushed.  When I added closures I wanted not to break the compiler and DTSTTCPW because I had a VM to write.  I wanted to clean things up but I risked making too many mistakes so I erred on the side of minimally extending the compiler rather than making it better.

Best,

Christoph


_,,,^..^,,,_
best, Eliot
Christoph Thiede
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: Compiler evaluate: '' | 'inner' argument in parser

Christoph Thiede

Hi Eliot,


thanks for your reaction to change this behavior via Compiler-eem.442.


Best,

Christoph

Von: Squeak-dev <[hidden email]> im Auftrag von Eliot Miranda <[hidden email]>
Gesendet: Montag, 14. September 2020 15:29:00
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] Compiler evaluate: '' | 'inner' argument in parser
 
Hi Christoph,

  ignore my first response.  I hadn't looked at the code yet.  It hasn't changed my conclusions, but it alters the proposed solution.


On Mon, Sep 14, 2020 at 5:17 AM Thiede, Christoph <[hidden email]> wrote:

Hi all,

I just evaluated the following line and was surprised by the result:

Compiler evaluate: '' for: 42.

The answer was nil, whereas I rather would have expected the answer to life the universe and everything 42, as it is the case for full methods compiled on an object. What do you think about this, which should be the right output?


I agree with you that the null statement should evaluate to self at method level.  IIRC, the ANSI standard says that the null statement evaluated to the last block argument did blocks with arguments (to legalize a quirk in Peter Deutsch’s closure compiler for ObjectWorks 2.5 and subsequent).  And clearly the null statement in a block evaluates to nil.  I hunk that the null statement should evaluate to self at method level and nil at block level, irrespective of the number of block arguments.

To be clear I would have these tests

self assert: (Compiler evaluate: '' for: 42) equals: 42.
self assert: ([:arg|] value: 42) equals: nil.
self assert: [] value equals: nil.

Looking at the code, when I added closures I took the existing compiler as a given and hence perpetuated the bug that (Compiler evaluate: '' for: 42) isNil.  Looking at the code I don't like the inner: argument, and for the longest time I have felt like the statements:innerBlock: method was unnecessary. So one fix in Parser>>#method:context: is to replace

self statements: #() innerBlock: doit.

with
self statements: #() innerBlock: false blockNode: BlockNode new.

Another improvement would be to simply substitute the variable to be used for empty statements, so we would use

Parser>>#method:context:
...
self statements: #() defaultVariable: 'self' blockNode: BlockNode new.
...

Parser>>#statements: argNodes defaultVariable: selfOrNil blockNode: theBlockNode

...
ifFalse: 
[self addComment.
 stmts size = 0 ifTrue: 
[stmts addLast:  (encoder encodeVariable: selfOrNil)]]].
...

Parser>>#blockExpression
...
self statements: variableNodes defaultVariable: 'nil' blockNode: blockNode.
...


but maybe the following:

Parser>>#method:context:
...
doit ifTrue: [blk returnLast]
ifFalse: [blk returnSelfIfNoOther: encoder].
...

indicates that either
stmts size = 0 ifTrue: 
[stmts addLast:  (encoder encodeVariable: selfOrNil)]
is wrong and the defaulting should happen in the sender (in Parser>>#method:context: and Parser>>#primaryExpression where it sends blockExpression), or that we should pass in a selector which is performed in the method to determine what to do.  So the method would then be e.g.

    Parser>>#statements:blockNode:ifEmpty:

and the senders would look like, e.g.

Parser>>#method:context:
...
   self statements: #() blockNode: BlockNode new ifEmpty: (doit ifTrue: [#returnLast:] ifFalse: [#returnSelfIfNoOther:).
   blk := parseNode.
   hereType == #doIt ifFalse: [^self expected: 'Nothing more']. 
...

Parser>>#blockExpression
...
   self statements: variableNodes blockNode: BlockNode new ifEmpty: #returnNilIfEmpty:.
... 

I'm interested in your reading.  I think you should implement whatever makes most sense to you. I'm happy to rebiew.

I was even more surprised after taking a look into the relevant Parser implementation, which is #statements:innerBlock:blockNode:. Actually, the 'inner' argument's sole purpose is to use 'nil' instead of 'self' as a return value if there are zero statements. If we would not like to have this extra rule for doIt snippets, we would only have to change #method:context:[encoder:] and pass 'false' instead of 'doit' to #statements:innerBlock:.

I'm probably missing some logical consequences of the syntactic concept of blocks, methods, and doits, so I would like to hear your thoughts on this.


This is not so much philosophical as rushed.  When I added closures I wanted not to break the compiler and DTSTTCPW because I had a VM to write.  I wanted to clean things up but I risked making too many mistakes so I erred on the side of minimally extending the compiler rather than making it better.

Best,

Christoph


_,,,^..^,,,_
best, Eliot


Carpe Squeak!
Free forum by Nabble Edit this page