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