Disallow testing for IS_TRASH in release builds

The idea of a "trash" cell is that debug builds can protect
against reading such cells before they are filled with
meaningful data.  But unlike an ordinary uninitialized cell,
these trash cells are ignored by the garbage collector...so
they can sit in arrays spanning evaluations.

There was a misguided idea that IS_TRASH() in a release build
was defined to false, based on the concept that no such cells
would exist.  This broke a case in the APPLY implementation
where the trash state was being used to carry meaning...because
in the release build initializing a cell to trash and then
asking if it was trash would return false.  The correct way
to avoid this problem is to not define IS_TRASH() in the
release build to catch such incorrect uses.

APPLY was changed to use a NONE state for the signal, as it
needed a signal that could be read as well as written.

src/core/m-gc.c

@@ -305,8 +305,10 @@ static void Queue_Mark_Cell_Deep(Cell(const*) cv)
 {
     Cell(*) v = m_cast(Cell(*), cv);  // we're the system, we can do this
 
-    if (IS_TRASH(cv))  // always false in release builds (no cost)
+  #if DEBUG_UNREADABLE_TRASH
+    if (IS_TRASH(cv))  // tolerate unreadable "trash" in debug builds
         return;
+  #endif
 
     // We mark based on the type of payload in the cell, e.g. its "unescaped"
     // form.  So if '''a fits in a WORD! (despite being a QUOTED!), we want

src/core/n-do.c

@@ -823,7 +823,7 @@ DECLARE_NATIVE(apply)
       case ST_APPLY_UNLABELED_EVAL_STEP :
         if (THROWING)
             goto finalize_apply;
-        if (IS_TRASH(iterator)) {
+        if (Is_None(iterator)) {
             assert(REF(relax));
             goto handle_next_item;
         }
@@ -907,7 +907,7 @@ DECLARE_NATIVE(apply)
 
         Init_Integer(ARG(index), index);
     }
-    else if (IS_TRASH(iterator)) {
+    else if (Is_None(iterator)) {
         STATE = ST_APPLY_UNLABELED_EVAL_STEP;
         param = nullptr;  // throw away result
     }
@@ -922,7 +922,7 @@ DECLARE_NATIVE(apply)
                     fail (Error_Apply_Too_Many_Raw());
 
                 FREE(EVARS, e);
-                Init_Trash(iterator);
+                Init_None(iterator);
                 param = nullptr;  // we're throwing away the evaluated product
                 break;
             }
@@ -993,13 +993,13 @@ DECLARE_NATIVE(apply)
 
 } finalize_apply: {  /////////////////////////////////////////////////////////
 
-    if (IS_TRASH(iterator))
+    if (Is_None(iterator))
         assert(REF(relax));
     else {
         EVARS *e = VAL_HANDLE_POINTER(EVARS, iterator);
         Shutdown_Evars(e);
         FREE(EVARS, e);
-        Init_Trash(iterator);
+        Init_None(iterator);
     }
 
     if (THROWING)  // assume Drop_Frame() called on SUBFRAME?

src/include/sys-trash.h

@@ -71,7 +71,7 @@
     #define Init_Trash_Untracked(out) \
         Init_Nothing_Untracked((out), REB_VOID, QUASI_2)
 
-    #define IS_TRASH(v) false  // should constant-fold out clauses in optimizer
+    #undef IS_TRASH  // testing for trash in release builds is not meaningful
 #endif
 
 #define Init_Trash(out) \

tests/parse/examples/red-replace.parse.test.reb

@@ -1,5 +1,5 @@
 ; %red-replace.parse.test.reb
-; 
+;
 ; Red's REPLACE has an invention that seems like a pretty bad idea.  It mixes
 ; PARSE mechanisms in with REPLACE, but it only works if the source data is
 ; a string...because it "knows" not to treat the block string-like.  This is
@@ -17,9 +17,9 @@
         ]
         rule: if activation? :f.replacement '[
             head: <here>
-            change [f.pattern, tail: <here>] ( 
+            change [f.pattern, tail: <here>] (
                 apply/relax :f.replacement [const head, const tail]
-            ) 
+            )
         ] else '[
             change f.pattern (f.replacement)
         ]
@@ -30,7 +30,7 @@
             to <end>
         ]]
         return f.target
-    ], true) 
+    ], true)
 
     ; These are the tests Red had demonstrating the feature