Opal is fun.
I have a few simplifications to propose for the AST->BC OCASTTranslator
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) I think 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).
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...
I can open 1 or more issues if there is an interest.
Nicolas
