-
-
Notifications
You must be signed in to change notification settings - Fork 159
fix: compile large TypeScript source graphs natively #8383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| Compile large TypeScript package graphs directly, preserving nested namespace, | ||
| barrel-export, class-origin, asset, WebAssembly, and CommonJS linkage while | ||
| keeping native-addon dependencies on explicit compatibility paths. |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| //! Runtime getters for namespace re-exports of Perry-native modules. | ||
|
|
||
| use perry_hir::Module as HirModule; | ||
|
|
||
| use crate::module::LlModule; | ||
| use crate::types::{DOUBLE, I64, PTR}; | ||
|
|
||
| pub(super) fn emit_native_namespace_reexport_getters( | ||
| llmod: &mut LlModule, | ||
| hir: &HirModule, | ||
| module_prefix: &str, | ||
| ) { | ||
| // A namespace re-export of a compiler-native module has no compiled | ||
| // source module (and therefore no `@__perry_ns_<prefix>` global) behind | ||
| // it. Expose it as a zero-argument value getter on the re-exporting module | ||
| // so ordinary named imports can materialize the runtime-native namespace: | ||
| // | ||
| // export * as NodeWS from "ws" | ||
| // import { NodeWS } from "./NodeSocket" | ||
| // | ||
| // The driver classifies the consumer binding as `imported_vars`, so its | ||
| // ExternFuncRef value path calls this getter rather than creating a closure | ||
| // around a nonexistent function export. | ||
| for export in &hir.exports { | ||
| let perry_hir::Export::NamespaceReExport { source, name } = export else { | ||
| continue; | ||
| }; | ||
| if !perry_hir::NATIVE_MODULES.contains(&source.strip_prefix("node:").unwrap_or(source)) { | ||
| continue; | ||
| } | ||
| let getter_name = format!("perry_fn_{}__{}", module_prefix, name); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Sanitize the export name before using it in the LLVM symbol.
Emit the sanitized name as the canonical getter. If the raw spelling is also needed, mirror the pattern in 🤖 Prompt for AI Agents |
||
| if llmod.has_function(&getter_name) { | ||
| continue; | ||
| } | ||
| let (source_global, source_len) = llmod.add_string_constant(source); | ||
| let getter = llmod.define_function(&getter_name, DOUBLE, vec![]); | ||
| let _ = getter.create_block("entry"); | ||
| let blk = getter.block_mut(0).unwrap(); | ||
| if let Some(install) = crate::nm_install::nm_install_symbol(source) { | ||
| blk.call_void(install, &[]); | ||
| } | ||
| let value = blk.call( | ||
| DOUBLE, | ||
| "js_create_native_module_namespace", | ||
| &[ | ||
| (PTR, &format!("@{}", source_global)), | ||
| (I64, &source_len.to_string()), | ||
| ], | ||
| ); | ||
|
Comment on lines
+28
to
+49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Two producers of a native module namespace disagree on the specifier spelling. Both sites call
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
| blk.ret(DOUBLE, &value); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -460,11 +460,10 @@ pub enum NamespaceEntryKind { | |
| source_prefix: String, | ||
| source_local: String, | ||
| }, | ||
| /// Re-exported function from another module. Codegen declares the | ||
| /// target's `perry_fn_*` as extern, emits a per-callsite | ||
| /// `__perry_wrap_extern_*` thin wrapper (if not already emitted by | ||
| /// the import-wrapper pass), and calls | ||
| /// `js_closure_alloc_singleton` against that wrapper. | ||
| /// Re-exported function from another module. The namespace populator | ||
| /// declares the target's `perry_fn_*`, emits its own | ||
| /// `__perry_wrap_extern_*` thunk, and calls `js_closure_alloc_singleton` | ||
| /// against that wrapper. | ||
|
Comment on lines
+463
to
+466
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win Correct the stale ForeignFunction wrapper documentation in both codegen locations. The documented 📍 Affects 2 files
🤖 Prompt for AI Agents |
||
| ForeignFunction { | ||
| source_prefix: String, | ||
| source_local: String, | ||
|
|
@@ -474,6 +473,10 @@ pub enum NamespaceEntryKind { | |
| /// nested value IS the target module's `@__perry_ns_<source_prefix>` | ||
| /// global, populated by the target's own `__init`. | ||
| NestedNamespace { source_prefix: String }, | ||
| /// `export * as Name from "node:..."` (or another Perry-native module). | ||
| /// Native modules have no compiled `@__perry_ns_*` global, so codegen asks | ||
| /// the runtime to materialize their namespace directly. | ||
| NativeNamespace { specifier: String }, | ||
| } | ||
|
|
||
| /// A class imported from another native module. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Confirm that an inline
export conststill produces a getter.exported_var_namesnow only contains locals that appear in anExport::Namedentry. Two consequences follow if HIR does not record anExport::Named { local, exported }for a plain inlineexport const foo = {...}:foo, so no@perry_global_<prefix>__<id>is emitted for it.public_namesat lines 441-450 is empty, so thefor public_name in public_namesloop body never runs and noperry_fn_<prefix>__foogetter is emitted.A consumer that imports
foothen references an undefined getter symbol and the link fails. The narrowing itself is correct for renames, becauseexported_objectscarries both sides ofexport { $i as filesFilter }. The open question is only whether the inline declaration shape is still covered.Run the following script to check how HIR lowers an inline
export const:Also applies to: 441-450
🤖 Prompt for AI Agents