Improve Error-tracking for Standard Library functions #4679

StachuDotNet · 2023-01-04T18:42:36Z

StachuDotNet
Jan 4, 2023
Maintainer

This comes out of a conversation on discord

The aim here is to improve trace-ability of errors in stdlib fns.

DvalSource is used throughout Dark's Runtime to track the source of Incomplete or Errors.
It has two values - SourceID of tlid * id and SourceNone (implying we don't have context of the source).
The Interpreter currently does a good job of tracking the sources of Dvals.

When a SourceID is returned (rather than a SourceNone), the Editor does a great job of highlighting the source of an Error:

The stdlib fns, however, do a poor job of tracking the source of errors - all stdlib functions that return a DError currently simply return a DError(SourceNone, "message") without context of which bit of code (often a fn arg) caused the error. I believe this is a limitation of our standard library functions simply not having the 'id's of each of their params.

The definition of BuiltInFnSig is
type BuiltInFnSig = (ExecutionState * List<Dval>) -> DvalTask
and a standard library function demonstrating this limitation may be found in Date::parse:

    { name = fn "Date" "parse" 0
      parameters = [ Param.make "s" TStr "" ]
      returnType = TDate
      description = "Parses a string representing a date and time..."
      fn =
        (function
        | _, [ DStr s ] ->
          match ocamlCompatibleDateParser s with
          | Error () -> Ply(DError(SourceNone, "Invalid date format"))
          | Ok d -> Ply(DDate d)
        | _ -> incorrectArgs ())
      sqlSpec = NotQueryable
      previewable = Pure
      deprecated = ReplacedBy(fn "Date" "parse" 1) }

If we updated BuiltInFnSig to instead look like
BuiltInFnSig = (ExecutionState * List<id * Dval>) -> DvalTask
then this function definition could instead be

      fn =
        (function
        | state, [ (sID, DStr s) ] ->
          match ocamlCompatibleDateParser s with
          | Error () -> Ply(DError(SourceID(state.tlid, sID), "Invalid date format"))
          | Ok d -> Ply(DDate d)
        | _ -> incorrectArgs ())

In order to support such, many things would have to change in the Interpreter (just a bunch of id-tracking and type changes) - I started going down this path, but got lost. This is me recording my thoughts/progress, to see if this makes sense to pursue after a break. What do you think?

Beyond this limitation with Standard Library functions, we also over-use SourceNone, or could seemingly choose a "better" (more precise) id to reference - those can be handled separately.

pbiggar · 2023-01-05T00:00:57Z

pbiggar
Jan 5, 2023

I believe this is a limitation of our standard library functions simply not having the 'id's of each of their params.

Can you clarify why the ID of each param matters? I'm not really sure what ID we want there and why, so I suspect we have a lack of articulation of what we want to put in the sourceID. But, I would have though we want the ID of the FnCall expression. Maybe a few use cases (specifically, what is the user doing) would help clarify this for us.

In the example of Date::parse "not a date", I'm not sure pointing at "not a date" is more valuable than pointing at the entire expression, and I think we could argue that the string expression is fine and it is only when Date::parse is called on it that a problem arises.

0 replies

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Improve Error-tracking for Standard Library functions #4679

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{editor}}'s edit

{{editor}}'s edit

Uh oh!

Replies: 1 comment

Uh oh!

{{title}}

Uh oh!

Select a reply

Uh oh!

Uh oh!

Improve Error-tracking for Standard Library functions #4679

Uh oh!

Uh oh!

StachuDotNet Jan 4, 2023 Maintainer

Replies: 1 comment

Uh oh!

pbiggar Jan 5, 2023

StachuDotNet
Jan 4, 2023
Maintainer

pbiggar
Jan 5, 2023