Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[eval] Don't choke on env_parent = None #11679

Merged
merged 12 commits into from
Jul 19, 2024

Conversation

kLabz
Copy link
Contributor

@kLabz kLabz commented May 30, 2024

Still not sure exactly how things go wrong here. Broke with 6e4fd96

@Simn
Copy link
Member

Simn commented Jun 3, 2024

As I've said on Slack this isn't the right fix, and the problem suggests that there's something wrong with the exception management.

kLabz added 2 commits June 27, 2024 16:23
Note that something wrong might be happening somewhere for this to happen...
@kLabz kLabz force-pushed the fix/compiler_failure_during_build_macro branch from dbba36c to ec58537 Compare June 27, 2024 14:32
@kLabz kLabz marked this pull request as ready for review June 27, 2024 14:32
@kLabz kLabz marked this pull request as draft June 27, 2024 14:36
@kLabz kLabz force-pushed the fix/compiler_failure_during_build_macro branch from ec58537 to 16312f8 Compare June 27, 2024 14:45
@kLabz
Copy link
Contributor Author

kLabz commented Jun 27, 2024

So this comes from exceptions in build macros triggered by another build macro.
Not sure if this env kind business is correct; do you have a better idea there?

Hopefully the included repro helps (along with the change in evalMain showing where we get an EKEntryPoint triggering the issue)

Note: should check eval debugger behavior here; will try to setup that...

@Simn
Copy link
Member

Simn commented Jun 28, 2024

I think this is a decent change, but I worry that it fixes the issue only incidentally... Either way I'm fine with going ahead with this.

@kLabz kLabz linked an issue Jun 28, 2024 that may be closed by this pull request
@kLabz
Copy link
Contributor Author

kLabz commented Jun 28, 2024

Testing this against eval debugger, with:

  • Original test (build macro + build macro + Context.parseInlineString)
  • Expr macro variant (expr macro + build macro + Context.parseInlineString)

Note that for these, Haxe compiler doesn't show the full stack:

[ERROR] src/Bar.hx:1: characters 1-2

 1 | @:build(Macro.build())
   | ^
   | Unexpected &

Eval debugger shows the whole stack, like current test (build macro + build macro + throw):
image

I updated kind_name (locally) to something I think is a bit better:
image

And also works for expression macro => build macro:
image


Haxe compiler not showing the stack for Context.parseInlineString example now seems weird...

@kLabz
Copy link
Contributor Author

kLabz commented Jun 28, 2024

With this, I can get the stack displayed with compiler too; but not sure we actually want that

diff --git a/src/macro/eval/evalExceptions.ml b/src/macro/eval/evalExceptions.ml
index b3954e206..7140610ca 100644
--- a/src/macro/eval/evalExceptions.ml
+++ b/src/macro/eval/evalExceptions.ml
@@ -139,7 +139,15 @@ let catch_exceptions ctx ?(final=(fun() -> ())) f p =
                                                        end else
                                                                Error.raise_typing_error (Printf.sprintf "Unexpected value where haxe.macro.Error was expected: %s" (s_value 0 v).sstring) null_pos
                                                ) (EvalArray.to_list sub)
-                               | _ -> []
+                               | _ ->
+                                       (* Careful: We have to get the message before resetting the context because toString() might access it. *)
+                                       let stack = match eval_stack with
+                                               | [] -> []
+                                               | l when p' = null_pos -> l (* If the exception position is null_pos, we're "probably" in a built-in function. *)
+                                               | _ :: l -> l (* Otherwise, ignore topmost frame position. *)
+                                       in
+                                       let stack = get_exc_error_stack ctx stack in
+                                       List.map (fun p -> (Error.Custom "Called from here", p)) (List.rev stack)
                        in
                        reset_ctx();
                        final();

Edit 2: will see if I can handle this earlier (where we create the haxe.macro.Error) doesn't seem to make much sense in make_runtime_error 🤔

Edit: surprisingly, this doesn't have a big impact on our misc tests:

Done running 661 tests with 1 failures
SUMMARY:
projects/Issue9389/compile-fail.hxml
 Main.hx:3: characters 5-8 : boop
+Main.hx:3: characters 3-9 : Called from here

Which actually sounds like a nice change to me, adding the position from where that macro was called:

───────┬──────────────────────────────────────────────────────────────────────────
       │ File: projects/Issue9389/Main.hx
───────┼──────────────────────────────────────────────────────────────────────────
   1   │ class Main {
   2   │     static function main() {
   3   │         f(123);
   4   │     }
   5   │ 
   6   │     static macro function f(e) {
   7   │         throw new haxe.macro.Expr.Error("boop", e.pos);
   8   │     }
   9   │ }
───────┴──────────────────────────────────────────────────────────────────────────

@kLabz kLabz self-assigned this Jul 18, 2024
@kLabz kLabz marked this pull request as ready for review July 19, 2024 08:04
@kLabz
Copy link
Contributor Author

kLabz commented Jul 19, 2024

Usual random windows/macro failure is unrelated

@kLabz kLabz merged commit 1a66e9b into development Jul 19, 2024
98 of 99 checks passed
@kLabz kLabz deleted the fix/compiler_failure_during_build_macro branch January 21, 2025 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Eval compiler exception when building hxb
2 participants