Re: [PATCH 0/2] Add more context to E_constraint msg

[Jose E. Marchesi]

> Hi Mohammad.
>
>> Hi Jose.
>>
>> This series enrich the msg field of E_constraint to give the user
>> more information of the definition location and evaluation location.
>>
>> Example:
>>
>> ```
>> (poke) type A =  struct { int i == 1; }
>> (poke) var a = A{}
>> (poke) a.i++
>> unhandled constraint violation exception
>> failed expression
>>   i == 1
>> in field A.i evaluated at <stdin>:1:1 (defined at <stdin>:1:24)
>> ```
>>
>> I think, it's much more readable than
>>
>> ```
>> unhandled constraint violation exception
>> failed expression
>>   i == 1
>> in field A.i
>> ```
>>
>> Imagine when you're loading a pickle that's loading another one,
>> and somewhere there, a constraint violation is happening.
>>
>> One thing that is, IMHO, even better, is to add a `detailed_info` field
>> to the exception, and report the whole stack of locations from the
>> first expr/stmt to the core expr that is the main point of failure.
>>
>> Currently, I've introduced a `locationstack' to keep track of the
>> location info, and only reporting the location from top of that stack.
>
> This is very nice.  I like the approach.
>
> I also agree that would be good to have a detailed_info field with a
> textual representation of the stack, like:
>
>   in field A.i
>   evaluated at LOC (defined at LOC)
>             at LOC (defined at LOC)
>             ...
>
> I think it would be good to have the "evaluated ..." in its own line,
> separated from the "in field FIELD" line.
>
> A Poke function could be written:
>
>   fun _pkl_print_location_stack (uint<32> indent_level = 0,
>                                  uint<32> depth = 0) void:
>
> That could print:
>
> at LOC (defined at LOC)NEWLINE
> INDENT_LEVEL_SPACEsat LOC (defined at LOC)NEWLINE
>
> Or something like that.

s/print/format I guess.

Actually, why not turning the location field in Exception from a single
string to an array of strings?  Then we could have a function in the
runtime that builds an array from the locations stack, another function
that prints the location array out, etc.

Makes sense?

> Also, I think this needs more tests 8-)
>
>>
>>
>> Regards,
>> Mohammad-Reza
>>
>>
>> Mohammad-Reza Nabipoor (2):
>>   pkl: report location of constraint definition
>>   pkl: report evaluation location in E_constraint exception
>>
>>  ChangeLog                           |  54 +++++++++++++
>>  libpoke/pkl-ast.c                   |   6 ++
>>  libpoke/pkl-ast.h                   |   7 ++
>>  libpoke/pkl-gen.c                   |  77 +++++++++++++++++++
>>  libpoke/pkl-gen.pks                 |   4 +
>>  libpoke/pkl-insn.def                |   5 ++
>>  libpoke/pkl-rt.pk                   |  16 ++--
>>  libpoke/pkl-tab.y                   |  22 +++++-
>>  libpoke/pvm.jitter                  | 113 ++++++++++++++++++++++++++++
>>  testsuite/poke.pickles/pcap-test.pk |   3 +-
>>  10 files changed, 299 insertions(+), 8 deletions(-)