Fix issue with nested binary operators counting as constant typed #567
Conversation
|
@yosiat I refactored a bunch in the eval_binary_node, if you wanted to take a look it would be much appreciated. |
| se, err := stateful.NewExpression(node) | ||
| if err != nil { | ||
| t.Fatalf("Failed to compile expression: %v", err) | ||
| panic(fmt.Sprintf("Failed to compile expression: %v", err)) |
There was a problem hiding this comment.
Why we do panic here? If one test is failing, we fail here the whole test suite.
By using "Fatal" we fail this test only.
There was a problem hiding this comment.
I changed it from panic so that you get an ugly stack trace ;) The stack trace makes it easier to figure out which expression didn't compile. Otherwise you have to look through all the expressions in a test.
Also it should only fail when you are changing a test and accidentally create an expression that fails to compile. If its possible for a given expression not to compile then don't use this method and check the error explicitly.
|
@nathanielc LGTM, except the change in mustCompileExpression which I added a comment. |
Fixes #561
Changes the error detection to compile time if possible.
Also refactors the
invalid ... operatorerrors to be left/right symmetric and correctly report Regex errors.