#1260 Inlined ifTrue:/ifFalse: invalid when false/true sometimes

Closed
opened 2 years ago by max · 6 comments
max commented 2 years ago

The code the compiler generates for the assignment of an inlined ifTrue:/ifFalse: completely ignores the implicit alternativeBlock.

this:

(true ifFalse: [3]) isNil

evaluates to true, while

| a |
a := 1.
a := true ifFalse: [3].
a isNil

is false and "a" still equals 1

The code the compiler generates for the assignment of an inlined ifTrue:/ifFalse: completely ignores the implicit alternativeBlock. this: ```smalltalk (true ifFalse: [3]) isNil ``` evaluates to true, while ```smalltalk | a | a := 1. a := true ifFalse: [3]. a isNil ``` is false and "a" still equals 1
Herby Vojčík commented 2 years ago
Owner

(edited to remove Smalltalk core assert: which is unnecessary and should not be used that way, actually it should not be used at all by hand, is purely a compilation helper and can be removed any time)

Nice one. Thanks.

(edited to remove `Smalltalk core assert:` which is unnecessary and should not be used that way, actually it should not be used at all by hand, is purely a compilation helper and can be removed any time) Nice one. Thanks.
Herby Vojčík commented 2 years ago
Owner

Has some similarities with #1242, maybe it helps with the solution.

Has some similarities with https://lolg.it/amber/amber/issues/1242, maybe it helps with the solution.
Herby Vojčík commented 2 years ago
Owner

Same problem with other context:

foo
[ ^ true ifFalse: [ 3 ] ] value

returns self.

The root cause is the same: compiler does not generate else branch.

Same problem with other context: ```smalltalk foo [ ^ true ifFalse: [ 3 ] ] value ``` returns self. The root cause is the same: compiler does not generate `else` branch.
Herby Vojčík commented 2 years ago
Owner

Other contexts work only by the side-effect of not generating else branch actually leading to nil semantics: eg. (true ifFalse: [2]) isNil or foo ^ true ifFalse: [2]; the former leaves internal var undefined which is interpreted as nil; the second doesn't generate return self as there is a return statement, returning undefined which is again interpreted as nil.

Other contexts work only by the side-effect of not generating `else` branch actually leading to `nil` semantics: eg. `(true ifFalse: [2]) isNil` or `foo ^ true ifFalse: [2]`; the former leaves internal var `undefined` which is interpreted as `nil`; the second doesn't generate `return self` as there is a return statement, returning `undefined` which is again interpreted as `nil`.
Herby Vojčík commented 2 years ago
Owner

Sorry for the delay, this is indeed a bug, but didn't have opportunity to tackle it, it needs bigger change in compiler (essentially, making everything into ifTrue:ifFalse: and the optimizing out the branches that actually do nothing).

Sorry for the delay, this is indeed a bug, but didn't have opportunity to tackle it, it needs bigger change in compiler (essentially, making everything into `ifTrue:ifFalse:` and the optimizing out the branches that _actually_ do nothing).
Herby Vojčík commented 2 years ago
Owner

Actually, when I looked into the code more deeply, I found out there is more direct (though not as perfect) solution for exactly this problem, so I used it.

It also allowed me to distinguish if[Not]Nil:s that use / don't use the return value, so non-value using ones are compiled if slightly better code.

Will be included in the next release, hopefully soon.

Actually, when I looked into the code more deeply, I found out there is more direct (though not as perfect) solution for exactly this problem, so I used it. It also allowed me to distinguish `if[Not]Nil:`s that use / don't use the return value, so non-value using ones are compiled if slightly better code. Will be included in the next release, hopefully soon.
Sign in to join this conversation.
No Label
No Milestone
No assignee
2 Participants
Loading...
Cancel
Save
There is no content yet.