Skip to content

Commit bb27386

Browse files
Fix cross-module generic class export/import (4 co-operating bugs) (#280)
A generic class's bare template declaration never reached the module's declaration-export text at all: mlirGen(ClassLikeDeclaration) returns early into registerGenericClass whenever processing the template (vs. a concrete instantiation), before ever calling addClassDeclarationToExport. Fixed by exporting the generic's own ORIGINAL source text (not a signature-only declaration like concrete classes get, since a generic has no compiled body anywhere for an importer to link against - it needs the real, recompilable source) via a new addGenericClassDeclarationToExport/printGenericClass. That exposed two more bugs once generics started actually reaching the reimport path: - The existing declaration-reimport mechanism parses with a "partial.d.ts" filename, which implicitly marks everything ambient/ external regardless of any per-declaration @dllimport marker - fine for every other declaration kind (bodies already compiled elsewhere) but fatal for a generic's real body. Fixed by giving generic declarations their own separate embedded global ("__decls_generic_<file>_<hash>"), reparsed with a plain ".ts" filename instead. - Reimporting the generic's own text re-declares it a second time inside the importer, which re-attempts the export call with a stale source-file/AST-position pairing, throwing an unhandled std::out_of_range (surfaced as a blocked "Microsoft Visual C++ Runtime Library" dialog - near-zero CPU, looked like a hang from the outside). Fixed with a bounds-check that skips the redundant re-export instead of crashing. Also fixes a related, separately-triggered bug: a concrete generic instantiation (e.g. Pair<number,string>) was inheriting its template's `export` modifier, and the mangled name's comma broke the linker's /EXPORT: directive syntax outright. Fixed by forcing isExport/isPublic false on any class reached via a generic specialization's codegen path. Re-enables all 3 tiers (compile / -shared / -jit-shared) of export_class_generic.ts/import_class_generic.ts, previously disabled. Full suite: 789/789, no regressions.
1 parent 71a1b7b commit bb27386

6 files changed

Lines changed: 209 additions & 28 deletions

File tree

tslang/include/TypeScript/MLIRLogic/MLIRDeclarationPrinter.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ namespace typescript
2121
void printVariableDeclaration(StringRef, NamespaceInfo::TypePtr, mlir::Type, bool);
2222
void print(StringRef, NamespaceInfo::TypePtr, mlir_ts::FunctionType);
2323
void print(ClassInfo::TypePtr);
24-
void print(InterfaceInfo::TypePtr);
24+
void print(InterfaceInfo::TypePtr);
25+
void printGenericClass(NamespaceInfo::TypePtr, StringRef);
2526

2627
protected:
2728
void newline();

tslang/lib/TypeScript/DeclarationPrinter.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,20 @@ namespace typescript
511511
printNamespaceEnd(classType->elementNamespace);
512512
}
513513

514+
void MLIRDeclarationPrinter::printGenericClass(NamespaceInfo::TypePtr elementNamespace, StringRef rawDeclarationText)
515+
{
516+
// unlike print(ClassInfo::TypePtr) above, a generic class has no compiled body for
517+
// any given instantiation to @dllimport against - each importing module
518+
// instantiates it locally instead, so the caller passes the ORIGINAL source text
519+
// (type parameters and method bodies intact) verbatim; this just supplies the
520+
// matching namespace wrapper, deliberately WITHOUT printBeforeDeclaration()'s
521+
// @dllimport marker.
522+
printNamespaceBegin(elementNamespace);
523+
os << rawDeclarationText;
524+
newline();
525+
printNamespaceEnd(elementNamespace);
526+
}
527+
514528
void MLIRDeclarationPrinter::print(InterfaceInfo::TypePtr interfaceType)
515529
{
516530
printNamespaceBegin(interfaceType->elementNamespace);

tslang/lib/TypeScript/MLIRGenClasses.cpp

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,16 @@ namespace mlirgen
1616
auto fullNamePtr = getFullNamespaceName(namePtr);
1717
if (fullNameGenericClassesMap.count(fullNamePtr))
1818
{
19+
// already registered - but the registration itself typically happens during
20+
// Stages::Discovering (before addGenericClassDeclarationToExport's
21+
// isAddedToExport gate, which only actually emits once stage ==
22+
// Stages::SourceGeneration, will do anything) - retry the export step alone
23+
// using the existing GenericClassInfo rather than skipping it entirely.
24+
if (getExportModifier(classDeclarationAST))
25+
{
26+
addGenericClassDeclarationToExport(fullNameGenericClassesMap.lookup(fullNamePtr));
27+
}
28+
1929
return mlir::success();
2030
}
2131

@@ -40,6 +50,18 @@ namespace mlirgen
4050
getGenericClassesMap().insert({namePtr, newGenericClassPtr});
4151
fullNameGenericClassesMap.insert(fullNamePtr, newGenericClassPtr);
4252

53+
// support dynamic loading: a generic class is never instantiated in this module
54+
// if nothing here uses it concretely, so mlirGen(ClassLikeDeclaration) never
55+
// reaches its own addClassDeclarationToExport call below (that only runs for a
56+
// SPECIALIZED instantiation, gated on genContext.typeParamsWithArgs being
57+
// non-empty) - the bare template needs to be exported here instead, the one
58+
// place every generic declaration passes through regardless of whether it is
59+
// ever instantiated locally.
60+
if (getExportModifier(classDeclarationAST))
61+
{
62+
addGenericClassDeclarationToExport(newGenericClassPtr);
63+
}
64+
4365
return mlir::success();
4466
}
4567

@@ -71,9 +93,27 @@ namespace mlirgen
7193
return {mlir::failure(), ""};
7294
}
7395

74-
// do not process specialized class second time;
7596
if (isGenericClass && genContext.typeParamsWithArgs.size() > 0)
7697
{
98+
// a concrete instantiation of a generic class (e.g. Box<number>) reprocesses the
99+
// SAME class declaration AST as the bare template (e.g. `export class Box<T>`),
100+
// so mlirGenClassInfo's isExport = getExportModifier(classDeclarationAST) above
101+
// would otherwise mark this LOCAL, per-instantiation specialization as exported
102+
// too - wrong on two counts: (1) semantically, a specialization materialized by
103+
// whichever module instantiates it is local to that module, not a re-export of
104+
// it (an importer using M.Box<number> isn't re-exporting M.Box<number> further);
105+
// (2) mechanically, a multi-type-param instantiation's mangled name contains a
106+
// raw comma (e.g. M.Pair<!ts.number,!ts.string>..instanceOf), which is a
107+
// metacharacter in the linker's `/EXPORT:name[,option]` directive syntax - lld
108+
// rejects it outright ("invalid /export:") the moment anything in it is
109+
// actually marked for export. The generic TEMPLATE's own declaration is already
110+
// exported correctly and separately (see registerGenericClass /
111+
// addGenericClassDeclarationToExport), which is the only export this class
112+
// needs.
113+
newClassPtr->isExport = false;
114+
newClassPtr->isPublic = false;
115+
116+
// do not process specialized class second time;
77117
// TODO: investigate why classType is provided already for class
78118
if (testProcessingState(newClassPtr, ProcessingStages::Processing, genContext))
79119
{

tslang/lib/TypeScript/MLIRGenImpl.h

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ class MLIRGenImpl
219219
#endif
220220

221221
mlir::LogicalResult createDeclarationExportGlobalVar(const GenContext &genContext);
222+
mlir::LogicalResult createGenericClassDeclarationExportGlobalVar(const GenContext &genContext);
222223

223224
bool isCodeStatment(SyntaxKind kind);
224225

@@ -10308,20 +10309,65 @@ class MLIRGenImpl
1030810309
{
1030910310
// already added
1031010311
return;
10311-
}
10312+
}
1031210313

1031310314
exportedTypes.insert(newClassPtr->classType);
1031410315

1031510316
addDependancyTypesToExport(newClassPtr->classType);
1031610317

1031710318
SmallVector<char> out;
10318-
llvm::raw_svector_ostream ss(out);
10319+
llvm::raw_svector_ostream ss(out);
1031910320
MLIRDeclarationPrinter dp(ss);
1032010321
dp.print(newClassPtr);
1032110322

1032210323
declExports << ss.str().str();
1032310324
}
1032410325

10326+
void addGenericClassDeclarationToExport(GenericClassInfo::TypePtr genericClassInfo)
10327+
{
10328+
if (isAddedToExport(genericClassInfo->classType))
10329+
{
10330+
// already added
10331+
return;
10332+
}
10333+
10334+
// A generic class re-declared while re-importing another module's embedded
10335+
// declarations (see mlirGenImportSharedLib) still carries its own `export` keyword
10336+
// verbatim (it is the original source, copied as-is), so it reaches this same
10337+
// function a second time from inside the IMPORTER's own compile - but at that
10338+
// point `sourceFile` is the ambient/outer file (e.g. the importer's own .ts), not
10339+
// the "partial" buffer parsePartialStatements actually parsed this declaration
10340+
// from, so classDeclaration's pos/_end (offsets into THAT buffer) do not correspond
10341+
// to sourceFile->text at all and can exceed its length. Re-exporting a
10342+
// re-declaration one level removed like this is also simply wrong (the importer
10343+
// isn't meant to re-export M's generics further) - bounds-check and skip rather
10344+
// than let getTextOfNodeFromSourceText's substr throw std::out_of_range.
10345+
auto declEnd = static_cast<size_t>(genericClassInfo->classDeclaration->_end);
10346+
if (declEnd > genericClassInfo->sourceFile->text.length())
10347+
{
10348+
return;
10349+
}
10350+
10351+
exportedTypes.insert(genericClassInfo->classType);
10352+
10353+
// unlike a concrete class (printed above as a signature-only, @dllimport-marked
10354+
// declaration - the real body is already compiled into this module's DLL), a
10355+
// generic class has no compiled body for any given instantiation: each importing
10356+
// module instantiates it locally, on demand, exactly like a same-file usage would.
10357+
// So the FULL original source - type parameters and method bodies intact, no
10358+
// @dllimport marker - must be re-exported verbatim for parsePartialStatements to
10359+
// recompile per instantiation in the importer.
10360+
auto declText = convertWideToUTF8(getTextOfNodeFromSourceText(
10361+
genericClassInfo->sourceFile->text, genericClassInfo->classDeclaration.as<Node>(), true));
10362+
10363+
SmallVector<char> out;
10364+
llvm::raw_svector_ostream ss(out);
10365+
MLIRDeclarationPrinter dp(ss);
10366+
dp.printGenericClass(genericClassInfo->elementNamespace, declText);
10367+
10368+
genericDeclExports << ss.str().str();
10369+
}
10370+
1032510371
auto getNamespaceName() -> StringRef
1032610372
{
1032710373
return currentNamespace->name;
@@ -10990,6 +11036,15 @@ class MLIRGenImpl
1099011036
bool declarationMode;
1099111037

1099211038
std::stringstream declExports;
11039+
// generic class declarations must be re-imported as plain (non-".d.ts") source: unlike
11040+
// declExports (parsed with a "partial.d.ts" filename, which makes the parser mark
11041+
// everything ambient/external regardless of any per-declaration @dllimport marker - see
11042+
// createDeclarationExportGlobalVar/mlirGenImportSharedLib), a generic class has no
11043+
// compiled body for any instantiation to link against - the importer instantiates it
11044+
// locally, and needs its real method bodies to actually be compilable, not treated as
11045+
// external stubs. Kept in a separate stream/global so it can be re-parsed with a plain
11046+
// ".ts" filename instead.
11047+
std::stringstream genericDeclExports;
1099311048
mlir::SmallPtrSet<mlir::Type, 32> exportCheckedDependenciesTypes;
1099411049
mlir::SmallPtrSet<mlir::Type, 32> exportedTypes;
1099511050

tslang/lib/TypeScript/MLIRGenModule.cpp

Lines changed: 63 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -334,9 +334,49 @@ namespace mlirgen
334334
llvm::sys::path::append(path, llvm::sys::path::filename(mainSourceFileName));
335335
llvm::sys::path::replace_extension(path, ".d.ts");
336336
return createDependencyDeclarationFile(path, declText);
337-
#else
338-
return success();
339-
#endif
337+
#else
338+
return success();
339+
#endif
340+
}
341+
342+
mlir::LogicalResult MLIRGenImpl::createGenericClassDeclarationExportGlobalVar(const GenContext &genContext)
343+
{
344+
if (!genericDeclExports.rdbuf()->in_avail() || !compileOptions.embedExportDeclarations)
345+
{
346+
return mlir::success();
347+
}
348+
349+
auto declText = genericDeclExports.str();
350+
351+
LLVM_DEBUG(llvm::dbgs() << "\n!! export generic class declaration: \n" << declText << "\n";);
352+
353+
auto typeWithInit = [&](mlir::Location location, const GenContext &genContext) {
354+
auto litValue = V(mlirGenStringValue(location, declText, true));
355+
return std::make_tuple(litValue.getType(), litValue, TypeProvided::No);
356+
};
357+
358+
auto loc = mlir::UnknownLoc::get(builder.getContext());
359+
360+
VariableClass varClass = VariableType::Var;
361+
varClass.isExport = true;
362+
varClass.isPublic = true;
363+
364+
// "generic" in the middle keeps the "__decls" prefix (so the existing
365+
// symbol.starts_with(SHARED_LIB_DECLARATIONS_2UNDERSCORE) enumeration in
366+
// mlirGenImportSharedLib still finds it) while staying distinguishable from the
367+
// regular per-file "__decls_<file>_<hash>" global, so that call site can tell the
368+
// two apart and parse each with the right file_d_ts flag.
369+
std::string varName(SHARED_LIB_DECLARATIONS_2UNDERSCORE);
370+
varName.append("_generic_");
371+
varName.append(llvm::sys::path::stem(llvm::sys::path::filename(mainSourceFileName)));
372+
varName.append("_");
373+
varName.append(to_string(hash_value(mainSourceFileName)));
374+
375+
auto varNameRef = StringRef(varName).copy(stringAllocator);
376+
377+
registerVariable(loc, varNameRef, true, varClass, typeWithInit, genContext);
378+
379+
return mlir::success();
340380
}
341381

342382
bool MLIRGenImpl::isCodeStatment(SyntaxKind kind)
@@ -649,6 +689,11 @@ namespace mlirgen
649689
outputDiagnostics(postponedMessages, 1);
650690
return mlir::failure();
651691
}
692+
693+
if (mlir::failed(createGenericClassDeclarationExportGlobalVar(genContext))) {
694+
outputDiagnostics(postponedMessages, 1);
695+
return mlir::failure();
696+
}
652697
}
653698

654699
clearTempModule();
@@ -879,15 +924,27 @@ namespace mlirgen
879924
LLVM_DEBUG(llvm::dbgs() << "\n!! Shared lib import: \n" << dataPtr << "\n";);
880925

881926
{
882-
MLIRLocationGuard vgLoc(overwriteLoc);
927+
MLIRLocationGuard vgLoc(overwriteLoc);
883928
overwriteLoc = location;
884929

930+
// a generic class's declaration (see
931+
// createGenericClassDeclarationExportGlobalVar) must NOT be parsed with
932+
// the ".d.ts" filename convention used below for every other kind of
933+
// declaration: that convention makes the parser mark everything ambient/
934+
// external regardless of any per-declaration @dllimport marker (see the
935+
// comment on parsePartialStatements's file_d_ts parameter), which would
936+
// make the generic's instantiated specializations wrongly look like
937+
// external stubs with no compilable body - they need to be treated as
938+
// ordinary, fully-compilable local source instead.
939+
auto isGenericClassDecl =
940+
declSymbol.starts_with(std::string(SHARED_LIB_DECLARATIONS_2UNDERSCORE) + "_generic_");
941+
885942
auto importData = convertUTF8toWide(dataPtr);
886-
if (mlir::failed(parsePartialStatements(importData, genContext, false, true)))
943+
if (mlir::failed(parsePartialStatements(importData, genContext, false, !isGenericClassDecl)))
887944
{
888945
//assert(false);
889946
return mlir::failure();
890-
}
947+
}
891948
}
892949
}
893950
else

0 commit comments

Comments
 (0)