diff --git a/CHANGELOG.md b/CHANGELOG.md index c49f3f67..62099852 100644 --- a/CHANGELOG.md +++ b/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 /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) diff --git a/src/semantics/env.ts b/src/semantics/env.ts index 648d52f1..6209dd63 100644 --- a/src/semantics/env.ts +++ b/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(); +const callProcesses = new WeakMap(); /** * 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'; diff --git a/test/env.test.ts b/test/env.test.ts index 0ae55367..4307aac0 100644 --- a/test/env.test.ts +++ b/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> = { + 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); + } + }); }); diff --git a/test/nested.test.ts b/test/nested.test.ts index e3d98f90..a2ad0ffd 100644 --- a/test/nested.test.ts +++ b/test/nested.test.ts @@ -505,7 +505,8 @@ 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); @@ -513,10 +514,12 @@ describe('nested scripts: the issues of a string another shell reads', () => { 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)}`); } }); }); diff --git a/test/state.test.ts b/test/state.test.ts index fbe13e9b..61e02cce 100644 --- a/test/state.test.ts +++ b/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']);