Simplify refinement and return typechecking

When refinements were adapted to be special cases of paths,
and characters as special cases of issues, this introduced
the need to be able to check not just a fundamental type but
a "type constraint".  This was done very hackily for just
those examples initially, but the system is now migrating
to where arbitrary functions can be used in type blocks as
constraints.

The messy evolution hadn't really been cleaned up (including
a detour into the type constraint of tuples with leading
blanks as being "predicates").  There was also separate type
checking for refinements which hadn't been folded in fully
when refinements became "their own arguments", that had some
code in specialization that was more complex than it needed
to be.

This takes a step toward a bit more clarity, deleting unneeded
bits and fusing the refinement type checking with the parameter
checking.  Also separates out the return type checking into
its own function for clarity.

extensions/javascript/mod-javascript.c

@@ -754,7 +754,9 @@ Bounce JavaScript_Dispatcher(Frame(*) frame_)
 
 } handle_resolved: { ////////////////////////////////////////////////////////
 
- FAIL_IF_BAD_RETURN_TYPE(f);
+ if (not Typecheck_Return(f, param, OUT))
+ fail (Error_Bad_Return_Type(f, OUT));
+
  return OUT;
 
 } handle_rejected: { ////////////////////////////////////////////////////////

src/core/c-error.c

@@ -1120,7 +1120,7 @@ Context(*) Error_Unexpected_Type(enum Reb_Kind expected, enum Reb_Kind actual)
 // a type different than the arg given (which had `arg_type`)
 //
 Context(*) Error_Arg_Type(
- Frame(*) f,
+ option(Frame(*)) f,
  const REBKEY *key,
  const REBPAR *param,
  const REBVAL *arg
@@ -1129,7 +1129,10 @@ Context(*) Error_Arg_Type(
  Init_Word(param_word, KEY_SYMBOL(key));
 
  DECLARE_LOCAL (label);
- Get_Frame_Label_Or_Nulled(label, f);
+ if (f)
+ Get_Frame_Label_Or_Nulled(label, unwrap(f));
+ else
+ Init_Nulled(label);
 
  DECLARE_LOCAL (spec);
  option(Array(const*)) param_array = VAL_PARAMETER_ARRAY(param);
@@ -1138,7 +1141,7 @@ Context(*) Error_Arg_Type(
  else
  Init_Block(spec, EMPTY_ARRAY);
 
- if (FRM_PHASE(f) != f->u.action.original) {
+ if (f and FRM_PHASE(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

src/core/d-eval.c

@@ -237,11 +237,7 @@ void Do_After_Action_Checks_Debug(Frame(*) f) {
  Action(*) phase = FRM_PHASE(f);
 
  if (ACT_HAS_RETURN(phase)) {
- const REBKEY *key = ACT_KEYS_HEAD(phase);
- const REBPAR *param = ACT_PARAMS_HEAD(phase);
- assert(KEY_SYM(key) == SYM_RETURN);
-
- if (not Typecheck_Including_Constraints(param, f->out)) {
+ if (not Typecheck_Return(f, f->out)) {
  assert(!"Native code violated return type contract!\n");
  panic (Error_Bad_Return_Type(f, f->out));
  }

src/core/evaluator/c-action.c

@@ -156,7 +156,7 @@ Bounce Proxy_Multi_Returns_Core(Frame(*) f, Value(*) v)
  if (VAL_PARAM_CLASS(PARAM) != PARAM_CLASS_OUTPUT)
  continue;
 
- if (not TYPE_CHECK(PARAM, ARG))
+ if (not Typecheck_Parameter(PARAM, ARG))
  fail (Error_Arg_Type(f, KEY, PARAM, ARG));
 
  Meta_Quotify(Copy_Cell(PUSH(), ARG));
@@ -903,35 +903,15 @@ Bounce Action_Executor(Frame(*) f)
  continue;
  }
 
- if (
- GET_PARAM_FLAG(PARAM, REFINEMENT)
- or GET_PARAM_FLAG(PARAM, SKIPPABLE)
- ){
- Typecheck_Refinement(KEY, PARAM, ARG); // extra check of # or NULL
- continue;
- }
-
- if (PARAM_CLASS_META == VAL_PARAM_CLASS(PARAM)) {
- //
- // !!! Now that everything is isotopic, there needs to be a new
- // policy on checking here.
- //
- }
-
  if (GET_PARAM_FLAG(PARAM, CONST))
  Set_Cell_Flag(ARG, CONST); // mutability override? see [5]
 
- if (GET_PARAM_FLAG(PARAM, REFINEMENT)) {
- Typecheck_Refinement(KEY, PARAM, ARG);
- continue; // !!! Review when # is used here
- }
-
  if (KEY_SYM(KEY) == SYM_RETURN)
  continue; // !!! let whatever go for now
 
  typecheck_again:
 
- if (not Typecheck_Including_Constraints(PARAM, ARG)) {
+ if (not Typecheck_Parameter(PARAM, ARG)) {
  if (Is_Activation(ARG)) {
  Deactivate_If_Activation(ARG);
  goto typecheck_again;

src/core/functionals/c-reframer.c

@@ -399,7 +399,14 @@ DECLARE_NATIVE(reframer_p)
  // Make sure the parameter is able to accept FRAME! arguments (the type
  // checking will ultimately use the same slot we overwrite here!)
  //
-/* if (not TYPE_CHECK(param, REB_FRAME)) {
+ // !!! This checks to see if it accepts *an* instance of a frame, but it's
+ // not narrow enough because there might be some additional check on the
+ // properties of the frame. This is a limit of the type constraints
+ // needing an instance of the type to check. It may suggest that we
+ // shouldn't do this at all, and just let it fail when called. :-/
+ //
+ Copy_Cell(SPARE, FRAME->rootvar);
+ if (not Typecheck_Parameter(param, SPARE)) {
  DECLARE_LOCAL (label_word);
  if (label)
  Init_Word(label_word, unwrap(label));
@@ -415,7 +422,7 @@ DECLARE_NATIVE(reframer_p)
  param_word
  );
  goto cleanup_binder;
- } */
+ }
  }
 
  cleanup_binder: {

src/core/functionals/c-specialize.c

@@ -322,61 +322,25 @@ bool Specialize_Action_Throws(
  if (Is_Specialized(param))
  continue;
 
- if (GET_PARAM_FLAG(param, REFINEMENT)) {
- if (Is_None(arg)) {
- //
- // Undefined refinements not explicitly marked hidden are
- // still candidates for usage at the callsite.
-
- goto unspecialized_arg; // ran out...no pre-empt needed
- }
-
- Typecheck_Refinement(key, param, arg);
- goto specialized_arg_no_typecheck;
- }
-
- // It's an argument, either a normal one or a refinement arg.
-
- if (Is_None(arg))
- goto unspecialized_arg;
-
- goto specialized_arg_with_check;
-
- unspecialized_arg:
-
- assert(Is_None(arg));
  assert(IS_PARAMETER(param));
- Copy_Cell(arg, param);
- continue;
 
- specialized_arg_with_check:
+ // You can't specialize with ~ isotopes ("none"), these indicate
+ // unspecialized arguments. Only ^META parameters can take meta-none
+ // (e.g. a plain quasi-void, or ~)
+ //
+ if (Is_None(arg)) { // unspecialized argument
+ Copy_Cell(arg, param);
+ continue;
+ }
 
  // !!! If argument was previously specialized, should have been type
  // checked already... don't type check again (?)
  //
  if (GET_PARAM_FLAG(param, VARIADIC))
  fail ("Cannot currently SPECIALIZE variadic arguments.");
 
- if (not Typecheck_Including_Constraints(param, arg))
- fail (arg); // !!! merge w/Error_Invalid_Arg()
-
- continue;
-
- specialized_arg_no_typecheck:
-
- // !!! Technically speaking, if you are trying to implement the ^META
- // parameter convention, SPECIALIZE does it too late here. The intent
- // of ~_~ isotope vs. NULL is lost...at least in theory, because
- // variables don't store the null isotope state. The "core" way to
- // do this is MAKE FRAME! (specialize could be written in usermode
- // with that). But as a higher-level tool, specialize can make the
- // tradeoff to make it easy by doing the meta-quoting for you...which
- // also means it can keep working if the parameter convention changes.
- //
- if (VAL_PARAM_CLASS(param) == PARAM_CLASS_META)
- Meta_Quotify(arg);
-
- continue;
+ if (not Typecheck_Parameter(param, arg))
+ fail (Error_Arg_Type(nullptr, key, param, arg));
  }
 
  // Everything should have balanced out for a valid specialization.
@@ -752,10 +716,8 @@ Action(*) Alloc_Action_From_Exemplar(
  continue;
  }
 
- if (GET_PARAM_FLAG(param, REFINEMENT))
- Typecheck_Refinement(key, param, arg);
- else
- Typecheck_Including_Constraints(param, arg);
+ if (not Typecheck_Parameter(param, arg))
+ fail (Error_Arg_Type(nullptr, key, param, arg));
  }
 
  // This code parallels Specialize_Action_Throws(), see comments there

src/core/functionals/c-typechecker.c

@@ -318,7 +318,7 @@ bool Typecheck_Value(
  if (VAL_PARAM_CLASS(param) == PARAM_CLASS_META)
  Meta_Quotify(arg);
 
- if (not TYPE_CHECK(param, arg)) {
+ if (not Typecheck_Parameter(param, arg)) {
  Drop_Action(f);
  if (match_all)
  return false;

src/core/functionals/n-function.c

@@ -620,12 +620,9 @@ DECLARE_NATIVE(definitional_return)
  // take [<opt> any-value!] as its argument, and then report the error
  // itself...implicating the frame (in a way parallel to this native).
  //
- if (GET_PARAM_FLAG(param, RETURN_NONE) and not Is_None(atom))
- fail ("If RETURN: <none> is in a function spec, RETURN NONE only");
-
- if (not TYPE_CHECK(param, atom)) {
+ if (not Typecheck_Return(target_frame, atom)) {
  Decay_If_Unstable(atom);
- if (not TYPE_CHECK(param, atom))
+ if (not Typecheck_Return(target_frame, atom))
  fail (Error_Bad_Return_Type(target_frame, atom));
  }
 

src/core/t-varargs.c

@@ -336,7 +336,7 @@ bool Do_Vararg_Op_Maybe_End_Throws_Core(
  Decay_If_Unstable(out);
 
  if (param) {
- if (not TYPE_CHECK(param, out)) {
+ if (not Typecheck_Parameter(param, out)) {
  //
  // !!! Array-based varargs only store the parameter list they are
  // stamped with, not the frame. This is because storing non-reified

src/include/datatypes/sys-frame.h

@@ -632,22 +632,6 @@ inline static REBVAL *D_ARG_Core(Frame(*) f, REBLEN n) { // 1 for first arg
  D_ARG_Core(frame_, (n))
 
 
-// Shared code for type checking the return result. It's used by the
-// Returner_Dispatcher(), but custom dispatchers use it to (e.g. JS-NATIVE)
-//
-inline static void FAIL_IF_BAD_RETURN_TYPE(Frame(*) f) {
- Action(*) phase = FRM_PHASE(f);
- const REBPAR *param = ACT_PARAMS_HEAD(phase);
- assert(KEY_SYM(ACT_KEYS_HEAD(phase)) == SYM_RETURN);
-
- // Typeset bits for locals in frames are usually ignored, but the RETURN:
- // local uses them for the return types of a function.
- //
- if (not Typecheck_Including_Constraints(param, f->out))
- fail (Error_Bad_Return_Type(f, f->out));
-}
-
-
 inline static bool Eval_Value_Core_Throws(
  REBVAL *out,
  Flags flags,
@@ -959,3 +943,26 @@ inline static Bounce Continue_Subframe_Helper(
 #define DELEGATE_SUBFRAME(sub) ( \
  Continue_Subframe_Helper(frame_, false, (sub)), \
  BOUNCE_DELEGATE)
+
+
+inline static bool Typecheck_Return(
+ Frame(*) f,
+ Value(*) atom // Typecheck_Parameter needs mutability (unused for returns)
+){
+ if (Is_Pack(atom))
+ return true; // For now, assume multi-return typechecked it
+ if (Is_Raised(atom))
+ return true; // For now, all functions return definitional errors
+
+ // Typeset bits for locals in frames are usually ignored, but the RETURN:
+ // local uses them for the return types of a function.
+ //
+ Action(*) phase = FRM_PHASE(f);
+ const REBPAR *param = ACT_PARAMS_HEAD(phase);
+ assert(KEY_SYM(ACT_KEYS_HEAD(phase)) == SYM_RETURN);
+
+ if (GET_PARAM_FLAG(param, RETURN_NONE) and not Is_None(atom))
+ fail ("If RETURN: <none> is in a function spec, RETURN NONE only");
+
+ return Typecheck_Parameter(param, atom);
+}

src/include/datatypes/sys-sequence.h

@@ -731,21 +731,6 @@ inline static bool IS_PREDICATE1_CELL(noquote(Cell(const*)) v) {
  return Get_Cell_Flag(v, REFINEMENT_LIKE); // !!! Review: test this first?
 }
 
-inline static Symbol(const*) VAL_PREDICATE1_SYMBOL(
- noquote(Cell(const*)) v
-){
- assert(IS_PREDICATE1_CELL(v));
- return SYM(VAL_NODE1(v));
-}
-
-inline static bool IS_PREDICATE(Cell(const*) v) {
- if (not IS_TUPLE(v))
- return false;
-
- DECLARE_LOCAL (temp);
- return IS_BLANK(VAL_SEQUENCE_AT(temp, v, 0));
-}
-
 inline static Symbol(const*) VAL_REFINEMENT_SYMBOL(
  noquote(Cell(const*)) v
 ){