jingoro2112 / wrench

practical embedded script interpreter
MIT License
106 stars 9 forks source link

Feature request: native runtime errors #24

Closed ohordiichuk closed 4 months ago

ohordiichuk commented 4 months ago

It would be great if native function calls can stop wr_run execution if it's needed. For example, in case of bad parameters supplied by script or some fatal unrecoverable error that must stop further script execution. Since the library already has WRError, it should be possible to easily extend it for 3rd parties:

enum WRError {
  ...
WR_ERR_user = 10000, // some big value to make some space for future library's errors
}

void wr_setLastError(WRContext* c, WRError error) {
     c->w->err = error; // not sure if "w" can be accessed like that
}

And then in wr_callFunction in all places where ccb is called, e.g.:

CASE(CallFunctionByHash):
{
    args = READ_8_FROM_PC(pc++);

    // initialize a return value of 'zero'
    register0 = stackTop;
    register0->p = 0;
    register0->p2 = INIT_AS_INT;

    if ( ! ((register1 = w->globalRegistry.getAsRawValueHashTable(READ_32_FROM_PC(pc)))->ccb) )
    {
        w->err = WR_ERR_function_hash_signature_not_found;
        return 0;
    }

    register1->ccb( context, stackTop - args, args, *stackTop, register1->usr );

        // ADD lines below:
        if (w->err) {
               return 0;
        }

        ....

P.S.: Sorry, if I'm opening too many issues :) I just want to share our feedback using this library and make it better. I think I also can help with implementation if needed.

jingoro2112 commented 4 months ago

Not at all! I would absolutely love help on the implementation. Also any feedback/amendments on the documentation would be most welcome, it's hard to keep up. Sort of a one-programmer operation here :P

On Sun, Jun 2, 2024 at 9:51 AM ohordiichuk @.***> wrote:

It would be great if native function calls can stop wr_run execution if it's needed. For example, in case of bad parameters supplied by script or some fatal unrecoverable error that must stop further script execution. Since the library already has WRError, it should be possible to easily extend it for 3rd parties:

enum WRError { ... WR_ERR_user = 10000, // some big value to make some space for future library's errors }

void wr_setLastError(WRContext* c, WRError error) { c->w->err = error; // not sure if "w" can be accessed like that }

And then in wr_callFunction in all places where ccb is called, e.g.:

CASE(CallFunctionByHash): { args = READ_8_FROM_PC(pc++);

// initialize a return value of 'zero' register0 = stackTop; register0->p = 0; register0->p2 = INIT_AS_INT;

if ( ! ((register1 = w->globalRegistry.getAsRawValueHashTable(READ_32_FROM_PC(pc)))->ccb) ) { w->err = WR_ERR_function_hash_signature_not_found; return 0; }

register1->ccb( context, stackTop - args, args, *stackTop, register1->usr );

    if (w->err) {
           return 0;
    }

    ....

P.S.: Sorry, if I'm opening too many issues :) I just want to share our feedback using this library and make it better. I think I also can help with implementation if needed.

— Reply to this email directly, view it on GitHub https://github.com/jingoro2112/wrench/issues/24, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALIKAYOU5I3YSCWLXRQVRTZFMPMZAVCNFSM6AAAAABIVCV2ISVHI2DSMVQWIX3LMV43ASLTON2WKOZSGMZDSNRWG42TMMQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

ohordiichuk commented 4 months ago

Okay, great. I'll try to do this feature this week.

jingoro2112 commented 4 months ago

oops sorry too late I already implemented it :P

Also putting finishing touches on import, new turned out to be a pain because the namespace table is linked into the imported code, so I had to change the bytecode format a bit and push to v5.0.0

On Sun, Jun 2, 2024 at 1:58 PM ohordiichuk @.***> wrote:

Okay, great. I'll try to do this feature this week.

— Reply to this email directly, view it on GitHub https://github.com/jingoro2112/wrench/issues/24#issuecomment-2143970095, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALIKA6ZHRFXNN3UVABOBDDZFNMMZAVCNFSM6AAAAABIVCV2ISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBTHE3TAMBZGU . You are receiving this because you commented.Message ID: @.***>

ohordiichuk commented 4 months ago

Oh, no problem. Great you have already done it :)

jingoro2112 commented 4 months ago

okay so this is a PRE release version that includes import functionality. it is NOT backwards compatible to 4.*

it includes wr_import(), sys::importCompile(), and sys::importByteCode()

Also there is sys::halt( err ) which takes an err argument between 100-255 as "user" level errors and will return immediately with that error.

I have some test code inside wrench_cli.cpp for the import testing but have not added it to the unit tests yet, JUST got this working and am anxious for you to try it out.

-Curt

On Mon, Jun 3, 2024 at 10:54 AM ohordiichuk @.***> wrote:

Oh, no problem. Great you have already done it :)

— Reply to this email directly, view it on GitHub https://github.com/jingoro2112/wrench/issues/24#issuecomment-2145411587, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALIKA6KZTDQUDUHZJM6P53ZFR7RXAVCNFSM6AAAAABIVCV2ISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBVGQYTCNJYG4 . You are receiving this because you commented.Message ID: @.***>

jingoro2112 commented 4 months ago

OH.. in order for this to work you have to use the new "export" keyword on structs that you want to export. this is not necessary for functions, they just work, but 'export' ensures the namespace hash table is generated and embedded in the bytecode, normally it only does this if it detects that you 'new'ed it somewhere. 'export' forces it to always be generated.

On Tue, Jun 4, 2024 at 12:48 AM Curt Hartung @.***> wrote:

okay so this is a PRE release version that includes import functionality. it is NOT backwards compatible to 4.*

it includes wr_import(), sys::importCompile(), and sys::importByteCode()

Also there is sys::halt( err ) which takes an err argument between 100-255 as "user" level errors and will return immediately with that error.

I have some test code inside wrench_cli.cpp for the import testing but have not added it to the unit tests yet, JUST got this working and am anxious for you to try it out.

-Curt

On Mon, Jun 3, 2024 at 10:54 AM ohordiichuk @.***> wrote:

Oh, no problem. Great you have already done it :)

— Reply to this email directly, view it on GitHub https://github.com/jingoro2112/wrench/issues/24#issuecomment-2145411587, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALIKA6KZTDQUDUHZJM6P53ZFR7RXAVCNFSM6AAAAABIVCV2ISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBVGQYTCNJYG4 . You are receiving this because you commented.Message ID: @.***>

ohordiichuk commented 4 months ago

Good job! We are happy to test it. But it seems you forgot to push the code into GitHub :)

jingoro2112 commented 4 months ago

I didn't forget, you are gettin ga pre-release :)

I'll push it up later today. once I have the unit tests complete.

On Tue, Jun 4, 2024 at 5:01 AM ohordiichuk @.***> wrote:

Good job! We are happy to test it. But it seems you forgot to push the code into GitHub :)

— Reply to this email directly, view it on GitHub https://github.com/jingoro2112/wrench/issues/24#issuecomment-2146989772, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALIKA5O727SCV5SEC75XGLZFV7APAVCNFSM6AAAAABIVCV2ISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBWHE4DSNZXGI . You are receiving this because you commented.Message ID: @.***>

jingoro2112 commented 4 months ago

okay current release implements this as sys::halt()