JoeStrout/miniscript

Functions referenced via indexing are invoked as command statements

JoeStrout opened this issue · 5 comments

When indexing (i.e. using []) into a list or map results in a function reference, that function should never be invoked; it should evaluate to just the function reference itself. This works correctly in expressions, but in a command statement, the function is actually invoked, just as if you'd used dot syntax. This is confusing and inconsistent.

Demonstration, showing several correct and incorrect cases:

> a = function(arg)
>>> print "Function invoked with argument: " + arg
>>> end function
> a
Function invoked with argument: 
> a 42
Function invoked with argument: 42
> arr = [@a]
> arr[0]    // <--- BUG: should print a function reference
Function invoked with argument: 
> arr[0] 42  // <--- BUG: should produce a Compiler Error
Function invoked with argument: 42
> x = arr[0]
> x
Function invoked with argument: 
> m = {}
> m.foo = @a
> m.foo
Function invoked with argument: 
> m["foo"]   // <--- BUG: should print a function reference
Function invoked with argument: 
> x = m.foo
Function invoked with argument: 
> x = m.foo(42)
Function invoked with argument: 42
> x = m["foo"]   // <-- correct (assigns the function reference to x)
> x = m["foo"](42)   // <-- correct
Compiler Error: got LParen where EOL is required [line 1]

The cases marked "BUG" should return a function reference, or if the user follows this with arguments (with or without parentheses), it should produce a Compiler Error.

(This issue affects both the C# and the C++ implementations.)

See #88 for the observation that led to discovering this issue.

Note that there is some chance that user code may be relying on this buggy behavior. So we may want to have a release or two where it prints a compile-time deprecation warning, but continues to work, before we make it throw a hard error.

It seems the rule is never mentioned in MiniScript-Manual.pdf. Maybe it needs update.

I don't think this is worth rolling out breaking changes for. This feature (yes, not a bug) is quite useful and sees real world use case, while feeling harmonic with the rest of MiniScript, so breaking user code just to remove this feature feels very wrong.

Last time deprecations and breaking change was used to fix an actual bug with default function arguments, but that was broken behavior, while in this case everything is working fine, just doesn't quite line up with your language design ideas, which is not good enough reason to roll out breaking changes.

OK, I will hold off on any action for now. It needs further consideration.