Handle phase errors more elegantly

A noble idea for reporting type checking errors that originate
from internal phases was implemented in a way that made it hard
to pass in symbols to argument type errors.  This refactors the
code and uses a switcheroo in the error ID to accomplish sharing
of code between the internal phase error and the ordinary arg
type error.

src/boot/errors.r

@@ -97,9 +97,6 @@ Script: [
 
     find-string-binary: {Can't search binary in string (must convert input)}
 
-    phase-bad-arg-type:
-        [:arg1 {internal phase disallows} :arg2 {for its} :arg3 {argument}]
-
     expect-val:         [{expected} :arg1 {not} :arg2]
     expect-type:        [:arg1 :arg2 {field must be of type} :arg3]
     cannot-use:         [{cannot use} :arg1 {on} :arg2 {value}]
@@ -148,7 +145,12 @@ Script: [
     invalid-arg:        [:arg1 {has an invalid} :arg2 {argument:} :arg3]
     isotope-arg:        [:arg1 {needs} :arg2 {as ^^META for} :arg3 {isotope}]
     no-arg:             [:arg1 {is missing its} :arg2 {argument}]
+
+    ; These need to have the same arguments (shared code coerces them)
+    ;
     expect-arg:         [:arg1 {expects} :arg2 {for its} :arg3 {argument}]
+    phase-expect-arg:
+        [:arg1 {internal phase expects} :arg2 {for its} :arg3 {argument}]
 
     invalid-type:       [:arg1 {type is not allowed here}]
     invalid-op:         [{invalid operator:} :arg1]

src/core/c-error.c

@@ -912,7 +912,7 @@ Context(*) Error_Not_Varargs(
     );
     UNUSED(honest_param);  // !!! pass to Error_Arg_Type(?)
 
-    return Error_Arg_Type(f, key, param, arg);
+    return Error_Phase_Arg_Type(f, key, param, arg);
 }
 
 
@@ -1104,11 +1104,15 @@ Context(*) Error_Unexpected_Type(enum Reb_Kind expected, enum Reb_Kind actual)
 //
 //  Error_Arg_Type: C
 //
-// Function in frame of `call` expected parameter `param` to be
-// a type different than the arg given (which had `arg_type`)
+// Function in frame of `call` expected parameter `param` to be a type
+// different than the arg given.
+//
+// !!! Right now, we do not include the arg itself in the error.  It would
+// potentially lead to some big molding, and the error machinery isn't
+// really equipped to handle it.
 //
 Context(*) Error_Arg_Type(
-    option(Frame(*)) f,
+    option(Symbol(const*)) name,
     const REBKEY *key,
     const REBPAR *param,
     const REBVAL *arg
@@ -1120,8 +1124,8 @@ Context(*) Error_Arg_Type(
     Init_Word(param_word, KEY_SYMBOL(key));
 
     DECLARE_LOCAL (label);
-    if (f)
-        Get_Frame_Label_Or_Nulled(label, unwrap(f));
+    if (name)
+        Init_Word(label, unwrap(name));
     else
         Init_Nulled(label);
 
@@ -1132,20 +1136,6 @@ Context(*) Error_Arg_Type(
     else
         Init_Block(spec, EMPTY_ARRAY);
 
-    if (f and FRM_PHASE(unwrap(f)) != unwrap(f)->u.action.original) {
-        //
-        // When RESKIN has been used, or if an ADAPT messes up a type and
-        // it isn't allowed by an inner phase, then it causes an error.  But
-        // it's confusing to say that the original function didn't take that
-        // type--it was on its interface.  A different message is needed.
-        //
-        return Error_Phase_Bad_Arg_Type_Raw(
-            label,
-            spec,
-            param_word
-        );
-    }
-
     return Error_Expect_Arg_Raw(
         label,
         spec,
@@ -1154,6 +1144,36 @@ Context(*) Error_Arg_Type(
 }
 
 
+//
+//  Error_Phase_Arg_Type: C
+//
+// When RESKIN has been used, or if an ADAPT messes up a type and it isn't
+// allowed by an inner phase, then it causes an error.  But it's confusing to
+// say that the original function didn't take that type--it was on its
+// interface.  A different message is helpful, so this does that by coercing
+// the ordinary error into one making it clear it's an internal phase.
+//
+Context(*) Error_Phase_Arg_Type(
+    Frame(*) f,
+    const REBKEY *key,
+    const REBPAR *param,
+    const REBVAL *arg
+){
+    if (FRM_PHASE(f) == f->u.action.original)  // not an internal phase
+        return Error_Arg_Type(f->label, key, param, arg);
+
+    if (VAL_PARAM_CLASS(param) == PARAM_CLASS_META and Is_Meta_Of_Raised(arg))
+        return VAL_CONTEXT(arg);
+
+    Context(*) error = Error_Arg_Type(f->label, key, param, arg);
+    ERROR_VARS* vars = ERR_VARS(error);
+    assert(IS_WORD(&vars->id));
+    assert(VAL_WORD_ID(&vars->id) == SYM_EXPECT_ARG);
+    Init_Word(&vars->id, Canon(PHASE_EXPECT_ARG));
+    return error;
+}
+
+
 //
 //  Error_Bad_Argless_Refine: C
 //

src/core/evaluator/c-action.c

@@ -157,7 +157,7 @@ Bounce Proxy_Multi_Returns_Core(Frame(*) f, Value(*) v)
             continue;
 
         if (not Typecheck_Coerce_Argument(PARAM, ARG))
-            fail (Error_Arg_Type(f, KEY, PARAM, ARG));
+            fail (Error_Phase_Arg_Type(f, KEY, PARAM, ARG));
 
         Meta_Quotify(Copy_Cell(PUSH(), ARG));
     }
@@ -868,7 +868,7 @@ Bounce Action_Executor(Frame(*) f)
         }
 
         if (not Typecheck_Coerce_Argument(PARAM, ARG))
-            fail (Error_Arg_Type(f, KEY, PARAM, ARG));
+            fail (Error_Phase_Arg_Type(f, KEY, PARAM, ARG));
     }
 
   // Action arguments now gathered, begin dispatching

src/core/functionals/c-combinator.c

@@ -783,6 +783,7 @@ DECLARE_NATIVE(combinatorize)
     INCLUDE_PARAMS_OF_COMBINATORIZE;
 
     Action(*) act = VAL_ACTION(ARG(c));
+    option(Symbol(const*)) label = VAL_ACTION_LABEL(ARG(c));
 
     // !!! The hack for PATH! handling was added to make /ONLY work; it only
     // works for refinements with no arguments by looking at what's in the path
@@ -807,7 +808,7 @@ DECLARE_NATIVE(combinatorize)
     //
     Copy_Cell(ARG(advanced), ARG(rules));
 
-    Action(*) parser = Make_Action_From_Exemplar(s.ctx);
+    Action(*) parser = Make_Action_From_Exemplar(s.ctx, label);
     DROP_GC_GUARD(s.ctx);
 
     Activatify(Init_Action(  // note: MAKE ACTION! copies the context, this won't

src/core/functionals/c-does.c

@@ -219,8 +219,8 @@ DECLARE_NATIVE(does)
     assert(KEY_SYM(CTX_KEY(exemplar, 1)) == SYM_RETURN);
     Copy_Cell(CTX_VAR(exemplar, 2), source);
 
-    Symbol(const*) label = ANONYMOUS;  // !!! Better answer?
+    Symbol(const*) label = Canon(DO);  // !!! Better answer?
 
-    Action(*) doer = Make_Action_From_Exemplar(exemplar);
+    Action(*) doer = Make_Action_From_Exemplar(exemplar, label);
     return Init_Activation(OUT, doer, label, UNBOUND);
 }

src/core/functionals/c-reframer.c

@@ -463,6 +463,7 @@ DECLARE_NATIVE(reframer_p)
     Manage_Series(CTX_VARLIST(exemplar));
     Action(*) reframer = Alloc_Action_From_Exemplar(
         exemplar,  // shim minus the frame argument
+        label,
         &Reframer_Dispatcher,
         IDX_REFRAMER_MAX  // details array capacity => [shim, param_index]
     );

src/core/functionals/c-specialize.c

@@ -339,8 +339,10 @@ bool Specialize_Action_Throws(
         if (GET_PARAM_FLAG(param, VARIADIC))
             fail ("Cannot currently SPECIALIZE variadic arguments.");
 
-        if (not Typecheck_Coerce_Argument(param, arg))
-            fail (Error_Arg_Type(nullptr, key, param, arg));
+        if (not Typecheck_Coerce_Argument(param, arg)) {
+            option(Symbol(const*)) label = VAL_ACTION_LABEL(specializee);
+            fail (Error_Arg_Type(label, key, param, arg));
+        }
     }
 
     // Everything should have balanced out for a valid specialization.
@@ -690,6 +692,7 @@ REBVAL *First_Unspecialized_Arg(option(const REBPAR **) param_out, Frame(*) f)
 //
 Action(*) Alloc_Action_From_Exemplar(
     Context(*) exemplar,
+    option(Symbol(const*)) label,
     Dispatcher* dispatcher,
     REBLEN details_capacity
 ){
@@ -717,7 +720,7 @@ Action(*) Alloc_Action_From_Exemplar(
         }
 
         if (not Typecheck_Coerce_Argument(param, arg))
-            fail (Error_Arg_Type(nullptr, key, param, arg));
+            fail (Error_Arg_Type(label, key, param, arg));
     }
 
     // This code parallels Specialize_Action_Throws(), see comments there
@@ -738,10 +741,13 @@ Action(*) Alloc_Action_From_Exemplar(
 //
 // Assumes you want a Specializer_Dispatcher with the exemplar in details.
 //
-Action(*) Make_Action_From_Exemplar(Context(*) exemplar)
-{
+Action(*) Make_Action_From_Exemplar(
+    Context(*) exemplar,
+    option(Symbol(const*)) label
+){
     Action(*) action = Alloc_Action_From_Exemplar(
         exemplar,
+        label,
         &Specializer_Dispatcher,
         IDX_SPECIALIZER_MAX  // details capacity
     );

src/core/t-function.c

@@ -115,10 +115,11 @@ Bounce MAKE_Action(
         Context(*) exemplar = VAL_CONTEXT(frame_copy);
         rebRelease(frame_copy);
 
+        option(Symbol(const*)) label = VAL_FRAME_LABEL(arg);
         return Init_Action(
             OUT,
-            Make_Action_From_Exemplar(exemplar),
-            VAL_FRAME_LABEL(arg),
+            Make_Action_From_Exemplar(exemplar, label),
+            label,
             VAL_FRAME_BINDING(arg)
         );
     }

src/core/t-varargs.c

@@ -347,7 +347,7 @@ bool Do_Vararg_Op_Maybe_End_Throws_Core(
             if (not vararg_frame)
                 fail (out);
 
-            fail (Error_Arg_Type(unwrap(vararg_frame), key, param, out));
+            fail (Error_Phase_Arg_Type(unwrap(vararg_frame), key, param, out));
         }
     }