Smalltalk › Squeak › Squeak - Dev

argument of ifNotNil: must be a 0-argument block

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

21 messages Options

Zulq Alam-2

argument of ifNotNil: must be a 0-argument block

Hi List,

Why does "Object new ifNotNil: [:object | ]" result in the syntax error
"argument of ifNotNil: must be a 0-argument block" when the code appears
to be fine with this?

ProtoObject>>ifNotNil: ifNotNilBlock
"Evaluate the block, unless I'm == nil (q.v.)"

^ ifNotNilBlock valueWithPossibleArgs: {self}

BlockClosure>>valueWithPossibleArgs: anArray

| n |
(n := self numArgs) = 0 ifTrue: [^ self value].
n = anArray size ifTrue: [^ self valueWithArguments: anArray].
^ self valueWithArguments: (n > anArray size
ifTrue: [anArray, (Array new: n - anArray size)]
ifFalse: [anArray copyFrom: 1 to: n]): anArray

A quick search of Mantis for ifNotNil or valueWithPossibleArgs hasn't
left me any wiser. Something to do with the interpreter, perhaps
optimisation?

Thanks,
Zulq.

Andreas.Raab

Re: argument of ifNotNil: must be a 0-argument block

Zulq Alam wrote:
> Why does "Object new ifNotNil: [:object | ]" result in the syntax error
> "argument of ifNotNil: must be a 0-argument block" when the code appears
> to be fine with this?
>
>
> ProtoObject>>ifNotNil: ifNotNilBlock
> "Evaluate the block, unless I'm == nil (q.v.)"
>
> ^ ifNotNilBlock valueWithPossibleArgs: {self}

Probably because whoever wrote this was blessfully unaware that this
message is never sent but inlined by the compiler. Admitted, the comment
should have said as much but the change was obviously never tested. And
that done in *Proto*Object...

Cheers,
- Andreas

Ken Causey-3

Re: argument of ifNotNil: must be a 0-argument block

In reply to this post by Zulq Alam-2

As Andreas points out this is because this apparent method call is
actually compiled by the compiler. (And the error is actually from the
compiler, not a result of sending the message.) Perhaps incorrectly
depending on your point of view. I would recommend using 'ifNotNilDo:'
instead when you want the offending object.

Ken

On Sat, 2006-09-02 at 00:25 +0100, Zulq Alam wrote:

> Hi List,
>
> Why does "Object new ifNotNil: [:object | ]" result in the syntax error
> "argument of ifNotNil: must be a 0-argument block" when the code appears
> to be fine with this?
>
>
> ProtoObject>>ifNotNil: ifNotNilBlock
> "Evaluate the block, unless I'm == nil (q.v.)"
>
> ^ ifNotNilBlock valueWithPossibleArgs: {self}
>
>
> BlockClosure>>valueWithPossibleArgs: anArray
>
> | n |
> (n := self numArgs) = 0 ifTrue: [^ self value].
> n = anArray size ifTrue: [^ self valueWithArguments: anArray].
> ^ self valueWithArguments: (n > anArray size
> ifTrue: [anArray, (Array new: n - anArray size)]
> ifFalse: [anArray copyFrom: 1 to: n]): anArray
>
>
>
> A quick search of Mantis for ifNotNil or valueWithPossibleArgs hasn't
> left me any wiser. Something to do with the interpreter, perhaps
> optimisation?
>
> Thanks,
> Zulq.
>
>
>

signature.asc (196 bytes) Download Attachment

Mathieu SUEN

Re: argument of ifNotNil: must be a 0-argument block

In reply to this post by Zulq Alam-2

Zulq Alam a écrit :

Because this is checked at compile time:

MessageNode>>transformIfNotNilIfNil: encoder
((self checkBlock: (arguments at: 1) as: 'NotNil arg' from: encoder)
and: [self checkBlock: (arguments at: 2) as: 'Nil arg' from: encoder])
ifTrue:
[selector _ SelectorNode new key: #ifTrue:ifFalse: code: #macro.
receiver _ MessageNode new
receiver: receiver
selector: #==
arguments: (Array with: NodeNil)
precedence: 2
from: encoder.
arguments swap: 1 with: 2.
^true]
ifFalse:
[^false]

So you can see that this message is transform and never send to the object. And it is the #==
message who are send...

But I don't know why the "dummy" method call #valueWithPossibleArgs:.

This it is how it's done in the old compiler but if we tack a look inside the new one(load
NewCompiler + AST package) it can compile and give you what you were expecting:

Object new ifNotNil: [:each | ^each]

give you in bytecode:

17 <40> pushLit: Object
18 <CC> send: new
19 <81 40> storeIntoTemp: 0
21 <73> pushConstant: nil
22 <C6> send: ==
23 <A8 02> jumpTrue: 27
25 <10> pushTemp: 0
26 <7C> returnTop
27 <78> returnSelf

Math

Mathieu SUEN

Re: argument of ifNotNil: must be a 0-argument block

In reply to this post by Ken Causey-3

Ken Causey a écrit :
> As Andreas points out this is because this apparent method call is
> actually compiled by the compiler. (And the error is actually from the
> compiler, not a result of sending the message.) Perhaps incorrectly
> depending on your point of view. I would recommend using 'ifNotNilDo:'
> instead when you want the offending object.
>
> Ken

Yes but it is less faster beceause old compiler don't optimize the message.

>
> On Sat, 2006-09-02 at 00:25 +0100, Zulq Alam wrote:
>> Hi List,
>>
>> Why does "Object new ifNotNil: [:object | ]" result in the syntax error
>> "argument of ifNotNil: must be a 0-argument block" when the code appears
>> to be fine with this?
>>
>>
>> ProtoObject>>ifNotNil: ifNotNilBlock
>> "Evaluate the block, unless I'm == nil (q.v.)"
>>
>> ^ ifNotNilBlock valueWithPossibleArgs: {self}
>>
>>
>> BlockClosure>>valueWithPossibleArgs: anArray
>>
>> | n |
>> (n := self numArgs) = 0 ifTrue: [^ self value].
>> n = anArray size ifTrue: [^ self valueWithArguments: anArray].
>> ^ self valueWithArguments: (n > anArray size
>> ifTrue: [anArray, (Array new: n - anArray size)]
>> ifFalse: [anArray copyFrom: 1 to: n]): anArray
>>
>>
>>
>> A quick search of Mantis for ifNotNil or valueWithPossibleArgs hasn't
>> left me any wiser. Something to do with the interpreter, perhaps
>> optimisation?
>>
>> Thanks,
>> Zulq.
>>
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>>

Ron Teitelbaum

RE: argument of ifNotNil: must be a 0-argument block

In reply to this post by Andreas.Raab

I spent a while tracking this down trying to fix it the other day. It was a
very interesting problem. It passes through a huge amount of code before
deciding what to do. transformifNil -> transformifNilIfNotNil: ->
transformIfTrueIfFalse or something like that. At the end I said ahh it's
not worth it. It's a bummer too because I like the ifNotNil: with an
argument.

Ron

> -----Original Message-----
> From: [hidden email] [mailto:squeak-dev-
> [hidden email]] On Behalf Of Andreas Raab
> Sent: Friday, September 01, 2006 7:40 PM
> To: The general-purpose Squeak developers list
> Subject: Re: argument of ifNotNil: must be a 0-argument block
>
> Zulq Alam wrote:
> > Why does "Object new ifNotNil: [:object | ]" result in the syntax error
> > "argument of ifNotNil: must be a 0-argument block" when the code appears
> > to be fine with this?
> >
> >
> > ProtoObject>>ifNotNil: ifNotNilBlock
> > "Evaluate the block, unless I'm == nil (q.v.)"
> >
> > ^ ifNotNilBlock valueWithPossibleArgs: {self}
>
> Probably because whoever wrote this was blessfully unaware that this
> message is never sent but inlined by the compiler. Admitted, the comment
> should have said as much but the change was obviously never tested. And
> that done in *Proto*Object...
>
> Cheers,
> - Andreas
>

Mathieu SUEN

Re: argument of ifNotNil: must be a 0-argument block

In reply to this post by Andreas.Raab

Andreas Raab a écrit :

> Zulq Alam wrote:
>> Why does "Object new ifNotNil: [:object | ]" result in the syntax
>> error "argument of ifNotNil: must be a 0-argument block" when the code
>> appears to be fine with this?
>>
>>
>> ProtoObject>>ifNotNil: ifNotNilBlock
>> "Evaluate the block, unless I'm == nil (q.v.)"
>>
>> ^ ifNotNilBlock valueWithPossibleArgs: {self}
>
> Probably because whoever wrote this was blessfully unaware that this
> message is never sent but inlined by the compiler.

I think this is more beacause the right behavior is this. As Ken Causey said (old) compiler do it wrong.

Math

Zulq Alam-2

Re: argument of ifNotNil: must be a 0-argument block

In reply to this post by Ken Causey-3

Thanks. I was curious how much this might save so I did a little test
that is most probably flawed :o).

| association |
association := #A -> #Z.
MessageTally spyOn:
[10000000 timesRepeat:
[| key |
key := association key.
key ifNotNil: [key yourself].
key yourself]].
MessageTally spyOn:
[10000000 timesRepeat:
[association key ifNotNilDo: [:key | key yourself].
key yourself]].

On my system #ifNotNilDo costs about 0.000557 ms whereas the inlined
version is virtually free (~0.0001 ms).

Also, while doing this I found what looks to be a bug with regard to the
scope of the block temporary.

| x |
x := 10.
x ifNotNilDo: [:y | ].
y factorial "y isn't in scope!"

This can be done in a workspace though I couldn't put it in a method.

Thanks,
Zulq.

Ken Causey wrote:

> As Andreas points out this is because this apparent method call is
> actually compiled by the compiler. (And the error is actually from the
> compiler, not a result of sending the message.) Perhaps incorrectly
> depending on your point of view. I would recommend using 'ifNotNilDo:'
> instead when you want the offending object.
>
> Ken
>
> On Sat, 2006-09-02 at 00:25 +0100, Zulq Alam wrote:
>> Hi List,
>>
>> Why does "Object new ifNotNil: [:object | ]" result in the syntax error
>> "argument of ifNotNil: must be a 0-argument block" when the code appears
>> to be fine with this?
>>
>>
>> ProtoObject>>ifNotNil: ifNotNilBlock
>> "Evaluate the block, unless I'm == nil (q.v.)"
>>
>> ^ ifNotNilBlock valueWithPossibleArgs: {self}
>>
>>
>> BlockClosure>>valueWithPossibleArgs: anArray
>>
>> | n |
>> (n := self numArgs) = 0 ifTrue: [^ self value].
>> n = anArray size ifTrue: [^ self valueWithArguments: anArray].
>> ^ self valueWithArguments: (n > anArray size
>> ifTrue: [anArray, (Array new: n - anArray size)]
>> ifFalse: [anArray copyFrom: 1 to: n]): anArray
>>
>>
>>
>> A quick search of Mantis for ifNotNil or valueWithPossibleArgs hasn't
>> left me any wiser. Something to do with the interpreter, perhaps
>> optimisation?
>>
>> Thanks,
>> Zulq.
>>
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>>

Mathieu SUEN

Re: argument of ifNotNil: must be a 0-argument block

Zulq Alam a écrit :

> Thanks. I was curious how much this might save so I did a little test
> that is most probably flawed :o).
>
> | association |
> association := #A -> #Z.
> MessageTally spyOn:
> [10000000 timesRepeat:
> [| key |
> key := association key.
> key ifNotNil: [key yourself].
> key yourself]].
> MessageTally spyOn:
> [10000000 timesRepeat:
> [association key ifNotNilDo: [:key | key yourself].
> key yourself]].
>
> On my system #ifNotNilDo costs about 0.000557 ms whereas the inlined
> version is virtually free (~0.0001 ms).
>
> Also, while doing this I found what looks to be a bug with regard to the
> scope of the block temporary.
>
> | x |
> x := 10.
> x ifNotNilDo: [:y | ].
> y factorial "y isn't in scope!"
>
> This can be done in a workspace though I couldn't put it in a method.

This is because block are inline in the method so scope of temps is the method. It's not really good
to have this but I think it was a question of memory space(ask Marcus Denker).

You have also notice that the return statement always know were to return from when it is in block.
That how interpreter know, by creating a context that point the bytecode were the block start in the
method.

For more clear information you can read :
http://www.iam.unibe.ch/~ducasse/FreeBooks/CollectiveNBlueBook/oe-tour-sept19.pdf
and:
http://users.ipa.net/~dwighth/smalltalk/bluebook/bluebook_imp_toc.html

Math

>
> Thanks,
> Zulq.
>
>
> Ken Causey wrote:
>> As Andreas points out this is because this apparent method call is
>> actually compiled by the compiler. (And the error is actually from the
>> compiler, not a result of sending the message.) Perhaps incorrectly
>> depending on your point of view. I would recommend using 'ifNotNilDo:'
>> instead when you want the offending object.
>>
>> Ken
>>
>> On Sat, 2006-09-02 at 00:25 +0100, Zulq Alam wrote:
>>> Hi List,
>>>
>>> Why does "Object new ifNotNil: [:object | ]" result in the syntax
>>> error "argument of ifNotNil: must be a 0-argument block" when the
>>> code appears to be fine with this?
>>>
>>>
>>> ProtoObject>>ifNotNil: ifNotNilBlock
>>> "Evaluate the block, unless I'm == nil (q.v.)"
>>>
>>> ^ ifNotNilBlock valueWithPossibleArgs: {self}
>>>
>>>
>>> BlockClosure>>valueWithPossibleArgs: anArray
>>>
>>> | n |
>>> (n := self numArgs) = 0 ifTrue: [^ self value].
>>> n = anArray size ifTrue: [^ self valueWithArguments: anArray].
>>> ^ self valueWithArguments: (n > anArray size
>>> ifTrue: [anArray, (Array new: n - anArray size)]
>>> ifFalse: [anArray copyFrom: 1 to: n]): anArray
>>>
>>>
>>>
>>> A quick search of Mantis for ifNotNil or valueWithPossibleArgs hasn't
>>> left me any wiser. Something to do with the interpreter, perhaps
>>> optimisation?
>>>
>>> Thanks,
>>> Zulq.
>>>
>>>
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>>
>
>
>

Andreas.Raab

Re: argument of ifNotNil: must be a 0-argument block

In reply to this post by Mathieu SUEN

Mathieu wrote:
>> Probably because whoever wrote this was blessfully unaware that this
>> message is never sent but inlined by the compiler.
>
> I think this is more beacause the right behavior is this.
> As Ken Causey said (old) compiler do it wrong.

No, the compiler got it exactly right. It was faithfully optimizing the
chosen definition of #ifNotNil: - whoever changed the definition "got it
wrong" since the definition should not be changed without also changing
the optimization in the compiler.

But what's *really* annoying to me is that this change went unnoticed.

Cheers,
- Andreas

stéphane ducasse-2

Re: argument of ifNotNil: must be a 0-argument block

On 2 sept. 06, at 03:06, Andreas Raab wrote:

> Mathieu wrote:
>>> Probably because whoever wrote this was blessfully unaware that this
>>> message is never sent but inlined by the compiler.
>> I think this is more beacause the right behavior is this. As Ken
>> Causey said (old) compiler do it wrong.
>
> No, the compiler got it exactly right. It was faithfully optimizing
> the chosen definition of #ifNotNil: - whoever changed the
> definition "got it wrong" since the definition should not be
> changed without also changing the optimization in the compiler.
>
> But what's *really* annoying to me is that this change went unnoticed.

Indeed I think that we should invest in tests and a testserver

Stef

David T. Lewis

Re: argument of ifNotNil: must be a 0-argument block

On Sat, Sep 02, 2006 at 10:29:46AM +0200, st?phane ducasse wrote:

>
> On 2 sept. 06, at 03:06, Andreas Raab wrote:
>
> >Mathieu wrote:
> >>>Probably because whoever wrote this was blessfully unaware that this
> >>>message is never sent but inlined by the compiler.
> >>I think this is more beacause the right behavior is this. As Ken
> >>Causey said (old) compiler do it wrong.
> >
> >No, the compiler got it exactly right. It was faithfully optimizing
> >the chosen definition of #ifNotNil: - whoever changed the
> >definition "got it wrong" since the definition should not be
> >changed without also changing the optimization in the compiler.
> >
> >But what's *really* annoying to me is that this change went unnoticed.
>
> Indeed I think that we should invest in tests and a testserver

Tests are good, but they do not detect this kind of problem.

Dave

Joshua Gargus-2

Re: argument of ifNotNil: must be a 0-argument block

In reply to this post by Ron Teitelbaum

Ron Teitelbaum wrote:

>It's a bummer too because I like the ifNotNil: with an
>argument.
>
>Ron
>
>
There's always #ifNotNilDo:

Josh

Andreas.Raab

Re: argument of ifNotNil: must be a 0-argument block

In reply to this post by David T. Lewis

David T. Lewis wrote:
>>> No, the compiler got it exactly right. It was faithfully optimizing
>>> the chosen definition of #ifNotNil: - whoever changed the
>>> definition "got it wrong" since the definition should not be
>>> changed without also changing the optimization in the compiler.
>>>
>>> But what's *really* annoying to me is that this change went unnoticed.
>> Indeed I think that we should invest in tests and a testserver
>
> Tests are good, but they do not detect this kind of problem.

Why not? If the purpose of a test is to detect a problem then the
inability to load a simple test like:

testIfNotNilArg
self assert: (3 ifNotNil:[:val| val = 3])

is precisely pointing out that there is a problem, e.g., the test
detected the problem and served its intended purposes.

Cheers,
- Andreas

David T. Lewis

Re: argument of ifNotNil: must be a 0-argument block

On Sat, Sep 02, 2006 at 03:00:10PM -0700, Andreas Raab wrote:

> David T. Lewis wrote:
> >>>No, the compiler got it exactly right. It was faithfully optimizing
> >>>the chosen definition of #ifNotNil: - whoever changed the
> >>>definition "got it wrong" since the definition should not be
> >>>changed without also changing the optimization in the compiler.
> >>>
> >>>But what's *really* annoying to me is that this change went unnoticed.
> >>Indeed I think that we should invest in tests and a testserver
> >
> >Tests are good, but they do not detect this kind of problem.
>
> Why not? If the purpose of a test is to detect a problem then the
> inability to load a simple test like:
>
> testIfNotNilArg
> self assert: (3 ifNotNil:[:val| val = 3])
>
> is precisely pointing out that there is a problem, e.g., the test
> detected the problem and served its intended purposes.

Agreed, the test would not load, and that would indicated a problem.
But if I understood right, this test does *not* describe the correct
implementation if #ifNotNil:.

What I meant is that if the intended behavior is something like this
(sorry for the totally bogus example):

testIfNotNilArg
self
should: [3 ifNotNil: [:val | v = 3]]
raise: SyntaxError
whoseDescriptionIncludes: 'argument of ifNotNil: must be a 0-argument block'

and if it were possible to have written and compiled such a unit test,
and if the person implementing #infNotNil: had both the foresight and the
motivation to have done so, then the test still would not have detected
the inappropriate change to to the method source of #ifNotNil (at least
not until you tried to recompile the unit test).

I was refering to the hypothetical perfect world in which unit tests
were broadly implemented, and we were relying on them to catch problems
such as this one. It would not have worked in this case. But you are
right that attempting to write a new test (that documented the incorrect
behavior) at the time of making the change would have detected the problem.

Dave

Klaus D. Witzel

Re: argument of ifNotNil: must be a 0-argument block

In reply to this post by David T. Lewis

On Sat, 02 Sep 2006 14:20:59 +0200, David T. Lewis wrote:
...
> Tests are good, but they do not detect this kind of problem.

Did you mean that in Smalltalk it is impossible to test, whether or not
the compiler accepts a syntactical construct, like in " Compiler evaluate:
'put your test here' ".

If not, mind to explain what it is that cannot be detected.

/Klaus

> Dave

Mathieu SUEN

Re: argument of ifNotNil: must be a 0-argument block

Klaus D. Witzel a écrit :

> On Sat, 02 Sep 2006 14:20:59 +0200, David T. Lewis wrote:
> ...
>> Tests are good, but they do not detect this kind of problem.
>
> Did you mean that in Smalltalk it is impossible to test, whether or not
> the compiler accepts a syntactical construct, like in " Compiler
> evaluate: 'put your test here' ".
>
> If not, mind to explain what it is that cannot be detected.
>
> /Klaus
>
>> Dave
>
>
>

This is beceause #ifNotNil: it's never call(compier inline it) so you do not know if the guy who
write #ifNotNil: implementation do it correctly.

I think it's why he say this.

Math

David T. Lewis

Re: argument of ifNotNil: must be a 0-argument block

On Sun, Sep 03, 2006 at 02:21:47PM +0200, Mathieu wrote:

> Klaus D. Witzel a ?crit :
> > On Sat, 02 Sep 2006 14:20:59 +0200, David T. Lewis wrote:
> > ...
> >> Tests are good, but they do not detect this kind of problem.
> >
> > Did you mean that in Smalltalk it is impossible to test, whether or not
> > the compiler accepts a syntactical construct, like in " Compiler
> > evaluate: 'put your test here' ".
> >
> > If not, mind to explain what it is that cannot be detected.
>
> This is beceause #ifNotNil: it's never call(compier inline it) so you do not know if the guy who
> write #ifNotNil: implementation do it correctly.
>
> I think it's why he say this.

Yes, that's what I meant. I expect that a pre-existing suite of
unit tests would probably not have detected the incorrect change.

Andreas rightly points out that if someone had attempted to write
a unit test that documented the incorrect change, the test would
not have been compilable, and the problem would have been noticed.

Furthermore, if my imaginary pre-existing tests had existed, and
if the person changing #ifNotNil: had also gone to the unit tests
and added something to document the new behavior, then the problem
would have been detected.

This all sounds like a more perfect world that any we are likely
to inhabit, but I certainly don't want to discourage it from
coming to pass, so I hereby withdraw my original remark ;-)

Setting aside the unit test discussion, has this issue been put
on Mantis? If noone has done so, I'll add one later today.

Dave

stéphane ducasse-2

Re: argument of ifNotNil: must be a 0-argument block

but a test tests what we want it to test.
If we want to have a test that check whether the method is correctly
inlined,
it will test that and will fail if not.
We can do a bytecode check or all kind of things.

Now we just have to continue to push the test effort.

Stef

PS: marcus told me that he knew that problems so this was not totally
new.

David T. Lewis

Re: argument of ifNotNil: must be a 0-argument block

In reply to this post by Zulq Alam-2

On Sat, Sep 02, 2006 at 12:25:07AM +0100, Zulq Alam wrote:
>
> Why does "Object new ifNotNil: [:object | ]" result in the syntax error
> "argument of ifNotNil: must be a 0-argument block" when the code appears
> to be fine with this?

I entered Mantis #4867 to track this issue.

Dave