Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion crates/perry-codegen/src/codegen/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,6 @@ pub(super) fn compile_closure(
option_object_locals: HashMap::new(),
object_literal_locals: HashSet::new(),
namespace_imports: &cross_module.namespace_imports,
namespace_reexport_named_imports: &cross_module.namespace_reexport_named_imports,
namespace_member_prefixes: &cross_module.namespace_member_prefixes,
namespace_member_origin_names: &cross_module.namespace_member_origin_names,
imported_async_funcs: &cross_module.imported_async_funcs,
Expand Down
2 changes: 0 additions & 2 deletions crates/perry-codegen/src/codegen/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,6 @@ pub(super) fn compile_module_entry(
option_object_locals: HashMap::new(),
object_literal_locals: HashSet::new(),
namespace_imports: &cross_module.namespace_imports,
namespace_reexport_named_imports: &cross_module.namespace_reexport_named_imports,
namespace_member_prefixes: &cross_module.namespace_member_prefixes,
namespace_member_origin_names: &cross_module.namespace_member_origin_names,
imported_async_funcs: &cross_module.imported_async_funcs,
Expand Down Expand Up @@ -1172,7 +1171,6 @@ pub(super) fn compile_module_entry(
option_object_locals: HashMap::new(),
object_literal_locals: HashSet::new(),
namespace_imports: &cross_module.namespace_imports,
namespace_reexport_named_imports: &cross_module.namespace_reexport_named_imports,
namespace_member_prefixes: &cross_module.namespace_member_prefixes,
namespace_member_origin_names: &cross_module.namespace_member_origin_names,
imported_async_funcs: &cross_module.imported_async_funcs,
Expand Down
1 change: 0 additions & 1 deletion crates/perry-codegen/src/codegen/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,6 @@ pub(super) fn compile_function(
option_object_locals: HashMap::new(),
object_literal_locals: HashSet::new(),
namespace_imports: &cross_module.namespace_imports,
namespace_reexport_named_imports: &cross_module.namespace_reexport_named_imports,
namespace_member_prefixes: &cross_module.namespace_member_prefixes,
namespace_member_origin_names: &cross_module.namespace_member_origin_names,
imported_async_funcs: &cross_module.imported_async_funcs,
Expand Down
2 changes: 0 additions & 2 deletions crates/perry-codegen/src/codegen/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,6 @@ pub(super) fn compile_method(
option_object_locals: HashMap::new(),
object_literal_locals: HashSet::new(),
namespace_imports: &cross_module.namespace_imports,
namespace_reexport_named_imports: &cross_module.namespace_reexport_named_imports,
namespace_member_prefixes: &cross_module.namespace_member_prefixes,
namespace_member_origin_names: &cross_module.namespace_member_origin_names,
imported_async_funcs: &cross_module.imported_async_funcs,
Expand Down Expand Up @@ -1363,7 +1362,6 @@ pub(super) fn compile_static_method(
option_object_locals: HashMap::new(),
object_literal_locals: HashSet::new(),
namespace_imports: &cross_module.namespace_imports,
namespace_reexport_named_imports: &cross_module.namespace_reexport_named_imports,
namespace_member_prefixes: &cross_module.namespace_member_prefixes,
namespace_member_origin_names: &cross_module.namespace_member_origin_names,
imported_async_funcs: &cross_module.imported_async_funcs,
Expand Down
1 change: 0 additions & 1 deletion crates/perry-codegen/src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1465,7 +1465,6 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>

let mut cross_module = CrossModuleCtx {
namespace_imports: opts.namespace_imports.iter().cloned().collect(),
namespace_reexport_named_imports: opts.namespace_reexport_named_imports.clone(),
namespace_member_prefixes: opts.namespace_member_prefixes,
namespace_member_origin_names: opts.namespace_member_origin_names,
imported_async_funcs: opts.imported_async_funcs,
Expand Down
17 changes: 0 additions & 17 deletions crates/perry-codegen/src/codegen/opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,21 +203,6 @@ pub struct CompileOptions {
/// Codegen uses this to know that `X.foo()` should be dispatched as
/// a cross-module call rather than an object method call.
pub namespace_imports: Vec<String>,
/// Issue #321: subset of `namespace_imports` populated by the
/// "named import resolves to a `export * as Foo from "./Foo"`" branch
/// in `compile.rs`. When the user wrote `import { Effect } from
/// "effect"` and effect's index.ts has `export * as Effect from
/// "./Effect.js"`, Effect lands in `namespace_imports` (so member
/// dispatch works) AND in this set (so the StaticMethodCall codegen
/// arm knows it's safe to route var-shape members through
/// `js_closure_callN`). Plain `import * as Effect from "./Effect"`
/// (used heavily by effect's INTERNAL modules) populates only
/// `namespace_imports`, NOT this set — the pre-existing direct-call
/// path preserves their long-standing silently-wrong-but-doesn't-throw
/// behavior on var-shape static calls (the right fix there is a
/// broader audit; doing it together with the named-import fix
/// surfaces init-order issues hiding behind the silent-wrong shape).
pub namespace_reexport_named_imports: std::collections::HashSet<String>,
/// Imported class definitions from other native modules, keyed by
/// the local alias (or original name when no alias). Each entry
/// carries the class HIR, the module prefix of its origin, and an
Expand Down Expand Up @@ -569,8 +554,6 @@ impl ImportedCtor {
/// Built once in `compile_module` from `CompileOptions`.
pub(crate) struct CrossModuleCtx {
pub namespace_imports: std::collections::HashSet<String>,
/// Issue #321: see `CompileOptions::namespace_reexport_named_imports`.
pub namespace_reexport_named_imports: std::collections::HashSet<String>,
/// Issue #680: per-namespace member resolution. See doc on
/// `CompileOptions::namespace_member_prefixes`.
pub namespace_member_prefixes: std::collections::HashMap<(String, String), String>,
Expand Down
8 changes: 0 additions & 8 deletions crates/perry-codegen/src/expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,14 +448,6 @@ pub(crate) struct FnCtx<'a> {
/// Codegen uses this to know that `X.foo()` should be dispatched as
/// a cross-module call rather than an object method call.
pub namespace_imports: &'a std::collections::HashSet<String>,
/// Issue #321: subset of `namespace_imports` populated only by the
/// "named import resolves to a `export * as Foo from "./Foo"`" branch
/// in `compile.rs`. The StaticMethodCall arm uses this to decide
/// whether to route var-shape members through `js_closure_callN`
/// (safe for the user-import shape) vs. preserving the pre-fix
/// direct-call (silently-wrong-but-doesn't-throw) path used by
/// `import * as` namespaces in effect's internal modules.
pub namespace_reexport_named_imports: &'a std::collections::HashSet<String>,
/// Issue #680: per-namespace member resolution. Keyed by
/// `(namespace_local_name, member_name)` → `source_prefix`. Consulted
/// by namespace member access lowering to disambiguate when the same
Expand Down
16 changes: 3 additions & 13 deletions crates/perry-codegen/src/expr/static_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,19 +262,9 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
// which `runSync` then read `._tag` off and threw
// `Cannot read properties of undefined`.
//
// SCOPE: only fire when the class_name was registered as a
// namespace via the *named-import-of-namespace-reexport*
// branch (`import { Effect } from "effect"` where effect's
// index.ts has `export * as Effect from "./Effect.js"`).
// Plain `import * as X from "./X.js"` (used in effect's
// INTERNAL modules) deliberately preserves the pre-fix
// direct-call (silently-wrong-but-doesn't-throw) path —
// switching them all over surfaces init-order bugs that
// were hiding behind the silent shape. Those need a
// separate audit.
if ctx.namespace_reexport_named_imports.contains(class_name)
&& ctx.imported_vars.contains(method_name)
{
// Both wildcard imports and namespaces reached through a
// re-export use the same getter ABI.
if ctx.imported_vars.contains(method_name) {
Comment on lines +265 to +267

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Scope variable classification by namespace.

imported_vars is a flat set. If A.make is a declared function and B.make is a closure-valued export, importing both namespaces inserts "make" and makes A.make() call its function body as a zero-arg getter, then invoke its return value as a closure.

Add a namespace-qualified variable set, populated in run_pipeline.rs alongside namespace_member_prefixes, and consult (class_name, method_name) here. Keep the flat set for named-import value lowering.

Proposed direction
- if ctx.imported_vars.contains(method_name) {
+ if ctx
+     .namespace_imported_vars
+     .contains(&(class_name.clone(), method_name.clone()))
+ {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/src/expr/static_method.rs` around lines 265 - 267,
Replace the flat imported_vars check in static-method handling with a
namespace-qualified variable classification keyed by (class_name, method_name).
Populate this set in run_pipeline.rs alongside namespace_member_prefixes, use it
to identify closure-valued namespace exports, and retain imported_vars for
named-import value lowering.

let mut lowered: Vec<String> = Vec::with_capacity(args.len());
for a in args {
lowered.push(lower_expr(ctx, a)?);
Expand Down
1 change: 0 additions & 1 deletion crates/perry-codegen/tests/argless_builtin_extra_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ fn empty_opts() -> CompileOptions {
verify_native_regions: false,
disable_buffer_fast_path: false,
namespace_imports: Vec::new(),
namespace_reexport_named_imports: std::collections::HashSet::new(),
imported_classes: Vec::new(),
imported_enums: Vec::new(),
imported_async_funcs: std::collections::HashSet::new(),
Expand Down
1 change: 0 additions & 1 deletion crates/perry-codegen/tests/class_keys_gc_root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ fn entry_opts() -> CompileOptions {
verify_native_regions: false,
disable_buffer_fast_path: false,
namespace_imports: Vec::new(),
namespace_reexport_named_imports: std::collections::HashSet::new(),
imported_classes: Vec::new(),
imported_enums: Vec::new(),
imported_async_funcs: std::collections::HashSet::new(),
Expand Down
1 change: 0 additions & 1 deletion crates/perry-codegen/tests/constructor_recursion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ fn empty_opts() -> CompileOptions {
verify_native_regions: false,
disable_buffer_fast_path: false,
namespace_imports: Vec::new(),
namespace_reexport_named_imports: std::collections::HashSet::new(),
imported_classes: Vec::new(),
imported_enums: Vec::new(),
imported_async_funcs: std::collections::HashSet::new(),
Expand Down
1 change: 0 additions & 1 deletion crates/perry-codegen/tests/destructure_call_location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ fn base_opts() -> CompileOptions {
verify_native_regions: false,
disable_buffer_fast_path: false,
namespace_imports: Vec::new(),
namespace_reexport_named_imports: std::collections::HashSet::new(),
imported_classes: Vec::new(),
imported_enums: Vec::new(),
imported_async_funcs: std::collections::HashSet::new(),
Expand Down
1 change: 0 additions & 1 deletion crates/perry-codegen/tests/large_object_barriers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ fn empty_opts() -> CompileOptions {
verify_native_regions: false,
disable_buffer_fast_path: false,
namespace_imports: Vec::new(),
namespace_reexport_named_imports: std::collections::HashSet::new(),
imported_classes: Vec::new(),
imported_enums: Vec::new(),
imported_async_funcs: std::collections::HashSet::new(),
Expand Down
1 change: 0 additions & 1 deletion crates/perry-codegen/tests/macos_bundle_chdir_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ fn entry_opts(target: Option<&str>) -> CompileOptions {
verify_native_regions: false,
disable_buffer_fast_path: false,
namespace_imports: Vec::new(),
namespace_reexport_named_imports: std::collections::HashSet::new(),
imported_classes: Vec::new(),
imported_enums: Vec::new(),
imported_async_funcs: std::collections::HashSet::new(),
Expand Down
1 change: 0 additions & 1 deletion crates/perry-codegen/tests/native_proof_buffer_views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ fn empty_opts() -> CompileOptions {
verify_native_regions: false,
disable_buffer_fast_path: false,
namespace_imports: Vec::new(),
namespace_reexport_named_imports: std::collections::HashSet::new(),
imported_classes: Vec::new(),
imported_enums: Vec::new(),
imported_async_funcs: std::collections::HashSet::new(),
Expand Down
1 change: 0 additions & 1 deletion crates/perry-codegen/tests/native_proof_regressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ fn empty_opts() -> CompileOptions {
verify_native_regions: false,
disable_buffer_fast_path: false,
namespace_imports: Vec::new(),
namespace_reexport_named_imports: std::collections::HashSet::new(),
imported_classes: Vec::new(),
imported_enums: Vec::new(),
imported_async_funcs: std::collections::HashSet::new(),
Expand Down
1 change: 0 additions & 1 deletion crates/perry-codegen/tests/shadow_slot_hygiene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ fn empty_opts() -> CompileOptions {
imported_func_has_rest: std::collections::HashSet::new(),
imported_func_synthetic_arguments: std::collections::HashSet::new(),
imported_func_return_types: std::collections::HashMap::new(),
namespace_reexport_named_imports: std::collections::HashSet::new(),
imported_vars: std::collections::HashSet::new(),
output_type: "executable".to_string(),
needs_stdlib: false,
Expand Down
1 change: 0 additions & 1 deletion crates/perry-codegen/tests/static_symbol_hygiene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ fn empty_opts() -> CompileOptions {
verify_native_regions: false,
disable_buffer_fast_path: false,
namespace_imports: Vec::new(),
namespace_reexport_named_imports: std::collections::HashSet::new(),
imported_classes: Vec::new(),
imported_enums: Vec::new(),
imported_async_funcs: std::collections::HashSet::new(),
Expand Down
1 change: 0 additions & 1 deletion crates/perry-codegen/tests/typed_feedback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ fn empty_opts() -> CompileOptions {
verify_native_regions: false,
disable_buffer_fast_path: false,
namespace_imports: Vec::new(),
namespace_reexport_named_imports: std::collections::HashSet::new(),
imported_classes: Vec::new(),
imported_enums: Vec::new(),
imported_async_funcs: std::collections::HashSet::new(),
Expand Down
1 change: 0 additions & 1 deletion crates/perry-codegen/tests/typed_shape_descriptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ fn empty_opts() -> CompileOptions {
verify_native_regions: false,
disable_buffer_fast_path: false,
namespace_imports: Vec::new(),
namespace_reexport_named_imports: std::collections::HashSet::new(),
imported_classes: Vec::new(),
imported_enums: Vec::new(),
imported_async_funcs: std::collections::HashSet::new(),
Expand Down
1 change: 0 additions & 1 deletion crates/perry-codegen/tests/typed_shape_descriptors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ fn empty_opts() -> CompileOptions {
verify_native_regions: false,
disable_buffer_fast_path: false,
namespace_imports: Vec::new(),
namespace_reexport_named_imports: std::collections::HashSet::new(),
imported_classes: Vec::new(),
imported_enums: Vec::new(),
imported_async_funcs: std::collections::HashSet::new(),
Expand Down
15 changes: 0 additions & 15 deletions crates/perry/src/commands/compile/object_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,20 +433,6 @@ fn compute_object_cache_key_with_env(
);
}

// Issue #321: namespace reexport named imports — separate subset that
// gates the codegen's StaticMethodCall var-shape routing. Cache must
// discriminate between two modules whose `namespace_imports` are
// identical but whose `namespace_reexport_named_imports` differ, else
// the wrong code-path winds up in the object file.
{
let mut v: Vec<&String> = opts.namespace_reexport_named_imports.iter().collect();
v.sort();
h.field(
"ns_reexport_named_imports",
&v.iter().map(|s| s.as_str()).collect::<Vec<_>>().join(","),
);
}

// Import function prefixes (HashMap — MUST sort).
{
let mut v: Vec<(&String, &String)> = opts.import_function_prefixes.iter().collect();
Expand Down Expand Up @@ -1064,7 +1050,6 @@ mod object_cache_tests {
verify_native_regions: false,
disable_buffer_fast_path: false,
namespace_imports: Vec::new(),
namespace_reexport_named_imports: std::collections::HashSet::new(),
imported_classes: Vec::new(),
imported_enums: Vec::new(),
imported_async_funcs: std::collections::HashSet::new(),
Expand Down
30 changes: 12 additions & 18 deletions crates/perry/src/commands/compile/run_pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2223,14 +2223,6 @@ pub fn run_with_parse_cache(
std::collections::HashMap<(String, String), String> =
std::collections::HashMap::new();
let mut namespace_imports: Vec<String> = Vec::new();
// Issue #321: subset of `namespace_imports` populated only by the
// named-import-of-namespace-reexport branch below (`import { Effect
// } from "effect"` where effect's index.ts has `export * as Effect
// from "./Effect.js"`). The codegen's StaticMethodCall arm consults
// this to decide whether it can route var-shape members through
// `js_closure_callN`; see the field doc in codegen.rs.
let mut namespace_reexport_named_imports: std::collections::HashSet<String> =
std::collections::HashSet::new();
let mut imported_classes: Vec<perry_codegen::ImportedClass> = Vec::new();
let mut imported_enums: Vec<(String, Vec<(String, perry_hir::EnumValue)>)> = Vec::new();
let mut imported_async_set: std::collections::HashSet<String> =
Expand Down Expand Up @@ -2714,13 +2706,6 @@ pub fn run_with_parse_cache(
break;
};
namespace_imports.push(local_name.clone());
// Issue #321: tag this local as a "named-import-
// of-namespace-reexport" so codegen's
// StaticMethodCall arm knows to route var-shape
// members through `js_closure_callN`. See the
// expr.rs StaticMethodCall comment for why this
// is scoped narrowly.
namespace_reexport_named_imports.insert(local_name.clone());
for (export_name, origin_path) in target_exports {
let origin_prefix =
compute_module_prefix(origin_path, &ctx.project_root);
Expand Down Expand Up @@ -2800,7 +2785,9 @@ pub fn run_with_parse_cache(
// else the export name itself.
namespace_member_origin_names.insert(
(local_name.clone(), export_name.clone()),
resolved_origin_name.unwrap_or_else(|| export_name.clone()),
resolved_origin_name
.clone()
.unwrap_or_else(|| export_name.clone()),
);

let key = (origin_path.clone(), export_name.clone());
Expand Down Expand Up @@ -2834,7 +2821,15 @@ pub fn run_with_parse_cache(
// `Cannot read properties of undefined`
// on `program._tag`. Mirrors the
// `Namespace { local }` branch above.
if exported_var_names.contains(&key) {
let origin_key_under_origin_name = resolved_origin_name
.as_ref()
.map(|name| (origin_path.clone(), name.clone()));
if exported_var_names.contains(&key)
|| origin_key_under_origin_name
.as_ref()
.map(|key| exported_var_names.contains(key))
.unwrap_or(false)
{
imported_vars.insert(export_name.clone());
}
if let Some(class) = exported_classes.get(&key) {
Expand Down Expand Up @@ -3817,7 +3812,6 @@ pub fn run_with_parse_cache(
verify_native_regions,
disable_buffer_fast_path,
namespace_imports,
namespace_reexport_named_imports,
imported_classes,
imported_enums,
imported_async_funcs: imported_async_set,
Expand Down
Loading
Loading