Source map improvements #1716

vouillon · 2024-10-18T10:18:26Z

I started working on improving the source map output of Wasm_of_ocaml, and got a bit side-tracked...

Many changes to preserve debug information from the bytecode as well as possible and put this information in the source map at appropriate places for the JavaScript debuggers.

Explain a bit how they are used by the debuggers.

The debuggers do not stop on some statements, like function declarations. So there is no point in outputting some debug information there.

The best position is at the beginning of the expression after the `=`.

There are some locations which are marked as ghost locations, but which actually make sense.

Chrome will stop right after a return before returning from a function. This will yield to an ambiguity if there is a statement there. To prevent this, we add a space after a return statement when source maps are enabled.

The Chrome debugger uses this information to get the original names of variables. It does not look at all the identifier occurences but only at where they are bound. Also, we provide the original name, not the shortened name, which is not very useful.

- Do not repeat the output if the location has not changed - Start a new source map mapping before outputing the debug info as a comment (we need to start a mapping right after a return statement, moving it after a comment would not work) - Outputting an identifier name will not change the location (there is no point in doing that)

This is simpler than to associate a location to all instructions. Also, this avoid the issue of locations getting lost because an instruction gets optimized away.

In particular, no longer track the location of variables. This may make sense during code generation, though: the location of a statement should be the earliest location of the effectful expressions it contains (if there is any).

If the first block of the function starts with an event, we use the event's location. This is useful in case of tail-calls, where some code is generated before this first block. If we don't have an event, we set the initial location to unkown to prevent previous locations to bleed into the function body.

There is some code at the beginnning of exception handlers for which we have no debugging information.

Statements will get the earliest location of the expressions performing a call it contains, if there is any. Then, for an expression `if (e) {...}`, we don't move first to the location of the conditional before going back to the location within `e` which might be strictly before. When queueing expressions, we ignore their locations if they don't perform a call; when flushed, we use the current location. This again prevent locations to be reordered.

The generated code is also hidden by using a dummy file.

Firefox assumes that a mapping stops at the end of a line, which is inconvenient. When this happens, we repeat the mapping on the next line.

Outputting this source map would fail on 32-bit architectures since its size, once Base64-encoded, would be over the string length limit of 16 MiB.

hhugo · 2024-10-18T21:25:34Z

compiler/lib/js_output.ml

+ ^ ^ ^
+
+Chrome uses the location of the opening parenthesis of a function
+declaration to determine the function name in the stack.


Where do these infos come from ? Are there any link you can join to this doc ?

hhugo · 2024-10-18T21:34:56Z

compiler/lib/js_output.ml

@@ -456,6 +456,33 @@ struct
 in
 traverse l e

+ let stop_on_statement st =


Maybe add a comment here

The debuggers do not stop on some statements, like function declarations. So there is no point in outputting some debug information there.

Where does this info come from ?

hhugo · 2024-10-18T21:44:20Z

compiler/lib/js_traverse.ml

+ , (Return_statement (Some e1, _), _)
+ , Some (Return_statement (Some e2, _), _) ) ->
+ ( Return_statement
+ (Some (ECond (cond, e1, e2)), U (*ZZZ Use end of function? *))


Is this a todo ?

hhugo · 2024-10-18T21:50:43Z

compiler/lib/ocaml_compiler.ml

+ | Env.Env_module (_summary, ident, _, description) when Ident.same ident ident' ->
+ Some description.Types.md_loc
+ | Env.Env_extension (_summary, ident, description) when Ident.same ident ident' ->
+ Some description.Types.ext_loc


Why doesn't this affect tests-full ?

hhugo · 2024-10-18T22:09:34Z

compiler/lib/specialize_js.ml

+ ( Extern "caml_blit_string"
+ , [ Pv a'; Pc (Int zero1); Pv bytes'; Pc (Int zero2); Pv alen'' ] ) )
+ , _ )
+ ; (Event _, _)


This pattern seems fragile, can we do better ?

hhugo · 2024-10-18T22:11:50Z

compiler/tests-compiler/effects_continuations.ml

- ? caml_cps_call2(Stdlib[79], cst_toto$0, _o_)
- : caml_cps_call2(Stdlib[79], cst_titi, _o_);
+ ? caml_cps_call2
+ (Stdlib[79], cst_toto$0, function(_q_){return _p_(_q_);})


Why do we have this diff ?

Maybe some inlining optim needs to be updated ?

hhugo · 2024-10-18T22:47:03Z

toplevel/examples/lwt_toplevel/dune

+ (link_flags
+ :standard
+ ; The source map would be too large on 32-bit architectures otherwise
+ --no-source-map)


Wouldn't it be enough to disable the inline sourcemap ?

(link_flags (:standard --source-map \ --source-map-inline))

Or maybe just disable sourcemap on 32bit arch only

hhugo · 2024-10-18T23:02:11Z

compiler/lib/generate.ml

@@ -501,55 +501,165 @@ let constant ~ctx x level =
 type queue_elt =
 { prop : int
 ; ce : J.expression
- ; loc : J.location


The let* rewrite make it hard to review the rest of the diff. could this commit be split, maybe ?

hhugo · 2024-10-18T23:08:23Z

compiler/lib/parse_bytecode.ml

+ - omitted else clause
+ *)
+ instrs
+ | _, _ ->


Can we have an exhaustive pattern for event (at least on ev_kind) ?

hhugo · 2024-10-18T23:42:28Z

compiler/lib/js_output.ml

+ let ignore_list =
+ List.filter sources ~f:(fun filename ->
+ String.length filename >= 9
+ && String.equal (String.sub filename ~pos:0 ~len:9) "/builtin/")


String.starts_with

vouillon added 21 commits October 18, 2024 10:38

Document source maps

1a65ad0

Explain a bit how they are used by the debuggers.

JavaScript if statement simplification: use appropriate locations

9e7293d

Do not emit unnecessary source mappings

a66efd9

The debuggers do not stop on some statements, like function declarations. So there is no point in outputting some debug information there.

Adjust source mapping location for variable declarations

0fcce13

The best position is at the beginning of the expression after the `=`.

Put the function call location on the opening parenthesis as well

3193b8e

Add location after return statement

57de9c1

Add location on new expression

51e9623

Change way of filtering out dummy locations

3ab65af

There are some locations which are marked as ghost locations, but which actually make sense.

Indicate the origin name of modules and exceptions as well

07fed13

Generate: put location on function calls

8a2eca5

Add space to remove ambiguity

4f1e6f6

Chrome will stop right after a return before returning from a function. This will yield to an ambiguity if there is a statement there. To prevent this, we add a space after a return statement when source maps are enabled.

Add an Event instruction

af63029

This is simpler than to associate a location to all instructions. Also, this avoid the issue of locations getting lost because an instruction gets optimized away.

Parse_bytecode: clean-up

fa160b9

In particular, no longer track the location of variables. This may make sense during code generation, though: the location of a statement should be the earliest location of the effectful expressions it contains (if there is any).

Propage locations across blocks

cb31d9a

Compile exception handlers starting from an unkown location

4f855fc

There is some code at the beginnning of exception handlers for which we have no debugging information.

Add appropriate events for partial application and over application

d333be1

Mutually recursive functions: add locations in wrapper

12855e5

vouillon requested a review from hhugo October 18, 2024 10:18

vouillon force-pushed the source-maps branch from c54e85e to 4494d81 Compare October 18, 2024 10:31

Source maps: use ignoreList to hide runtime files

96f2f74

The generated code is also hidden by using a dummy file.

vouillon force-pushed the source-maps branch 2 times, most recently from 0dc5e0e to 2913f20 Compare October 18, 2024 14:12

vouillon added 2 commits October 18, 2024 16:29

Source map: repeat mappings over new lines

f88fb80

Firefox assumes that a mapping stops at the end of a line, which is inconvenient. When this happens, we repeat the mapping on the next line.

Changes

4e10e29

vouillon force-pushed the source-maps branch from 2913f20 to 4e10e29 Compare October 18, 2024 14:30

LWT toplevel example: do not generate a source map

52715dc

Outputting this source map would fail on 32-bit architectures since its size, once Base64-encoded, would be over the string length limit of 16 MiB.

hhugo reviewed Oct 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Source map improvements #1716

Source map improvements #1716

vouillon commented Oct 18, 2024

hhugo Oct 18, 2024

hhugo Oct 18, 2024

hhugo Oct 18, 2024

hhugo Oct 18, 2024

hhugo Oct 18, 2024

hhugo Oct 18, 2024

hhugo Oct 18, 2024

hhugo Oct 18, 2024

hhugo Oct 18, 2024

hhugo Oct 18, 2024

hhugo Oct 18, 2024

Source map improvements #1716

Are you sure you want to change the base?

Source map improvements #1716

Conversation

vouillon commented Oct 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment