HaxeFoundation / haxe

Haxe - The Cross-Platform Toolkit
https://haxe.org
6.18k stars 655 forks source link

[eval] exception when stepping over to end of function #8020

Open Gama11 opened 5 years ago

Gama11 commented 5 years ago
class Main {
    static function main() {
        foo();
    }

    static macro function foo()
        return macro {};
}

This crashes with the following:

Thread 1 killed on uncaught exception Option.No_value
Raised at file "option.ml", line 44, characters 18-26
Called from file "_build/src/macro/eval/evalDebugSocket.ml", line 750, characters 35-43
Called from file "_build/src/core/json/jsonRpc.ml", line 45, characters 5-9
Called from file "_build/src/core/json/jsonRpc.ml", line 45, characters 5-9
Called from file "_build/src/core/json/jsonRpc.ml", line 45, characters 5-9
Called from file "_build/src/core/json/jsonRpc.ml", line 45, characters 5-9
Called from file "_build/src/core/json/jsonRpc.ml", line 45, characters 5-9
Called from file "_build/src/core/json/jsonRpc.ml", line 45, characters 5-9
Called from file "_build/src/core/json/jsonRpc.ml", line 45, characters 5-9
Called from file "_build/src/core/json/jsonRpc.ml", line 45, characters 5-9
Called from file "_build/src/core/json/jsonRpc.ml", line 45, characters 5-9
Called from file "_build/src/core/json/jsonRpc.ml", line 45, characters 5-9
Called from file "_build/src/core/json/jsonRpc.ml", line 45, characters 5-9
Called from file "_build/src/macro/eval/evalDebugSocket.ml", line 754, characters 3-10
Called from file "thread.ml", line 39, characters 8-14

This worked in rc.1.

Simn commented 5 years ago

Can you check if a debug adapter can tell vscode that execution should continue? So far we only support termination, but that is not correct on macro exit.

Gama11 commented 5 years ago

Shouldn't it just not send a terminated event here (assuming that's what's happening)?

Simn commented 5 years ago

Oops wrong issue x)

Simn commented 5 years ago

Shouldn't it just not send a terminated event here (assuming that's what's happening)?

That's not the problem. I think the problem is that the adapter is still in "paused on step" mode, which is why it sends another stack trace request. We should obviously change it so that the request doesn't cause a crash, but ideally it shouldn't send that request in the first place.

Simn commented 5 years ago

The vscode interaction is still not ideal here, but I'm not sure what it wants from me. I tried sending a ContinuedEvent but that changed nothing. In cases like this:

class Main {
    static function main() {
        foo();
        trace("ok");
        bar();
    }

    static macro function foo() {
        return macro {};
    }

    static macro function bar() {
        return macro {};
    }
}

If you break on the foo macro and step over, you'll get the correct stack trace but the cursor line doesn't move. The debug log also seems weird:

Sending command: {"id":9,"method":"next","params":{"threadId":0}}
GOT MESSAGE {"jsonrpc":"2.0","id":9,"result":null}

Sending command: {"id":10,"method":"getThreads","params":{}}
GOT MESSAGE {"jsonrpc":"2.0","id":10,"result":[{"id":0,"name":"Thread 0"}]}

Sending command: {"id":11,"method":"stackTrace","params":{"threadId":0}}
GOT MESSAGE {"jsonrpc":"2.0","id":11,"error":{"code":1,"message":"No frame found"}}

Sending command: {"id":12,"method":"stackTrace","params":{"threadId":0}}
GOT MESSAGE {"jsonrpc":"2.0","id":12,"result":[{"id":3,"name":"Main.bar","source":"C:\\GitHub\\HaxeRepro\\source\\Main.hx","line":13,"column":3,"endLine":13,"endColumn":18,"artificial":false},{"id":4,"name":"source/Main.hx","source":"C:\\GitHub\\HaxeRepro\\source\\Main.hx","line":5,"column":3,"endLine":5,"endColumn":8,"artificial":true}]}

Note how after the "next" it immediately sends another "getThreads". I wonder if we're supposed to only send a reply to "next" if we're in a stopped state again. But then the question becomes what if we aren't? Like in this case with a macro exit...

Gama11 commented 5 years ago

Hm.. while Haxe no longer crashes, it also doesn't show that "leaving-function indicator" from #7767 anymore it seems? Or is that part of the non-ideal VSCode interaction you mentioned?

Simn commented 5 years ago

Somebody told me it should only do that when control flow falls through, not on return. Which means it's never going to do that for macros.

Gama11 commented 5 years ago

Oh, right. And actually, it does work for init macros.

Simn commented 5 years ago

Something else we have to figure out is who is supposed to send whom termination events and who is responsible for closing the socket gracefully.

Simn commented 5 years ago

I'm tentatively setting this to 4.1 because I don't know if I'll do another eval-debugger pass before we release that. I think the main issue here has been addressed and the rest is not high priority.