FogBugz (Case [Issue]10470) Compiler Opal - Simplify handling of ForEffect ForValue in Translator

A FogBugz case was assigned to Clement Bera by Nicolas Cellier.

Case ID:      10470
Title:        Simplify handling of ForEffect ForValue in Translator
Status:       Work Needed
Category:     Enhancement
Project:      Compiler Opal
Area:         Misc
Priority:     6 - Fix If Time
Milestone:    Later
Assigned To:  Clement Bera

URL:          https://pharo.fogbugz.com/default.asp?10470


Changes:
Category changed from 'Bug' to 'Enhancement'.
Milestone changed from 'Undecided' to 'Later'.

As discussed on mailing list:

1) instead of testing isEffectTranslator or refining in subclass, most often we could just let self visit the node. Indeed, self will already be either a Translator ForEffect or ForValue.

For example, this could apply to #emitIfFalseIfTrue: #emitIfNilIfNotNil: #visitCascadeNode:

This might also remove a few useless pushNil; pop BC sequences.

2) We can simplify the #visitBlockNode: / #visitSequenceNode: pair.

Indeed, visitSequenceNode: tests if its parent isBlock, and if so will emitValue, else emitEffect.
Inversely visitBlockNode: always ask to emitEffect, but since it contains a SequenceNode, this is a big lie.

I suggest to always let the valueTranslator emitValue visitBlockNode: and simplify #visitSequenceNode: to just let (self visitNode: statements last).

In this case, we also need to remove visitInlinedBlockNode: ForEffect

3) I suggest changing the BC generated for or: and and:

instead of:

valueTranslator visitNode: aMessageNode receiver.
methodBuilder
jumpAheadTo: #else if: false;
pushLiteral: true;
jumpAheadTo: #end;
jumpAheadTarget: #else.
valueTranslator visitInlinedBlockNode: aMessageNode arguments first.
methodBuilder jumpAheadTarget: #end.

We could just use dup and thus remove a jump:

valueTranslator visitNode: aMessageNode receiver.
methodBuilder
pushDup;
jumpAheadTo: #end if: true;
popTop.
self visitInlinedBlockNode: aMessageNode arguments first.
methodBuilder jumpAheadTarget: #end.

ForEffect, this would just be

self emitIfFalse: aMessageNode

Of course, that last change might disturb the Decompiler a bit, but since we care far less now...


You are subscribed to this case.  If you do not want to receive automatic notifications in the future, unsubscribe (https://pharo.fogbugz.com/default.asp?pre=preUnsubscribe&pg=pgEditBug&command=view&ixBug=10470) from this case.

_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker