Skip to content

Commit

Permalink
GH-18 Fix LOAD and EVALUATE handling of input source specification.
Browse files Browse the repository at this point in the history
  • Loading branch information
@SirWumpus
SirWumpus committed Aug 18, 2024
1 parent f51f831 commit 11590df
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 14 deletions.
25 changes: 15 additions & 10 deletions src/post4.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,8 @@ p4Repl(P4_Ctx *ctx, int rc)
P4_WORD("_hook_add", &&_hook_add, 0, 0x10), // p4
P4_WORD("_hook_call", &&_hook_call, 0, 0x00), // p4
#endif
P4_WORD("_input_push", &&_input_push, 0, 0x0600), // p4
P4_WORD("_input_pop", &&_input_pop, 0, 0x6000), // p4
P4_WORD("_longjmp", &&_longjmp, 0, 0x10), // p4
P4_WORD("_rs", &&_rs, 0, 0x03), // p4
P4_WORD("_rsp@", &&_rsp_get, 0, 0x01), // p4
Expand Down Expand Up @@ -1811,6 +1813,18 @@ _rm_marker: x.w = w.xt;
p4WordFree(word);
NEXT;

_input_push: p4StackGuards(ctx);
w.p = ctx->rs.top+1;
P4_DROP(ctx->rs, -sizeof (P4_Input) / sizeof (P4_Cell));
(void) memcpy(w.v, &ctx->input, sizeof (ctx->input));
NEXT;

_input_pop: P4_DROP(ctx->rs, sizeof (P4_Input) / sizeof (P4_Cell));
w.p = ctx->rs.top+1;
(void) memcpy(&ctx->input, w.v, sizeof (ctx->input));
p4StackGuards(ctx);
NEXT;

This comment has been minimized.

Sign in to view

Copy link
@ruv

ruv Aug 19, 2024

Contributor

As I see, you choose saving and restoring the input source spec on the Forth code level 😄 (as that was discussed)

Why not save ctx->input in a C local variable, i.e., use P4_INPUT_PUSH and P4_INPUT_POP?

It seems, it is impossible that P4_INPUT_PUSH saves the different values in the same local variable (in the same activation). Because a different value will be in another activation (in a recursive call of p4Repl).

This comment has been minimized.

Sign in to view

Copy link
@SirWumpus

SirWumpus Aug 19, 2024

Author Owner

As I see, you choose saving and restoring the input source spec on the Forth code level

Ya. I saw the writing on the walls (floor, ceiling).

P4_INPUT_PUSH and P4_INPUT_POP best used in nested C call frames. One issue I noticed though was with CATCH I could rewind the Forth stacks, but the C frames would be out of sync and frankly confusing debug.

As you suggested (and I suspected long time before) I would have to separate some operational units into smaller pieces ("words" even wow 🤯🤔), _input_push and _input_pop for use directly in Forth. I resisted this a lot because I wanted to save Forth stack space in favour of the C stack (a cheat I guess). Ok I could make the Forth stacks bigger, but that seemed like another cheat since Forth has historically managed with small stacks.

The other question does the input source specification need to consist of so many words? Essentially fileid, buffer, length (SOURCE), size, offset (>IN), and BLK. I suppose size could be hard coded throughout, but that doesn't save much.

Input with INCLUDED, INCLUDE-FILE, LOAD and EVALUATE shall be nestable in any order to at least eight levels.

So 8 levels * 6 words = 48 words (of 64 default stack size data/return) plus return IPs (8) = 56. Gets expensive. Using the C stack for some of this seemed like a good idea.

This comment has been minimized.

Sign in to view

Copy link
@ruv

ruv Aug 19, 2024 •
edited
Loading

Contributor

P4_INPUT_PUSH and P4_INPUT_POP best used in nested C call frames. One issue I noticed though was with CATCH I could rewind the Forth stacks, but the C frames would be out of sync and frankly confusing debug.

I see now. It's really a problem.
One idea how to keep them in sync: Save catch_frame and store 0 to it in p4Repl(). Then, you get control back in p4Repl even when throw occurs in Forth, and there is another catch (that was called before this p4Repl activation).

Update: I mean, SETJMP is performed in p4Repl() too.

This comment has been minimized.

Sign in to view

Copy link
@ruv

ruv Aug 19, 2024 •
edited
Loading

Contributor

The other question does the input source specification need to consist of so many words? Essentially fileid, buffer, length (SOURCE), size, offset (>IN), and BLK.

Do you mean so many memory words, or Forth words?

So 8 levels * 6 words = 48 words (of 64 default stack size data/return) plus return IPs (8) = 56. Gets expensive. Using the C stack for some of this seemed like a good idea.

As for memory. It's possible to use different amount of memory depending on the input source kind. But difference is too small compared to the size of the input buffer, which must be separate for each nesting of the input source.

When the stacks are in the memory (in the heap), it does not matter where you allocate memory for nesting the input source — in the return stack, or in the heap. You are limited be the total amount of memory, which is huge on desktops.
// ( i*x caddr u -- j*x )
_evaluate: w = P4_POP(ctx->ds);
x = P4_POP(ctx->ds);
Expand Down Expand Up @@ -2900,22 +2914,13 @@ p4EvalFile(P4_Ctx *ctx, const char *file)
int
p4EvalString(P4_Ctx *ctx, const P4_Char *str, size_t len)
{
int rc;

/* Do not save STATE, see A.6.1.2250 STATE. */
P4_INPUT_PUSH(&ctx->input);

ctx->input.fp = (FILE *) -1;
ctx->input.buffer = (P4_Char *) str;
ctx->input.length = len;
ctx->input.offset = 0;
ctx->input.blk = 0;

rc = p4Repl(ctx, P4_THROW_OK);

P4_INPUT_POP(&ctx->input);

return rc;
return p4Repl(ctx, P4_THROW_OK);
}

#ifdef TEST
Expand Down
6 changes: 2 additions & 4 deletions src/post4.p4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1900,18 +1900,16 @@ VARIABLE SCR
\
\ (S: i*x u -- j*x )
\
: LOAD BLK @ >R DUP BLK ! BLOCK 1024 ['] EVALUATE CATCH R> BLK ! THROW ;
: LOAD _input_push DUP BLK ! BLOCK 1024 ['] EVALUATE CATCH _input_pop THROW ;

\ ... EVALUATE ...
\
\ ( i*x caddr u -- j*x )
\
\ Redefine in order to set BLK to zero _after_ implementing LOAD.
\
\ @see
\ https://forth-standard.org/standard/block/EVALUATE
\
: EVALUATE BLK @ >R 0 BLK ! ['] EVALUATE CATCH R> BLK ! THROW ;
: EVALUATE _input_push ['] EVALUATE CATCH _input_pop THROW ;

This comment has been minimized.

Sign in to view

Copy link
@ruv

ruv Aug 19, 2024

Contributor

I would suggest to use _evaluate or (evaluate) instead of EVALUATE for that internal word.

This comment has been minimized.

Sign in to view

Copy link
@SirWumpus

SirWumpus Aug 19, 2024

Author Owner

Good idea.

\ ... THRU ...
\
Expand Down

0 comments on commit 11590df

Please sign in to comment.