From 70c8715e257e7e48dbabfb9d5064b4267f88959b Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Sun, 27 Sep 2026 23:34:15 +0100 Subject: [PATCH] An import's errors are the importer's to report 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 --- tslang/lib/TypeScript/MLIRGenImpl.h | 5 +++ tslang/lib/TypeScript/MLIRGenModule.cpp | 33 ++++++++++++++++--- tslang/test/tester/CMakeLists.txt | 9 +++++ .../tester/reference-path/cycle_class_node.ts | 26 +++++++++++++++ .../tester/reference-path/cycle_class_text.ts | 10 ++++++ .../reference-path/import_cycle_class.ts | 13 ++++++++ 6 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 tslang/test/tester/reference-path/cycle_class_node.ts create mode 100644 tslang/test/tester/reference-path/cycle_class_text.ts create mode 100644 tslang/test/tester/reference-path/import_cycle_class.ts diff --git a/tslang/lib/TypeScript/MLIRGenImpl.h b/tslang/lib/TypeScript/MLIRGenImpl.h index f0cf0ce48..87a5eac50 100644 --- a/tslang/lib/TypeScript/MLIRGenImpl.h +++ b/tslang/lib/TypeScript/MLIRGenImpl.h @@ -12743,6 +12743,11 @@ class MLIRGenImpl // next pass; see mlirGen(FunctionDeclaration) std::map> declaredFunctions; + // Set while an imported source file is generated (mlirGenInclude): the errors of the file go + // here instead of being printed, and the import hands them to the importer, whose + // processStatements tries a failed import again and prints only what the last attempt left. + mlir::SmallVector> *importDiagnostics = nullptr; + // set while an `import { a as b }` alias is resolved to its target (resolveImportAlias) bool resolvingImportAlias = false; diff --git a/tslang/lib/TypeScript/MLIRGenModule.cpp b/tslang/lib/TypeScript/MLIRGenModule.cpp index 2d3e0c9fa..fa9bb43b8 100644 --- a/tslang/lib/TypeScript/MLIRGenModule.cpp +++ b/tslang/lib/TypeScript/MLIRGenModule.cpp @@ -783,8 +783,15 @@ namespace mlirgen mlir::LogicalResult MLIRGenImpl::outputDiagnostics(mlir::SmallVector> &postponedMessages, int notResolved) { - // print errors - if (notResolved) + // print errors, or hand them to the importer of this file + if (notResolved && importDiagnostics) + { + for (auto &diag : postponedMessages) + { + importDiagnostics->push_back(std::move(diag)); + } + } + else if (notResolved) { printDiagnostics(sourceMgrHandler, postponedMessages, compileOptions.disableWarnings); } @@ -1117,8 +1124,26 @@ namespace mlirgen // we need to override filename to track it in DBG info SourceFileScope sourceFileScope(*this, importSource); - if (mlir::succeeded(mlirDiscoverAllDependencies(importSource, importIncludeFiles)) && - mlir::succeeded(mlirCodeGenModule(importSource, importIncludeFiles, false, false))) + // An import cycle (a imports b imports a) fails b on its first attempt: a has not declared + // what b needs yet. a's processStatements tries the import again once it has, so b's errors + // are the importer's to report, and only if the import still fails. + mlir::SmallVector> diagnostics; + auto generated = false; + { + MLIRValueGuard> *> diagnosticsGuard(importDiagnostics); + importDiagnostics = &diagnostics; + + generated = mlir::succeeded(mlirDiscoverAllDependencies(importSource, importIncludeFiles)) && + mlir::succeeded(mlirCodeGenModule(importSource, importIncludeFiles, false, false)); + } + + // the import's own diagnostic handlers are gone: these reach the importer's + for (auto &diag : diagnostics) + { + builder.getContext()->getDiagEngine().emit(std::move(*diag)); + } + + if (generated) { // only now: an import that failed is tried again on the next pass, and must fail // again rather than find itself already done. Its library declarations, if a diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index f5087e746..c41991441 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -2435,6 +2435,15 @@ tslang_add_import_tests(reference-path-import-and-reference "${reference_path_di tslang_add_import_tests(reference-path-two-imports "${reference_path_dir}" two_imports module_left module_right) tslang_add_import_tests(reference-path-import-diamond "${reference_path_dir}" import_diamond plain_module via_module) tslang_add_import_tests(reference-path-import-cycle "${reference_path_dir}" import_cycle cycle_module_a cycle_module_b) +# Static linking only: a -shared library of these two modules declares Node twice to its importer +# ("redefinition of symbol named 'Node..new'"), a separate gap. +tslang_add_test(NAME test-compile-reference-path-import-cycle-class COMMAND test-runner "${reference_path_dir}/import_cycle_class.ts" "${reference_path_dir}/cycle_class_node.ts" "${reference_path_dir}/cycle_class_text.ts") +# The first attempt at an import in a cycle fails and is tried again; its errors were printed +# although the compile succeeded. +add_test(NAME test-compile-reference-path-import-cycle-quiet + COMMAND $ --emit=obj --no-default-lib + "${reference_path_dir}/cycle_class_node.ts" -o "${CMAKE_CURRENT_BINARY_DIR}/cycle_class_node.obj") +set_tests_properties(test-compile-reference-path-import-cycle-quiet PROPERTIES FAIL_REGULAR_EXPRESSION "error:") # `import { a as b }` and `import * as M` - each used to be "can't resolve name". set(import_bindings_dir "${PROJECT_SOURCE_DIR}/test/tester/import-bindings") diff --git a/tslang/test/tester/reference-path/cycle_class_node.ts b/tslang/test/tester/reference-path/cycle_class_node.ts new file mode 100644 index 000000000..6ab5e2886 --- /dev/null +++ b/tslang/test/tester/reference-path/cycle_class_node.ts @@ -0,0 +1,26 @@ +// cycle_class_node and cycle_class_text import each other, and TextNode extends Node: the first +// attempt at cycle_class_text fails (Node is not declared yet) and is tried again. That attempt's +// errors used to be printed although the compile then succeeded. +import { TextNode } from "./cycle_class_text"; + +export class Node { + childNodes: Node[] = []; + + get textContent(): string { + let t = ""; + for (let i = 0; i < this.childNodes.length; i++) { + const c = this.childNodes[i]; + if (c instanceof TextNode) { + t += c.data; + } else { + t += c.textContent; + } + } + + return t; + } + + addText(s: string) { + this.childNodes.push(new TextNode(s)); + } +} diff --git a/tslang/test/tester/reference-path/cycle_class_text.ts b/tslang/test/tester/reference-path/cycle_class_text.ts new file mode 100644 index 000000000..4bc663b3b --- /dev/null +++ b/tslang/test/tester/reference-path/cycle_class_text.ts @@ -0,0 +1,10 @@ +import { Node } from "./cycle_class_node"; + +export class TextNode extends Node { + data: string; + + constructor(d: string) { + super(); + this.data = d; + } +} diff --git a/tslang/test/tester/reference-path/import_cycle_class.ts b/tslang/test/tester/reference-path/import_cycle_class.ts new file mode 100644 index 000000000..d4f4c2e58 --- /dev/null +++ b/tslang/test/tester/reference-path/import_cycle_class.ts @@ -0,0 +1,13 @@ +import { Node } from "./cycle_class_node"; + +function main() { + const n = new Node(); + n.addText("a"); + n.addText("b"); + const inner = new Node(); + inner.addText("c"); + n.childNodes.push(inner); + assert(n.textContent == "abc"); + + print("done."); +}