Rethink previous HIJACK commit in light of problems

Further experimenting with HIJACK revealed that direct modification of
the paramlist was not a good idea, due to the potential dependencies
other locations (specializations, adaptations, MAKE FRAME!s, running
function calls...) might depend on the shape of that paramlist not
changing.

This takes back the idea of doing a replacement of the function body
only if the underlying paramlists are compatible...which will be true
for many adaptations, chains, or specializations from copies.  It is
also true for taking the function back from its original--which solves
the issue of un-hijacking (one of the draws of the approach in the
previous commit, but this way works).

However, a non-compatible underlying function means that the frames
will need to be proxied--effectively making a hijack cost similar to
an extra layer of function call.

At an intermediate point in trying to solve this, it was noticed that
f->func is a somewhat misleading name for the frame's current function,
because it changes.  This changes it to f->phase and preserves the
original function in f->original.

Adds a non-trivial specialization test to hijack tests.

src/core/c-context.c

@@ -1397,7 +1397,7 @@ void Assert_Context_Core(REBCTX *c)
         /*REBFRM *f = CTX_FRAME_IF_ON_STACK(c);
         if (f != NULL) {
             REBFUN *rootkey_fun = VAL_FUNC(rootkey);
-            REBFUN *frame_fun = FUNC_UNDERLYING(f->func);
+            REBFUN *frame_fun = FRM_UNDERLYING(f);
 
             if (rootkey_fun != frame_fun) {
                 printf("FRAME! context function doesn't match its REBFRM");

src/core/c-eval.c

@@ -545,7 +545,7 @@ reevaluate:;
 
         Eval_Functions++; // this isn't free...is it worth tracking?
 
-        // Now that we have extracted f->func, we do not have to worry that
+        // Now that we have extracted f->phase, we do not have to worry that
         // f->value might have lived in f->cell.eval.  We can't overwrite
         // f->out during the argument evaluations, in case that is holding the
         // first argument to an infix function, so f->cell gets used for
@@ -554,7 +554,7 @@ reevaluate:;
         assert(f->refine == ORDINARY_ARG || f->refine == LOOKBACK_ARG);
 
         f->arg = f->args_head;
-        f->param = FUNC_FACADE_HEAD(f->func);
+        f->param = FUNC_FACADE_HEAD(f->phase);
         // f->special is END, f->args_head, or first specialized value
 
         // Same as check before switch.  (do_function_arglist_in_progress:
@@ -747,7 +747,7 @@ reevaluate:;
             case PARAM_CLASS_RETURN:
                 assert(VAL_PARAM_SYM(f->param) == SYM_RETURN);
 
-                if (NOT_VAL_FLAG(FUNC_VALUE(f->func), FUNC_FLAG_RETURN)) {
+                if (NOT_VAL_FLAG(FUNC_VALUE(f->phase), FUNC_FLAG_RETURN)) {
                     SET_VOID(f->arg);
                     goto continue_arg_loop;
                 }
@@ -757,8 +757,7 @@ reevaluate:;
                 if (f->varlist) // !!! in specific binding, always for Plain
                     f->arg->extra.binding = f->varlist;
                 else
-                    f->arg->extra.binding =
-                        FUNC_PARAMLIST(FUNC_UNDERLYING(f->func));
+                    f->arg->extra.binding = FUNC_PARAMLIST(FRM_UNDERLYING(f));
 
                 if (f->special != END)
                     ++f->special; // specialization being overwritten is right
@@ -767,7 +766,7 @@ reevaluate:;
             case PARAM_CLASS_LEAVE:
                 assert(VAL_PARAM_SYM(f->param) == SYM_LEAVE);
 
-                if (NOT_VAL_FLAG(FUNC_VALUE(f->func), FUNC_FLAG_LEAVE)) {
+                if (NOT_VAL_FLAG(FUNC_VALUE(f->phase), FUNC_FLAG_LEAVE)) {
                     SET_VOID(f->arg);
                     goto continue_arg_loop;
                 }
@@ -777,8 +776,7 @@ reevaluate:;
                 if (f->varlist) // !!! in specific binding, always for Plain
                     f->arg->extra.binding = f->varlist;
                 else
-                    f->arg->extra.binding =
-                        FUNC_PARAMLIST(FUNC_UNDERLYING(f->func));
+                    f->arg->extra.binding = FUNC_PARAMLIST(FRM_UNDERLYING(f));
 
                 if (f->special != END)
                     ++f->special; // specialization being overwritten is right
@@ -1205,7 +1203,7 @@ reevaluate:;
         }
 
     #if !defined(NDEBUG)
-        if (GET_VAL_FLAG(FUNC_VALUE(f->func), FUNC_FLAG_LEGACY_DEBUG))
+        if (GET_VAL_FLAG(FUNC_VALUE(f->phase), FUNC_FLAG_LEGACY_DEBUG))
             Legacy_Convert_Function_Args(f); // BLANK!+NONE! vs. FALSE+UNSET!
     #endif
 
@@ -1215,7 +1213,7 @@ reevaluate:;
     //
     //==////////////////////////////////////////////////////////////////==//
 
-    execute_func:
+    redo_unchecked:
         assert(IS_END(f->param));
         // refine can be anything.
         assert(
@@ -1251,7 +1249,7 @@ reevaluate:;
         // used to process the return result after the switch.
         //
         REBNAT dispatcher; // goto would cross initialization
-        dispatcher = FUNC_DISPATCHER(f->func);
+        dispatcher = FUNC_DISPATCHER(f->phase);
         switch (dispatcher(f)) {
         case R_FALSE:
             SET_FALSE(f->out); // no VALUE_FLAG_UNEVALUATED
@@ -1294,7 +1292,7 @@ reevaluate:;
 
             ASSERT_ARRAY(VAL_BINDING(f->out));
 
-            if (VAL_BINDING(f->out) == FUNC_PARAMLIST(f->func)) {
+            if (VAL_BINDING(f->out) == FUNC_PARAMLIST(FRM_UNDERLYING(f))) {
                 //
                 // The most recent instance of a function on the stack (if
                 // any) will catch a FUNCTION! style exit.
@@ -1365,11 +1363,11 @@ reevaluate:;
         case R_REDO_UNCHECKED:
             //
             // This instruction represents the idea that it is desired to
-            // run the f->func again.  The dispatcher may have changed the
-            // value of what f->func is, for instance.
+            // run the f->phase again.  The dispatcher may have changed the
+            // value of what f->phase is, for instance.
             //
             SET_END(f->out);
-            goto execute_func;
+            goto redo_unchecked;
 
         case R_REEVALUATE:
             args_evaluate = TRUE; // unnecessary?
@@ -1407,8 +1405,8 @@ reevaluate:;
     // double checks any function marked with RETURN in the debug build.
 
 #if !defined(NDEBUG)
-    if (GET_VAL_FLAG(FUNC_VALUE(f->func), FUNC_FLAG_RETURN)) {
-        REBVAL *typeset = FUNC_PARAM(f->func, FUNC_NUM_PARAMS(f->func));
+    if (GET_VAL_FLAG(FUNC_VALUE(f->phase), FUNC_FLAG_RETURN)) {
+        REBVAL *typeset = FUNC_PARAM(f->phase, FUNC_NUM_PARAMS(f->phase));
         assert(VAL_PARAM_SYM(typeset) == SYM_RETURN);
         if (!TYPE_CHECK(typeset, VAL_TYPE(f->out)))
             fail (Error_Bad_Return_Type(f->label, VAL_TYPE(f->out)));

src/core/c-function.c

@@ -1674,7 +1674,7 @@ REB_R Action_Dispatcher(REBFRM *f)
 {
     enum Reb_Kind type = VAL_TYPE(FRM_ARG(f, 1));
     assert(type < REB_MAX); // actions should not allow void first arguments
-    REBSYM sym = STR_SYMBOL(VAL_WORD_SPELLING(FUNC_BODY(f->func)));
+    REBSYM sym = STR_SYMBOL(VAL_WORD_SPELLING(FUNC_BODY(f->phase)));
     assert(sym != SYM_0);
 
     REBACT subdispatch = Value_Dispatch[type];
@@ -1706,7 +1706,7 @@ REB_R Noop_Dispatcher(REBFRM *f)
 //
 REB_R Datatype_Checker_Dispatcher(REBFRM *f)
 {
-    RELVAL *datatype = FUNC_BODY(f->func);
+    RELVAL *datatype = FUNC_BODY(f->phase);
     assert(IS_DATATYPE(datatype));
     if (VAL_TYPE(FRM_ARG(f, 1)) == VAL_TYPE_KIND(datatype))
         return R_TRUE;
@@ -1721,7 +1721,7 @@ REB_R Datatype_Checker_Dispatcher(REBFRM *f)
 //
 REB_R Typeset_Checker_Dispatcher(REBFRM *f)
 {
-    RELVAL *typeset = FUNC_BODY(f->func);
+    RELVAL *typeset = FUNC_BODY(f->phase);
     assert(IS_TYPESET(typeset));
     if (TYPE_CHECK(typeset, VAL_TYPE(FRM_ARG(f, 1))))
         return R_TRUE;
@@ -1738,7 +1738,7 @@ REB_R Typeset_Checker_Dispatcher(REBFRM *f)
 //
 REB_R Unchecked_Dispatcher(REBFRM *f)
 {
-    RELVAL *body = FUNC_BODY(f->func);
+    RELVAL *body = FUNC_BODY(f->phase);
     assert(IS_BLOCK(body) && IS_RELATIVE(body) && VAL_INDEX(body) == 0);
 
     if (Do_At_Throws(
@@ -1763,7 +1763,7 @@ REB_R Unchecked_Dispatcher(REBFRM *f)
 //
 REB_R Voider_Dispatcher(REBFRM *f)
 {
-    RELVAL *body = FUNC_BODY(f->func);
+    RELVAL *body = FUNC_BODY(f->phase);
     assert(IS_BLOCK(body) && IS_RELATIVE(body) && VAL_INDEX(body) == 0);
 
     if (Do_At_Throws(
@@ -1788,7 +1788,7 @@ REB_R Voider_Dispatcher(REBFRM *f)
 //
 REB_R Returner_Dispatcher(REBFRM *f)
 {
-    RELVAL *body = FUNC_BODY(f->func);
+    RELVAL *body = FUNC_BODY(f->phase);
     assert(IS_BLOCK(body) && IS_RELATIVE(body) && VAL_INDEX(body) == 0);
 
     if (Do_At_Throws(
@@ -1800,7 +1800,7 @@ REB_R Returner_Dispatcher(REBFRM *f)
         return R_OUT_IS_THROWN;
     }
 
-    REBVAL *typeset = FUNC_PARAM(f->func, FUNC_NUM_PARAMS(f->func));
+    REBVAL *typeset = FUNC_PARAM(f->phase, FUNC_NUM_PARAMS(f->phase));
     assert(VAL_PARAM_SYM(typeset) == SYM_RETURN);
 
     // The type bits of the definitional return are not applicable
@@ -1829,22 +1829,49 @@ REB_R Returner_Dispatcher(REBFRM *f)
 //
 REB_R Specializer_Dispatcher(REBFRM *f)
 {
-    REBVAL *exemplar = KNOWN(FUNC_BODY(f->func));
-    f->func = VAL_FUNC(CTX_FRAME_FUNC_VALUE(VAL_CONTEXT(exemplar)));
+    REBVAL *exemplar = KNOWN(FUNC_BODY(f->phase));
+    f->phase = VAL_FUNC(CTX_FRAME_FUNC_VALUE(VAL_CONTEXT(exemplar)));
     f->binding = VAL_BINDING(exemplar);
 
     return R_REDO_UNCHECKED;
 }
 
 
+//
+//  Hijacker_Dispatcher: C
+//
+// A hijacker takes over another function's identity, replacing it with its
+// own implementation, injecting directly into the paramlist and body_holder
+// nodes held onto by all the victim's references.
+//
+// Sometimes the hijacking function has the same underlying function
+// as the victim, in which case there's no need to insert a new dispatcher.
+// The hijacker just takes over the identity.  But otherwise it cannot,
+// and a "shim" is needed...since something like an ADAPT or SPECIALIZE
+// or a MAKE FRAME! might depend on the existing paramlist shape.
+//
+REB_R Hijacker_Dispatcher(REBFRM *f)
+{
+    RELVAL *hijacker = FUNC_BODY(f->phase);
+
+    // We need to build a new frame compatible with the hijacker, and
+    // transform the parameters we've gathered to be compatible with it.
+    //
+    if (Redo_Func_Throws(f, VAL_FUNC(hijacker)))
+        return R_OUT_IS_THROWN;
+
+    return R_OUT;
+}
+
+
 //
 //  Adapter_Dispatcher: C
 //
 // Dispatcher used by ADAPT.
 //
 REB_R Adapter_Dispatcher(REBFRM *f)
 {
-    RELVAL *adaptation = FUNC_BODY(f->func);
+    RELVAL *adaptation = FUNC_BODY(f->phase);
     assert(ARR_LEN(VAL_ARRAY(adaptation)) == 2);
 
     RELVAL* prelude = VAL_ARRAY_AT_HEAD(adaptation, 0);
@@ -1867,9 +1894,9 @@ REB_R Adapter_Dispatcher(REBFRM *f)
         return R_OUT_IS_THROWN;
     }
 
-    f->func = VAL_FUNC(adaptee);
+    f->phase = VAL_FUNC(adaptee);
     f->binding = VAL_BINDING(adaptee);
-    return R_REDO_CHECKED; // Have Do_Core run the adaptee updated into f->func
+    return R_REDO_CHECKED; // Have Do_Core run the adaptee updated into f->phase
 }
 
 
@@ -1880,7 +1907,7 @@ REB_R Adapter_Dispatcher(REBFRM *f)
 //
 REB_R Chainer_Dispatcher(REBFRM *f)
 {
-    REBVAL *pipeline = KNOWN(FUNC_BODY(f->func)); // array of functions
+    REBVAL *pipeline = KNOWN(FUNC_BODY(f->phase)); // array of functions
 
     // Before skipping off to find the underlying non-chained function
     // to kick off the execution, the post-processing pipeline has to
@@ -1896,7 +1923,7 @@ REB_R Chainer_Dispatcher(REBFRM *f)
 
     // Extract the first function, itself which might be a chain.
     //
-    f->func = VAL_FUNC(value);
+    f->phase = VAL_FUNC(value);
     f->binding = VAL_BINDING(value);
 
     return R_REDO_UNCHECKED; // signatures should match
@@ -1959,7 +1986,7 @@ void Get_If_Word_Or_Path_Arg(
 // Expects the following Reb_Frame fields to be preloaded:
 //
 //    f->out (just valid pointer, pointed-to value can be garbage)
-//    f->func
+//    f->phase
 //    f->binding
 //
 // If opt_def is NULL, then f->varlist.context must be set
@@ -2019,7 +2046,7 @@ REB_R Apply_Frame_Core(REBFRM *f, REBSTR *label, REBVAL *opt_def)
 
         f->args_head = CTX_VARS_HEAD(AS_CONTEXT(f->varlist));
 
-        REBCTX *exemplar = FUNC_EXEMPLAR(f->func);
+        REBCTX *exemplar = FUNC_EXEMPLAR(f->phase);
         if (exemplar)
             f->special = CTX_VARS_HEAD(exemplar);
         else
@@ -2039,7 +2066,7 @@ REB_R Apply_Frame_Core(REBFRM *f, REBSTR *label, REBVAL *opt_def)
     // know if the user writes them or not...so making them "write-only"
     // isn't an option either.  One has to
     //
-    f->param = FUNC_FACADE_HEAD(f->func);
+    f->param = FUNC_FACADE_HEAD(f->phase);
     f->arg = f->args_head;
     while (NOT_END(f->param)) {
         if (f->special != END && !IS_VOID(f->special)) {

src/core/c-port.c

@@ -365,7 +365,7 @@ REBOOL Redo_Func_Throws(REBFRM *f, REBFUN *func_new)
     // invocation is the total number of parameters to the *old* function's
     // invocation (if it had no refinements or locals).
     //
-    REBARR *code_array = Make_Array(FUNC_NUM_PARAMS(f->func));
+    REBARR *code_array = Make_Array(FUNC_NUM_PARAMS(f->phase));
     RELVAL *code = ARR_HEAD(code_array);
 
     // We'll walk through the original functions param and arglist only, and
@@ -374,7 +374,7 @@ REBOOL Redo_Func_Throws(REBFRM *f, REBFUN *func_new)
     //
     // !!! See note in function description about arity mismatches.
     //
-    f->param = FUNC_FACADE_HEAD(f->func);
+    f->param = FUNC_FACADE_HEAD(f->phase);
     f->arg = f->args_head;
     REBOOL ignoring = FALSE;
 
@@ -383,7 +383,7 @@ REBOOL Redo_Func_Throws(REBFRM *f, REBFUN *func_new)
     // opposite case where it had only refinements and then the function
     // at the head...
     //
-    REBARR *path_array = Make_Array(FUNC_NUM_PARAMS(f->func) + 1);
+    REBARR *path_array = Make_Array(FUNC_NUM_PARAMS(f->phase) + 1);
     RELVAL *path = ARR_HEAD(path_array);
 
     Move_Value(path, FUNC_VALUE(func_new));

src/core/d-break.c

@@ -182,8 +182,8 @@ REBOOL Do_Breakpoint_Throws(
                 if (
                     frame != FS_TOP
                     && (
-                        FUNC_DISPATCHER(frame->func) == &N_pause
-                        || FUNC_DISPATCHER(frame->func) == &N_breakpoint
+                        FUNC_DISPATCHER(frame->phase) == &N_pause
+                        || FUNC_DISPATCHER(frame->phase) == &N_breakpoint
                     )
                 ) {
                     // We hit a breakpoint (that wasn't this call to
@@ -457,8 +457,8 @@ REBNATIVE(resume)
             if (Is_Function_Frame_Fulfilling(frame)) continue;
 
             if (
-                FUNC_DISPATCHER(frame->func) == &N_pause
-                || FUNC_DISPATCHER(frame->func) == &N_breakpoint
+                FUNC_DISPATCHER(frame->phase) == &N_pause
+                || FUNC_DISPATCHER(frame->phase) == &N_breakpoint
             ) {
                 break;
             }

src/core/d-dump.c

@@ -270,7 +270,7 @@ void Dump_Stack(REBFRM *f, REBCNT level)
 
     REBINT n = 1;
     REBVAL *arg = FRM_ARG(f, 1);
-    REBVAL *param = FUNC_PARAMS_HEAD(f->func);
+    REBVAL *param = FUNC_PARAMS_HEAD(f->phase);
 
     for (; NOT_END(param); ++param, ++arg, ++n) {
         Debug_Fmt(

src/core/d-eval.c

@@ -324,7 +324,8 @@ REBUPT Do_Core_Expression_Checks_Debug(REBFRM *f) {
     TRASH_POINTER_IF_DEBUG(f->args_head);
     TRASH_POINTER_IF_DEBUG(f->varlist);
 
-    TRASH_POINTER_IF_DEBUG(f->func);
+    TRASH_POINTER_IF_DEBUG(f->original);
+    TRASH_POINTER_IF_DEBUG(f->phase);
     TRASH_POINTER_IF_DEBUG(f->binding);
 
     // Mutate va_list sources into arrays at fairly random moments in the

src/core/d-legacy.c

@@ -91,7 +91,7 @@ REBOOL In_Legacy_Function_Debug(void)
 //
 void Legacy_Convert_Function_Args(REBFRM *f)
 {
-    REBVAL *param = FUNC_FACADE_HEAD(f->func);
+    REBVAL *param = FUNC_FACADE_HEAD(f->phase);
     REBVAL *arg = f->args_head;
 
     REBOOL set_blank = FALSE;