CMDHELPER-3174 - Reported by Pieter12345

Running the optimizer on:
@var = @bool1 || !@bool2
Results in:
sconcat(assign(@var,or(@bool1)),@bool2)
Which is equivalent to:
@var = or(@bool1); @bool2;
Workaround:
@var = @bool1 || not(@bool2)

CommandHelper version: 3.3.2-SNAPSHOT build 3381

Comment by PseudoKnight

Confirmed for build #3320, so it wasn't recently introduced.

Comment by PseudoKnight

Hah. Wow. I ran into three more bugs within seconds of each other when looking into this.

@var = @a || ++@b;
// or more sensibly, something like...
@var = @a || ++@b == 3;

Error: 'Expecting an integer, but received "||" instead'.

If I post increment instead, some fun things happen.

@var = @a || @b++ == 3;

Error: 'Unexpected symbol (==). Did you forget to quote your symbols?'

If I throw this in a closure, something I do to see the compiled output, we get a stacktrace.

closure(@var = @a || @b++);

Exception in thread "main" java.lang.IndexOutOfBoundException: Index: 3, Size: 3
  at java.util.ArrayList.rangeCheck(Unknown Source)
  at java.util.ArrayList.get(Unknown Source)
  at com.laytonsmith.core.functions.Compiler$__autoconcat__.optimizeSpecial(Compiler.java:234)
  at com.laytonsmith.core.functions.Compiler$__autoconcat__.optimizeDynamic(Compiler.java:144)
  at com.laytonsmith.core.MethodScriptCompiler.optimizeAutoconcats(MethodScriptCompiler.java:1764
  ...

Comment by PseudoKnight

Problem is here:

https://github.com/EngineHub/CommandHelper/blob/master/src/main/java/com/laytonsmith/core/functions/Compiler.java#L230

It looks like this is for multi-assignment chaining, but it's eating the "||" and "!" as the next two children, leaving @b because it's not a symbol. I'm not sure why it's going by twos here, exactly. If it's for chaining, it should maybe be checking if it's actually chaining an ivar and an equals symbol.

Comment by LadyCailin

Order of operations is screwed up in edge cases like this. While it's fine to patch this, the real solution is to finish writing the object oriented, more formalized version here. https://github.com/EngineHub/CommandHelper/blob/master/src/main/java/com/laytonsmith/core/compiler/LexerObject.java and here: https://github.com/EngineHub/CommandHelper/blob/master/src/main/java/com/laytonsmith/core/compiler/CompilerObject.java and particularly here: https://github.com/EngineHub/CommandHelper/blob/master/src/main/java/com/laytonsmith/core/compiler/NewMethodScriptCompiler.java

While I don't think this is useable as is, (it's probably easier to start over) it should be rewritten using this as the inspiration. Particularly using the LexerObject would be a huge improvement, as it formalizes and cleans up the tokenization, and then formalize the order of operations (I don't think I ever actually wrote that particular code).

Comment by Pieter12345

Update:
Since somewhere between build 3511 and 3589, this bug results in an uncaught exception during compile time. In build 3590, this was changed to a compile exception (even though the code is syntactically valid). This update is positive in the sense that code now breaks at compile time instead of having wrong runtime behavior,

Another workaround is to use braces: @var = @bool1 || (!@bool2) instead of @var = @bool1 || !@bool2.