Add mechanism to provide context to exceptions

[mbasmanova]

Errors happening during expression evaluation are hard to troubleshoot because
it is not clear which sub-expression triggered the error. This PR adds a
mechanism to specify context for exceptions via a thread_local variable and
uses it to specify the expression being evaluated and add it to the exception.

Here are some examples of the new "context" information being added to the
exceptions:

(c0 + c1) % 0 -> Context: mod(cast(plus(c0, c1)), literal)
c0 + (c1 % 0) -> Context: mod(cast(c1), literal)

[facebook-github-bot]

@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[oerling]

This is a nice idea. I’d suggest to make the setter take something like Typedef std::string(*ContextMessageFunc)(void* atrg); Setter::Setter(ContextMessageFunc func, void* arg); Making the string when going inside the block is way too expensive. But pushing a function pointer and a pointer that can refer to stack allocated extra state information, e.g. the Expr(* costs nothing. Note that a lambda with capture, when its lifetime is not obvious and its only use inlined, will be heap allocated and thus expensive. So a C function pointer and a void8* argument are the best choice here since these can be on all the time with no measurable overhead. Another idea is to also print these in the SIGSEGV handler, along with the status line from process/TraceContext.h.

[Yuhta]

This is a nice idea. I’d suggest to make the setter take something like Typedef std::string(ContextMessageFunc)(void atrg); Setter::Setter(ContextMessageFunc func, void* arg); Making the string when going inside the block is way too expensive. But pushing a function pointer and a pointer that can refer to stack allocated extra state information, e.g. the Expr(* costs nothing. Note that a lambda with capture, when its lifetime is not obvious and its only use inlined, will be heap allocated and thus expensive. So a C function pointer and a void8* argument are the best choice here since these can be on all the time with no measurable overhead. Another idea is to also print these in the SIGSEGV handler, along with the status line from process/TraceContext.h.

Can we cache the string in Expr to avoid recomputing for every batch?

[facebook-github-bot]

@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[mbasmanova]

@oerling Orri, thank you for the suggestion. I modified the code accordingly. Would you take another look?

[facebook-github-bot]

@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[oerling]

Making the string at prepare time or first use would work in this case but would take a lot of memory. Consider strings of outer expressions contain those of inner ones, at least to a point, even if we print … past a certain depth. So making the string on first need is far better, specially if we have tens of thousands of Expr instances like in some very wide projections. Another question of semantics is whether we just consider the innermost of all the nested contexts on the stack. Probably all, I’d say.