Skip to content

An import's errors are the importer's to report - #381

Merged
ASDAlexander77 merged 1 commit into
mainfrom
fix-import-retry-diagnostics
Sep 28, 2026
Merged

ASDAlexander77 merged 1 commit into
mainfrom
fix-import-retry-diagnostics

Conversation

@ASDAlexander77

Copy link
Copy Markdown
Owner

A compile that succeeded printed errors when its imports formed a cycle through a class hierarchy. Found in the BrowserLib sources attached to #231: Node.ts and TextNode.ts import each other, and TextNode extends Node.

// cycle_class_node.ts
import { TextNode } from "./cycle_class_text";
export class Node { /* … uses TextNode … */ }

// cycle_class_text.ts
import { Node } from "./cycle_class_node";
export class TextNode extends Node { … }
> tslang --emit=obj cycle_class_node.ts
cycle_class_text.ts:3:31: error: can't resolve name: Node
cycle_class_text.ts:3:1: error: failed statement
> echo $?
0

Cause

processStatements runs a failed statement again once the statements after it have been generated, and it discards the errors of an attempt that later succeeds. An import is one such statement. When cycle_class_node imports cycle_class_text, the first attempt fails because Node is not declared yet, and the next pass succeeds.

However, mlirGenInclude generates the imported file with its own mlirDiscoverAllDependencies and mlirCodeGenModule. Each of those installs its own diagnostic handler and prints its errors through outputDiagnostics before returning failure. By the time the importer decided to retry, the errors were already on the screen.

Fix

  • While an imported source file is generated, outputDiagnostics moves its errors into a list set up by mlirGenInclude, instead of printing them.
  • Once the import's own handlers are gone, mlirGenInclude emits them again, so they reach the importer's handler.
  • From there the importer's normal rules apply: a retry that succeeds drops them, and an import that still fails prints them, together with failed statement at the import line.
  • Parse errors of the imported file (showMessages) are unchanged.

Tests

  • New test-compile-reference-path-import-cycle-quiet compiles cycle_class_node.ts and fails if the output contains error:. It fails on main.
  • New test-compile-reference-path-import-cycle-class links the cycle into a program that walks the tree through instanceof TextNode.
  • The tests that expect an import to fail with a specific error still pass: test-compile-gc-shared-auto, test-compile-foreign-target-import-errors, and the reference-path-missing* tests.
  • Full release suite: 2989/2989 passed.

Not in this PR

The same two modules built into one -shared library fail for the importer with redefinition of symbol named 'Node..new'. The library declares Node twice to whoever imports it, once for each module. This fails the same way on main, so only the static variant is registered.

🤖 Generated with Claude Code

In an import cycle where a class extends one from the other file, the first
attempt at the inner import fails: the outer file has not declared the base
class yet. processStatements tries the import again and it succeeds, but the
import's own mlirDiscoverAllDependencies / mlirCodeGenModule had already
printed its errors, so a compile that succeeded printed "can't resolve name"
and "failed statement" (BrowserLib Node.ts / TextNode.ts, #231).

While an imported source file is generated, outputDiagnostics now collects its
errors instead of printing them, and mlirGenInclude re-emits them to the
importer's handler. There, a retry that succeeds discards them, and an import
that still fails prints them as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ASDAlexander77
ASDAlexander77 force-pushed the fix-import-retry-diagnostics branch from f6923d2 to 70c8715 Compare September 28, 2026 08:31
@ASDAlexander77
ASDAlexander77 merged commit 048c28f into main Sep 28, 2026
2 checks passed
@ASDAlexander77
ASDAlexander77 deleted the fix-import-retry-diagnostics branch September 28, 2026 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant