Switch to indefinite lifetime for function args/locals

Ren-C brought in a "reasonable cost" model for "specific binding",
which is the idea that WORD!s bound in the body of functions retain
a memory to which instantiation of that function they are for.  (This
replaced a technique of walking the stack to presume a word should
always be looked up in the most recent invocation of a function.)

However, when functions would finish the Drop_Action() call they'd mark
the frame of that specific instance as expired.  This was done due
to a belief that it would be expensive if an object instance (with
as many cells as a function had parameters and locals) would be kept
alive on every usermode function call after the call ended.  The
result would be an increased tax on the GC, creating one object per
(usermode) function call.

Desire to avoid that inefficiency ran up against the practical desire
of the semantics provided by what R3-Alpha called "CLOSURE".  A closure
would allow words to have a binding that survived the end of a
function call:

    r3-alpha>> foo: function [x] [return [x]]
    r3-alpha>> data: foo 10
    == [x]
    r3-alpha>> reduce data
    ** Script error: x word is not bound to a context

    r3-alpha>> foo: closure [x] [return [x]]
    r3-alpha>> data: foo 10
    == [x]
    r3-alpha>> reduce data
    == [10]

All things being equal, this behavior is usually the more desirable
one.  Especially when coding in an environment where functions must
frequently remove themselves from the stack and provide callbacks,
it helps if those callbacks can retain state from the environment
in which they were constructed.  As a canonical example:

    foo: function [x] [
        let y: 10
        return function [z] [x + y + z]
    ]

The generated function which is returned will be useless if x and y
are not still live after FOO has returned.

When considering the big picture--especially when trying to convince
JavaScript users that Rebol is a good language--it does not make sense
to make them need to learn a distinct word or concept to get this
feature.  Instead, the pressure should be on the system to optimize
and recycle frames that did not leak word references in this way.  Or
on those seeking optimization to use a special form of function that
does not permit this due to frame expiration.

This is a "future bet" on what the long tail of the language needs to
pick as its defaults.

src/core/m-gc.c

@@ -508,30 +508,31 @@ static void Mark_Root_Series(void)
                 assert(not IS_SER_DYNAMIC(s));
                 assert(
                     not LINK(s).owner
-                    or LINK(s).owner->header.bits & NODE_FLAG_MANAGED
+                    or (LINK(s).owner->header.bits & NODE_FLAG_MANAGED)
                 );
 
                 if (not (s->header.bits & NODE_FLAG_MANAGED)) {
                     assert(not LINK(s).owner);
                 }
-                else if (GET_SERIES_INFO(LINK(s).owner, INACCESSIBLE)) {
-                    if (NOT_SERIES_FLAG(LINK(s).owner, VARLIST_FRAME_FAILED)) {
-                        //
-                        // Long term, it is likely that implicit managed-ness
-                        // will allow users to leak API handles.  It will
-                        // always be more efficient to not do that, so having
-                        // the code be strict for now is better.
-                        //
-                      #if !defined(NDEBUG)
-                        printf("handle not rebReleased(), not legal ATM\n");
-                      #endif
-                        panic (s);
-                    }
-
-                    GC_Kill_Series(s);
+                else if (
+                    GET_SERIES_FLAG(LINK(s).owner, VARLIST_FRAME_FAILED)
+                ){
+                    GC_Kill_Series(s);  // auto-free API handles on failure
                     continue;
                 }
-                else // note that Mark_Frame_Stack_Deep() will mark the owner
+                else if (not Is_Frame_On_Stack(CTX(LINK(s).owner))) {
+                    //
+                    // Long term, it is likely that implicit managed-ness
+                    // will allow users to leak API handles.  It will
+                    // always be more efficient to not do that, so having
+                    // the code be strict for now is better.
+                    //
+                  #if !defined(NDEBUG)
+                    printf("handle not rebReleased(), not legal ATM\n");
+                  #endif
+                    panic (s);
+                }
+                else  // Note that Mark_Frame_Stack_Deep() will mark the owner
                     s->header.bits |= NODE_FLAG_MARKED;
 
                 // Note: Eval_Core() might target API cells, uses END

src/include/datatypes/sys-context.h

@@ -189,6 +189,11 @@ static inline void INIT_CTX_KEYLIST_UNIQUE(REBCTX *c, REBARR *keylist) {
 #define CTX_KEYS_HEAD(c) \
     SER_AT(REBVAL, SER(CTX_KEYLIST(c)), 1) // a CTX_KEY can't hold a RELVAL
 
+inline static bool Is_Frame_On_Stack(REBCTX *c) {
+    assert(IS_FRAME(CTX_ARCHETYPE(c)));
+    return (LINK_KEYSOURCE(c)->header.bits & NODE_FLAG_CELL);
+}
+
 inline static REBFRM *CTX_FRAME_IF_ON_STACK(REBCTX *c) {
     REBNOD *keysource = LINK_KEYSOURCE(c);
     if (not (keysource->header.bits & NODE_FLAG_CELL))

src/include/datatypes/sys-frame.h

@@ -363,7 +363,7 @@ inline static void UPDATE_EXPRESSION_START(REBFRM *f) {
 
 inline static void Abort_Frame(REBFRM *f) {
     if (f->varlist and NOT_SERIES_FLAG(f->varlist, MANAGED))
-        GC_Kill_Series(SER(f->varlist)); // not alloc'd with manuals tracking
+        GC_Kill_Series(SER(f->varlist));  // not alloc'd with manuals tracking
     TRASH_POINTER_IF_DEBUG(f->varlist);
 
     // Abort_Frame() handles any work that wouldn't be done done naturally by
@@ -409,7 +409,7 @@ inline static void Abort_Frame(REBFRM *f) {
         }
     }
 
-pop:;
+  pop:
 
     assert(TG_Top_Frame == f);
     TG_Top_Frame = f->prior;
@@ -702,17 +702,29 @@ inline static void Drop_Action(REBFRM *f) {
     }
     else if (GET_SERIES_FLAG(f->varlist, MANAGED)) {
         //
-        // The varlist wound up getting referenced in a cell that will outlive
-        // this Drop_Action().  The pointer needed to stay working up until
-        // now, but the args memory won't be available.  But since we know
-        // there were outstanding references to the varlist, we need to
-        // convert it into a "stub" that's enough to avoid crashes.
+        // Varlist wound up getting referenced in a cell that will outlive
+        // this Drop_Action().
+        //
+        // !!! The new concept is to let frames survive indefinitely in this
+        // case.  This is in order to not let JavaScript have the upper hand
+        // in "closure"-like scenarios.  See:
+        //
+        // "What Happens To Function Args/Locals When The Call Ends"
+        // https://forum.rebol.info/t/234
+        //
+        // Previously this said:
+        //
+        // "The pointer needed to stay working up until now, but the args
+        // memory won't be available.  But since we know there are outstanding
+        // references to the varlist, we need to convert it into a "stub"
+        // that's enough to avoid crashes.
         //
         // ...but we don't free the memory for the args, we just hide it from
         // the stub and get it ready for potential reuse by the next action
         // call.  That's done by making an adjusted copy of the stub, which
-        // steals its dynamic memory (by setting the stub not HAS_DYNAMIC).
+        // steals its dynamic memory (by setting the stub not HAS_DYNAMIC)."
         //
+      #if 0
         f->varlist = CTX_VARLIST(
             Steal_Context_Vars(
                 CTX(f->varlist),
@@ -721,6 +733,10 @@ inline static void Drop_Action(REBFRM *f) {
         );
         assert(NOT_SERIES_FLAG(f->varlist, MANAGED));
         INIT_LINK_KEYSOURCE(f->varlist, NOD(f));
+      #endif
+
+        INIT_LINK_KEYSOURCE(f->varlist, NOD(f->original));
+        f->varlist = nullptr;
     }
     else {
         // We can reuse the varlist and its data allocation, which may be

src/include/datatypes/sys-string.h

@@ -560,7 +560,10 @@ inline static REBCHR(*) STR_AT(REBSTR *s, REBLEN at) {
     else {
         if (len < sizeof(REBVAL)) {
             if (not IS_STR_SYMBOL(s))
-                assert(not bookmark);  // mutations must ensure this
+                assert(
+                    not bookmark  // mutations must ensure this usually but...
+                    or GET_SERIES_FLAG(s, ALWAYS_DYNAMIC)  // !!! mold buffer?
+                );
             goto scan_from_tail;  // good locality, avoid bookmark logic
         }
         if (not bookmark and not IS_STR_SYMBOL(s)) {

tests/context/bind.test.reb

@@ -48,6 +48,5 @@
 
 [#1893 (
     word: reeval func [x] ['x] 1
-    e: trap [same? word bind 'x word]
-    e/id = 'expired-frame
+    same? word bind 'x word
 )]

tests/context/valueq.test.reb

@@ -1,5 +1,7 @@
 ; functions/context/valueq.r
 (false == set? 'nonsense)
 (true == set? 'set?)
-; #1914 ... Ren-C indefinite extent prioritizes failure if not indefinite
-(error? trap [set? reeval func [x] ['x] blank])
+
+[#1914 (
+    set? reeval func [x] ['x] blank
+)]

tests/datatypes/function.test.reb

@@ -331,15 +331,24 @@
     f: does (reduce [does [true]])
     f
 )]
-; no-rebind test--succeeds in R3-Alpha but fails in Ren-C.  Second time f is
-; called, `a` has been cleared so `a [d]` doesn't recapture the local, and
-; `c` holds the `[d]` from the first call.
-(
-    a: func [b] [a: _ c: b]
-    f: func [d] [a [d] do c]
+
+; Second time f is called, `a` has been cleared so `a [d]` doesn't recapture
+; the local, and `c` holds the `[d]` from the first call.  This succeeds in
+; R3-Alpha for a different reason than it succeeds in Ren-C; Ren-C has
+; closure semantics for functions so the c: [d] where d is 1 survives.
+; R3-Alpha recycles variables based on stack searching (non-specific binding).
+(
+    a: func [b] [
+        a: _  comment "erases a so only first call saves c"
+        c: b
+    ]
+    f: func [d] [
+        a [d]
+        do c
+    ]
     did all [
         1 = f 1
-        error? trap [2 = f 2]
+        1 = f 2
     ]
 )
 [#1528