Keep where each environment's process is: no freeze on a chain of functions called in $( ) (refactor 3.2)

CHANGELOG.md

@@ -43,6 +43,7 @@
 - The "Idiom" notes are read from the commands, not from the text. They are found with other spacing, order and quoting: `while  true ; do` is an endless loop, `done <f` ends a `while read` loop over a file, and `IFS= read -d '' -r`, `$(cd "$(dirname "$0")"; pwd)`, `command -v jq 2>/dev/null >/dev/null`, `exec >> >(tee -a log)`, `$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )`, `while [ ${#} -gt 0 ]`, `while (( $# != 0 ))`, `trap cleanup 0` and `[[ $BASH_SOURCE != $0 ]] && return` are known. They are no longer given where bash does something else: `trap - EXIT`, `trap '' EXIT` and `trap -p EXIT` set no clean-up; `read -rd'' f` reads up to an `f`, not a NUL byte; `while [ $# > 0 ]` writes to a file named `0` and does not end; and text in quotes (`echo '$(mktemp)'`) is not run. Before, each of these got the idiom's note.
 - A script is explained in POSIX mode only when its shebang starts bash in it (`#!/bin/bash --posix`, or `#!/usr/bin/env -S` with `bash --posix`, `bash -o posix` or `POSIXLY_CORRECT=1 bash`), and `POSIXLY_CORRECT` is then set, as bash sets it. Before, `--posix` anywhere on the shebang line counted, also in `#!/bin/bash --posix -e` and `#!/usr/bin/env bash --posix`, where Linux passes the rest of the line as one word and bash does not start.
 - A script that assigns an element before a function or `eval` (`n[0]=5 f`) is explained: bash refuses the assignment ("not a valid identifier") and runs the command. The same assignment before a special builtin in POSIX mode (`set -o posix; n[0]=5 :`) no longer fails either. Before, explaining such a script failed.
+- A script with a long chain of functions, each calling the one before in `$( )` and the one before that plainly (`f3() { local a=$(f2); f1; }`), is explained at once. Before, 40 such functions froze the explanation for about a minute.
 
 [All changes](https://github.com/sf624/bash-explainer/compare/v0.7.0...main)
 

src/semantics/env.ts

@@ -245,22 +245,28 @@ export const within = (env: ExecEnv, link: EnvLink): ExecEnv => ({ links: [...en
 
 /** The kind of process: the main shell, a subshell of it, another shell, or a program started by a slot. */
 export type ProcessKind = 'main' | 'subshell' | 'shell' | 'program';
+type Process = { readonly kind: ProcessKind; readonly sure: boolean; readonly link?: EnvLink };
+
+/** `processOf` of each environment, and `callSites` of each call link (both are never changed once made). */
+const processes = new WeakMap<ExecEnv, Process>();
+const callProcesses = new WeakMap<CallLink, { some: boolean; all: boolean }>();
 
 /**
  * The process that runs code in `env`: the innermost link that starts one (a `current-shell` slot starts none).
  * `sure: false` when it is a subshell on some paths only, and the main shell on the others; `link` is the link
  * that starts it.
  */
-export function processOf(env: ExecEnv): { kind: ProcessKind; sure: boolean; link?: EnvLink } {
-  let out: { kind: ProcessKind; sure: boolean; link?: EnvLink } = { kind: 'main', sure: true };
+export function processOf(env: ExecEnv): Process {
+  const hit = processes.get(env);
+  if (hit) return hit;
+  let out: Process = { kind: 'main', sure: true };
   for (const l of env.links) {
     if (l.kind === 'call') {
       // (A function is called from the script's own shells: none is in another program.)
       if (out.kind === 'subshell' && out.sure) continue;
-      const all = l.sites.map((x) => processOf(x.env));
-      const sub = all.filter((p) => p.kind === 'subshell');
-      out = !sub.length ? { kind: 'main', sure: out.sure && !l.unsure }
-        : { kind: 'subshell', sure: sub.length === all.length && sub.every((p) => p.sure) && !l.unsure, link: l };
+      const calls = callSites(l);
+      out = !calls.some ? { kind: 'main', sure: out.sure && !l.unsure }
+        : { kind: 'subshell', sure: calls.all && !l.unsure, link: l };
     } else if (l.kind === 'slot') {
       if (l.slot.run === 'subshell') out = { kind: 'subshell', sure: true, link: l };
       else if (l.slot.run !== 'current-shell') out = { kind: l.slot.run === 'exec' ? 'program' : 'shell', sure: true, link: l };
@@ -269,9 +275,26 @@ export function processOf(env: ExecEnv): { kind: ProcessKind; sure: boolean; lin
       out = { kind: 'subshell', sure: l.sure || (out.kind === 'subshell' && out.sure), link: l };
     }
   }
+  processes.set(env, out);
   return out;
 }
 
+/**
+ * The calls of a function: whether some run in a subshell, and whether all surely do. Kept per link, as
+ * `processOf` is per environment: the chains of a function's calls hold those of its callers' calls, which would be
+ * read again at each (a chain of functions, each called in `$( )` and also plainly, doubles them at each step).
+ */
+function callSites(l: CallLink): { some: boolean; all: boolean } {
+  let hit = callProcesses.get(l);
+  if (!hit) {
+    const all = l.sites.map((x) => processOf(x.env));
+    const sub = all.filter((p) => p.kind === 'subshell');
+    hit = { some: sub.length > 0, all: sub.length === all.length && sub.every((p) => p.sure) };
+    callProcesses.set(l, hit);
+  }
+  return hit;
+}
+
 /** Where the code runs: on this machine, on a remote one (ssh), or in a container. */
 export function whereOf(env: ExecEnv): 'local' | 'remote' | 'container' {
   let where: 'local' | 'remote' | 'container' = 'local';

test/env.test.ts

@@ -311,4 +311,29 @@ describe('explanations from the execution environment', () => {
     assert.match(about('[ "$1" ] && shopt -s lastpipe\necho | { a; }', '{ ‹commands› }', 'en', 1), /the last one runs in the shell itself on the paths where `shopt -s lastpipe` is on/);
     assert.match(about('( { a; } )', '{ ‹commands› }'), /in the current shell/);
   });
+  it('explains a long chain of functions, each called in `$( )` and plainly, in little time', function () {
+    // (Where each function runs was read again for each of its callers' calls, which doubled at each function: 40 of
+    // them took about a minute.)
+    this.timeout(60000);
+    const bodies: Record<string, (a: number, b: number) => string> = {
+      plain: (a, b) => `local a=$(f${a}); f${b};`,
+      'set -e': (a, b) => `local a=$(f${a}); f${b};`,
+      if: (a, b) => `if x=$(f${a}); then f${b}; fi;`,
+      '||': (a, b) => `a=$(f${a}) || f${b};`,
+      '&&': (a, b) => `f${b} && echo "$(f${a})";`,
+      '|': (a, b) => `f${a} | cat; f${b};`,
+    };
+    for (const [shape, body] of Object.entries(bodies)) {
+      const lines = [...(shape === 'set -e' ? ['set -e'] : []), 'f0() { cd /; }', 'f1() { f0; }'];
+      for (let i = 2; i < 40; i++) lines.push(`f${i}() { ${body(i - 1, i - 2)} }`);
+      lines.push('f39');
+      const tree = parser.parse(lines.join('\n'));
+      const analyzer = new Analyzer('en');
+      const t = Date.now();
+      lines.forEach((_, i) => analyzer.explainLine(tree, i));
+      assert.ok(Date.now() - t < 5000, `${shape}: ${Date.now() - t} ms`);
+      // (f0 runs in a subshell on some of its calls only.)
+      assert.strictEqual(kind(lines.join('\n'), 'cd /'), 'subshell?', shape);
+    }
+  });
 });

test/nested.test.ts

@@ -505,18 +505,21 @@ describe('nested scripts: the issues of a string another shell reads', () => {
     assert.deepStrictEqual(issues(`bash -c "eval 'sh -c \\"[[ a ]]\\"'"`).length, 1);
   });
 
-  it('takes a time linear in the strings, however many variables the script exports', () => {
+  it('takes a time linear in the strings, however many variables the script exports', function () {
+    this.timeout(60000);
     const time = (n: number, line: (i: number) => string) => {
       const src = Array.from({ length: n }, (_, i) => line(i)).join('\n');
       const tree = parser.parse(src);
       const t = Date.now();
       documentIssues({ lines: src.split('\n'), tree }, 'en', {});
       return Date.now() - t;
     };
+    // (Ten times the lines take about ten times as long, measured in the same run: a fixed time fails on a loaded
+    // machine. Quadratic, they would take a hundred times as long.)
     for (const line of [(i: number) => `export v${i}=1; bash -c 'echo $v${i}; x=2'`, (i: number) => `sh -c '[[ -n $x${i} ]] && echo ok'`]) {
       time(200, line);
-      const t = time(2000, line);
-      assert.ok(t < 1000, `${t} ms for 2000 lines: ${line(0)}`);
+      const [few, many] = [time(200, line), time(2000, line)];
+      assert.ok(many < 25 * few + 500, `${few} ms for 200 lines, ${many} ms for 2000: ${line(0)}`);
     }
   });
 });

test/state.test.ts

@@ -430,16 +430,21 @@ describe('shell state: more builtins', () => {
     ['on', 'off', 'unknown', 'unknown', 'on', 'off']);
   });
   it('is fast on long scripts with many functions', function () {
-    this.timeout(20000);
-    const lines: string[] = [];
-    for (let i = 0; i < 300; i++) {
-      lines.push(`f${i}() {`, `  local a${i}=1`, '  for x in a b c; do', `    while read -r line; do v${i}=$line; f${(i * 7) % 300}; done < /dev/null`, '  done',
-        `  g${i}=2; IFS=, ; [[ -n $x ]] && shopt -s extglob`, `  f${(i * 13) % 300}`, '}', `f${i} "$@"`, `if [ -n "$x" ]; then y${i}=1; else declare -A m${i}; fi`);
-    }
-    const tree = parser.parse(lines.join('\n'));
-    const t = Date.now();
-    shellState(tree);
-    assert.ok(Date.now() - t < 3000, `${Date.now() - t} ms`);
+    this.timeout(60000);
+    const time = (n: number): number => {
+      const lines: string[] = [];
+      for (let i = 0; i < n; i++) {
+        lines.push(`f${i}() {`, `  local a${i}=1`, '  for x in a b c; do', `    while read -r line; do v${i}=$line; f${(i * 7) % n}; done < /dev/null`, '  done',
+          `  g${i}=2; IFS=, ; [[ -n $x ]] && shopt -s extglob`, `  f${(i * 13) % n}`, '}', `f${i} "$@"`, `if [ -n "$x" ]; then y${i}=1; else declare -A m${i}; fi`);
+      }
+      const tree = parser.parse(lines.join('\n'));
+      const t = Date.now();
+      shellState(tree);
+      return Date.now() - t;
+    };
+    // (Twice the functions take about twice as long, measured in the same run: a fixed time fails on a loaded machine.)
+    const [half, all] = [time(150), time(300)];
+    assert.ok(all < 4 * half + 500, `150 functions: ${half} ms, 300: ${all} ms`);
   });
 });
 
@@ -685,17 +690,26 @@ describe('shell state: arithmetic', () => {
     // What may be assigned is looked for in the values the expression reads, also through a nameref.
     assert.deepStrictEqual(after('x=\'a[1] = 2, i++, ++j\'; declare -n r=x; (( y = $1 + r ))', 'a', 'i', 'j', 'y'), ['unknown', 'unknown', 'unknown', 'unknown']);
   });
-  it('gives up on an expression too long or too deep to follow, quickly', () => {
+  it('gives up on an expression too long or too deep to follow, quickly', function () {
+    this.timeout(60000);
     // (Each value reads the one before twice, and assigns: 2 ** 30 steps.)
     const values = ['v0=\'c++\'', ...Array.from({ length: 30 }, (_, i) => `v${i + 1}='v${i}+v${i}'`)].join('; ');
-    let start = Date.now();
+    // (Followed, the 2 ** 30 steps would take minutes: the bound is far above the 0.2 s it takes, so that a loaded
+    // machine does not fail it.)
+    const start = Date.now();
     // (`u`: an argument of `let` not followed may fail, which stops `let`.)
     assert.deepStrictEqual(after(`${values}; (( y = v30, z = 1 )); let w=v30 u=1`, 'y', 'z', 'w', 'u', 'c'), ['unknown', 'unknown', 'unknown', 'unknown', 'unknown']);
-    assert.ok(Date.now() - start < 1000, `took ${Date.now() - start} ms`);
-    // Past the steps of the whole script, the rest of it is not followed (each line here takes all the steps it may).
-    start = Date.now();
-    assert.deepStrictEqual(after(`${values}\n${'(( y = v30 ))\n'.repeat(60)}(( z = 1 ))`, 'z'), ['unknown']);
-    assert.ok(Date.now() - start < 3000, `took ${Date.now() - start} ms`);
+    assert.ok(Date.now() - start < 5000, `took ${Date.now() - start} ms`);
+    // Past the steps of the whole script, the rest of it is not followed (each line here takes all the steps it may):
+    // ten times the lines take about as long, measured in the same run (not against a fixed time, which a loaded
+    // machine may fail; followed, they would take ten times as long).
+    const timed = (lines: number): number => {
+      const t = Date.now();
+      assert.deepStrictEqual(after(`${values}\n${'(( y = v30 ))\n'.repeat(lines)}(( z = 1 ))`, 'z'), ['unknown']);
+      return Date.now() - t;
+    };
+    const [some, many] = [timed(60), timed(600)];
+    assert.ok(many < 3 * some + 500, `60 lines: ${some} ms, 600 lines: ${many} ms`);
     // Values nested deeper than followed here (bash: 1), or than the stack of the parser.
     const deep = ['v0=1', ...Array.from({ length: 100 }, (_, i) => `v${i + 1}='${'('.repeat(40)}v${i}${')'.repeat(40)}'`)].join('\n');
     assert.deepStrictEqual(after(`${deep}\n(( y = v100 ))`, 'y'), ['unknown']);