Rationalize certain edge cases for function execution

This concerns edge cases when executing a function. Historically, when we parse
fish script, we identify early whether or not it's a function; but only record
the function's name and not its properties (i.e. not its source).

This means that the function can change between parsing and execution. Example:

    function foo; echo alpha; end
    foo (function foo; echo beta; end)

This has historically output "beta" because the function is replaced as part of
its own arguments.

Worse is if the function is deleted:

    function foo; echo alpha; end
    foo (functions --erase foo)

This outputs an error but in an awkward place; that's OK since it's very rare.

Let's codify this behavior since someone might be depending on it.

src/exec.rs

@@ -965,11 +965,15 @@ fn function_restore_environment(parser: &Parser, block: BlockId) {
     Option<&mut OutputStream>,
 ) -> ProcStatus;
 
-// Return a function which may be to run the given process \p.
-// May return an empty std::function in the rare case that the to-be called fish function no longer
-// exists. This is just a dumb artifact of the fact that we only capture the functions name, not its
+// Return a function which may be to run the given process 'p'.
+// May return None in the rare case that the to-be called fish function no longer
+// exists. This is just an artifact of the fact that we only capture the functions name, not its
 // properties, when creating the job; thus a race could delete the function before we fetch its
-// properties.
+// properties. Note this is user visible. An example:
+//
+//    function foo; echo hi; end
+//    foo (functions --erase foo)
+//
 fn get_performer_for_process(
     p: &Process,
     job: &Job,

src/proc.rs

@@ -51,9 +51,12 @@ pub enum ProcessType {
     /// A regular external command.
     #[default]
     External,
-    /// A builtin command.
+    /// A builtin command. The builtin name is stored in `argv[0]`.
     Builtin,
     /// A shellscript function.
+    /// The function name is stored in `argv[0]`.
+    /// Note we don't capture the function body here, because the
+    /// function body may change as part of argument expansion.
     Function,
     /// A block of commands, represented as a node.
     /// This is always either block, ifs, or switchs, never boolean or decorated.

tests/checks/function.fish

@@ -174,4 +174,16 @@ functions -q foo
 echo exists $status
 # CHECK: exists 1
 
+# If a function is changed as part of its own arguments, then we see the change.
+# This codifies historic behavior.
+function foo; echo before; end
+foo (function foo; echo after; end)
+# CHECK: after
+
+# If a function is deleted as part of its own arguments, then we see the change.
+# This codifies historic behavior.
+function foo; echo before; end
+foo (functions --erase foo)
+# CHECKERR: error: Unknown function 'foo'
+
 exit 0