fix(FUN-7): restore _currentEnv on all error paths in buildTable:firstList:row:

[kostub]

Summary

• Three early return nil; paths in -[MTMathListBuilder buildTable:firstList:row:] skipped the _currentEnv = oldEnv restoration that the success path correctly performed, leaving the builder's environment pointer stale after any parse failure inside a table/matrix environment.
• Fix: add _currentEnv = oldEnv; before each of the three early returns — the buildInternal: failure path, the MTParseErrorMissingEnd path, and the tableWithEnvironment:rows:error: failure path.
• This is a latent/defensive fix: the builder sets _error on every error path and then unwinds without inspecting _currentEnv again, so no current call sequence observes the leaked pointer. The fix eliminates a future trap if error-handling or builder-reuse semantics change.

Closes FUN-7 (issues.md#L243).

Changes

• iosMath/lib/MTMathListBuilder.m — +3 lines, 1 file, no API or behavior change.

Test plan

• swift build — passes (Build complete)
• swift test — all tests pass, 0 failures (Test Suite 'All tests' passed)
• No new tests added: the fix is behavior-preserving for all existing paths; MTMathListBuilderTest already covers valid table parsing and MTParseErrorMissingEnd error cases. The leaked _currentEnv is not externally observable today (builder is single-use-per-parse, unwinds immediately on _error), so no meaningful new assertion is possible beyond confirming existing error cases still return the same nil + error code — which the existing suite already asserts.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request updates MTMathListBuilder.m to ensure that the previous environment (oldEnv) is correctly restored to _currentEnv before returning early on error paths within the buildTable:firstList:rows: method. There are no review comments, and I have no additional feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[kostub]

EM-REVIEW v1

Scope verified via gh pr diff 237 and the full buildTable:firstList:row: body on em/2026-06-11-issues/t15 (MTMathListBuilder.m lines 957-1010). Diff stat: 1 file, +3 insertions, no test.

All early-return paths covered — none missed. buildTable saves oldEnv at line 960 and installs a fresh _currentEnv at 961. There are exactly four exit points after that install:

• L981 (buildInternal returned nil) — now restores oldEnv ✓
• L998 (Missing \end) — now restores oldEnv ✓
• L1005 (tableWithEnvironment failed, !_error) — now restores oldEnv ✓
• L1008/1009 (success) — already restored pre-PR ✓

I grepped every return nil in the file; the only ones inside the function range (957-1010) are 981/998/1005, all three now handled. No other exit exists (no exceptions, no @throw).

Restore placement is correct. In each case _currentEnv = oldEnv; is the last statement before return nil, and it sits after the last use of the new env: at L996 setError: reads nothing env-related; at L1001 tableWithEnvironment:_currentEnv.envName already consumed the new env before the L1004 restore. No use-after-restore.

No behavioral change for current inputs — confirmed reachable-observation analysis. Every nil-return from buildTable coincides with _error being set (propagated from buildInternal, or set at L996 / L1003). The main parse loop buildInternal checks if (_error) return nil; at the top of each iteration (L166), so once any error is set the builder unwinds without re-entering env-dependent logic. The three callers (L294, L827, L897) only consume the returned table/nil and never read _currentEnv afterward. So the restored value is never observed today. Latent/defensive, as the PR claims.

"No test" is acceptable. There is no reachable code path that observes the restored _currentEnv after an error, so there is no externally observable (LaTeX-level) behavior to assert. _currentEnv is a private ivar, not public API; a test would have to reach into private state to prove the assignment ran, which contradicts the project's "LaTeX is the public API" compat contract and would pin an implementation detail rather than behavior. The existing 293-test suite is the correct regression guard for the success path. Agreed: no test warranted.

Sanity: swift build clean. The change is a strict superset of consistent env restores; no risk to the success path.

Verdict: non-blocking. Correct, minimal, consistent. The fix makes all four exit paths symmetric and removes a latent footgun for any future code that inspects _currentEnv after a table-build failure. Ship it.