New trace mechanism#1069
Conversation
denismerigoux
left a comment
There was a problem hiding this comment.
Congratulations Vincent, this is a major piece of work and very well-done!
| let tag_to_runtime = function | ||
| | ScopeCall scope_name -> | ||
| let name = ScopeName.original_base scope_name in | ||
| let _, decl_pos = ScopeName.get_info scope_name in | ||
| Runtime.ScopeCall { name; decl_pos = Expr.pos_to_runtime decl_pos } | ||
| | ScopeVarDef { var; io } -> | ||
| let name = ScopeVar.to_string var in | ||
| let _, decl_pos = ScopeVar.get_info var in | ||
| Runtime.ScopeVarDef | ||
| { var = { name; decl_pos = Expr.pos_to_runtime decl_pos }; io } | ||
| | LocalVarDef { name } -> Runtime.LocalVarDef name | ||
| | LocalTupDef { names } -> Runtime.LocalTupDef names | ||
| | FunCall topdef -> | ||
| let name = TopdefName.original_base topdef in | ||
| let _, decl_pos = TopdefName.get_info topdef in | ||
| Runtime.FunCall { name; decl_pos = Expr.pos_to_runtime decl_pos } | ||
| | BranchingCondition -> BranchingCondition | ||
| | Branching None -> IfBranching | ||
| | Branching (Some constructor_name) -> MatchBranching { constructor_name } | ||
| | Exception { label; cons_pos } -> | ||
| let label = | ||
| match label with | ||
| | None -> None | ||
| | Some (l, p) -> Some (l, Expr.pos_to_runtime p) | ||
| in | ||
| Exception { label; cons_pos = Expr.pos_to_runtime cons_pos } | ||
| | Assertion -> Assertion | ||
|
|
There was a problem hiding this comment.
Is there a strong reason for the tags in Runtime to not be exactly the same as the tags in Definitions?
There was a problem hiding this comment.
I wanted to keep track of the UIds so that the renaming traversal could also handle these. I'm not sure it's currently mandatory but I thought it would be more resilient this way.
1329df9 to
192d7e0
Compare
7b74d63 to
868730b
Compare
| let cond = translate_expr ctx cond in | ||
| let etrue = translate_expr ctx etrue in | ||
| let efalse = translate_expr ctx efalse in | ||
| Expr.eifthenelse cond etrue efalse m |
There was a problem hiding this comment.
How is this different from the base case Expr.map ~f:(translate_expr ctx) ~op:Operator.translate (e, m) ?
There was a problem hiding this comment.
The difference is 5 extra-lines :) Fixed up.
| match io.io_input with | ||
| | Runtime.NoInput -> "NoInput" | ||
| | Runtime.OnlyInput -> "OnlyInput" | ||
| | Runtime.Reentrant -> "Reentrant" |
There was a problem hiding this comment.
Maybe reuse Catala_runtime.Json.io_input ?
(it takes a Buffer.t but there already is a str wrapper for most of its siblings)
There was a problem hiding this comment.
I removed this function altogether. Also, I tried to enforce snake case in the JSON so that won't do it.
| typ : typ; | ||
| io : Desugared.Ast.io; | ||
| e : 'm expr; | ||
| e : 'm expr; (* sub_scope_var: *) |
There was a problem hiding this comment.
Oops, some refactoring artifact got in there. I'll remove it.
| | Error (error, locs, message) as e | ||
| when trace_context.exception_handled = false | ||
| -> | ||
| trace_context.exception_handled <- true; |
There was a problem hiding this comment.
Maybe try to preserve the backtrace ?
There was a problem hiding this comment.
I tried, I swear. But this wouldn't give a proper backtrace. I suspected weird intertwining with clerk with in the end I couldn't understand what happened. I didn't dig much more into it. Do you want me to get back at it ?
There was a problem hiding this comment.
No need to stall the whole PR, we'll have a look later on if needed.
| (** {3:instruments Logging instruments} *) | ||
| It also catches runtime's [error]s and insert a [single_trace] before | ||
| re-raising it. When it occurs, a flag is also set so that callers do not | ||
| catch it again. *) |
There was a problem hiding this comment.
I might expect this to return 'a * trace ; maybe at least make it clear that retrieve_trace can be called afterwards ?
There was a problem hiding this comment.
Why would you expect that? The previous traces constructors all return unit, why would this one give you a trace ? Also, the val retrieve_trace : unit -> trace function is two lines below.
We could change the name from with_trace to wrap_trace or something if you'd prefer. I agree that it's not necessarily the best naming.
| let default | ||
| ~code_coverage | ||
| ~(trace : [ `FileName of Catala_utils.Global.raw_file | `Stdout ] option) | ||
| ~(trace_format : Catala_utils.Global.format_enum option) |
There was a problem hiding this comment.
I wish these two could be gathered in a single record (but that's in Catala CLI, outside of the scope of this PR).
c3fb257 to
f4797c3
Compare
Co-authored-by: Louis Gesbert <louis.gesbert@inria.fr>
f4797c3 to
357dfef
Compare
This PR replaces the existing logging mechanism with a trace-like mechanism that outputs metadata for important program execution points.
The existing logging displayed scope calls, scope variables definitions, exceptions. We augment those with local bindings, conditionals pathing, function calls, and more. The trace is also organized as a tree so that we have a good idea of the callstacks.
For now, traces are supported by the OCaml and java backend but are disabled in python. Traces outputs in backends are only available as JSON objects. Although, it is still possible to access them through the API.
The "user-friendly" output is still available through the interpreter when
--trace-format human(default) is specified.The new output may be observed in the cram tests diffs.
We also plugged a
--traceoption inclerk run.JSON trace outputs generated by the interpreter and backends aim to be equivalent but, in some specific cases, it will not be. E.g., functions in java are not represented as values and, hence, are not displayed (
"<function>"), the OCamloroperator is still lazy compared to the "official" semantics that the other backends follows.P.S. Sorry for the huge commit, the original git history was not salvageable.
Edit:
Checklist
If this PR adds a feature or has breaking changes