kuroko-lang / kuroko

Dialect of Python with explicit variable declaration and block scoping, with a lightweight and easy-to-embed bytecode compiler and interpreter.
https://kuroko-lang.github.io/
MIT License
431 stars 25 forks source link

keyword arguments can't default to themselves #24

Closed notwa closed 2 years ago

notwa commented 2 years ago

this bug is a little awkward to describe in words, so instead, consider the code:

def greet(print=print):
    print("Hello, world.")

greet()
Traceback (most recent call last):
  File "issue01b.krk", line 4, in <module>
    greet()
  File "issue01b.krk", line 2, in greet
    print("Hello, world.")
TypeError: 'object' object is not callable

at first, I thought this behavior was specific to built-ins, but I just now thought to try it with other types:

let message = "Hello, world."

def greet(message=message):
    print(repr(message))

greet()

ArgumentError: repr() takes exactly 1 argument (0 given)

this bug only occurs when the left hand side and right hand side are the same. therefore, these programs will work:

let message = "Hello, world."

def greet(message_=message, print_=print):
    print_(message_)

greet()
let message_ = "Hello, world."
let print_ = print

def greet(message=message_, print=print_):
    print(message)

greet()

tested on commit 8fb1689.

klange commented 2 years ago

This looks to be a case of scope confusion in argumentDefinition.

I opted to have default arguments expanded at call time rather than function definition time, and the way this is done is by inlining the default expression behind a virtual branch on a sentinel check. That expression is being compiled incorrectly and sees the argument name as a local variable, resulting in the reassignment of the sentinel value.

Since this results in an escaped sentinel value, it results in an erroneous argument count when the variable is then used in a function call.

There are two places in the compiler where argumentDefinition is called, and I think both need to have the define step moved after the compilation of the relevant expression.

klange commented 2 years ago

There is an open question to answer about whether just the current argument name should be unbound in the default expression context, or if the whole argument list should remain unbound. I think the latter makes more sense, but I can come up with ways the former could be useful.

klange commented 2 years ago

Masking only the current argument name is a lightweight change:

diff --git a/src/compiler.c b/src/compiler.c
index 2ff7a30..3737fc4 100644
--- a/src/compiler.c
+++ b/src/compiler.c
@@ -583,6 +583,9 @@ static ssize_t resolveLocal(Compiler * compiler, KrkToken * name) {
            if (local->depth == -1) {
                error("Invalid recursive reference in declaration initializer");
            }
+           if (local->depth == -2) {
+               continue;
+           }
            return i;
        }
    }
@@ -1235,6 +1238,10 @@ static void typeHint(KrkToken name) {
    current->enclosing->enclosed = NULL;
 }

+static void hideLocal(void) {
+   current->locals[current->localCount - 1].depth = -2;
+}
+
 static void argumentDefinition(void) {
    if (match(TOKEN_EQUAL)) {
        /*
@@ -1350,13 +1357,14 @@ static void function(FunctionType type, size_t blockWidth) {
            }
            ssize_t paramConstant = parseVariable("Expected parameter name.");
            if (parser.hadError) goto _bail;
-           defineVariable(paramConstant);
+           hideLocal();
            if (check(TOKEN_COLON)) {
                KrkToken name = parser.previous;
                match(TOKEN_COLON);
                typeHint(name);
            }
            argumentDefinition();
+           defineVariable(paramConstant);
        } while (match(TOKEN_COMMA));
    }
    stopEatingWhitespace();
@@ -1569,8 +1577,9 @@ static void lambda(int exprType) {
        do {
            ssize_t paramConstant = parseVariable("Expected parameter name.");
            if (parser.hadError) goto _bail;
-           defineVariable(paramConstant);
+           hideLocal();
            argumentDefinition();
+           defineVariable(paramConstant);
        } while (match(TOKEN_COMMA));
    }
>>> def foo(abs=abs):
  >     print(abs)
  >     
 => <function foo at 0x5600e054c020>
>>> foo()
<function abs at 0x5600e0527330>
>>> foo(abs)
<function abs at 0x5600e0527330>
>>> foo('test')
test

It also allows for default arguments to reference previous arguments, which I think is neat:

>>> def foo(baz=123,bar=baz):
  >     print(bar)
  >     
 => <function foo at 0x55e8a3cd3990>
>>> foo(47)
47
>>> foo(47,82)
82
>>>
notwa commented 2 years ago

nice work! I think this was the last show-stopper that I ran into in my experiment.