Merge fix/module-call-byvalue: module free call lowers by value, not borrow

olwang · claude · olwang · commit 8f0659137e0a · 2026-06-15T03:05:35.000-07:00
Fixes rss run E0308 (&amp;T vs T) for bare module-qualified calls
(m.fn()); de-prefixed modules in tinygrad-rsmc now execute.
Co-Authored-By: Claude Opus 4.8 &lt;noreply@anthropic.com&gt;
diff --git a/crates/rsscript/src/analyzer.rs b/crates/rsscript/src/analyzer.rs
@@ -6250,7 +6250,7 @@ fn callee_display(callee: &Callee) -> String {
             effect,
         } => format!(
             "{} {}.{method}",
-            effect.as_str(),
+            (*effect).unwrap_or(DataEffect::Read).as_str(),
             analyzer_expr_label(receiver)
         ),
     }
diff --git a/crates/rsscript/src/checks/body.rs b/crates/rsscript/src/checks/body.rs
@@ -3355,7 +3355,7 @@ fn body_callee_display(callee: &Callee) -> String {
             receiver,
             method,
             effect,
-        } => format!("{} {}.{method}", effect.as_str(), body_expr_label(receiver)),
+        } => format!("{} {}.{method}", (*effect).unwrap_or(DataEffect::Read).as_str(), body_expr_label(receiver)),
     }
 }
 
diff --git a/crates/rsscript/src/checks/calls.rs b/crates/rsscript/src/checks/calls.rs
@@ -1993,13 +1993,13 @@ fn check_receiver_call_self_effect(
         );
         return;
     };
-    if expected.as_str() != effect.as_str() {
+    if expected.as_str() != (*effect).unwrap_or(DataEffect::Read).as_str() {
         analyzer.diagnostics.push(
             Diagnostic::error(
                 code::MISSING_DATA_EFFECT,
                 format!(
                     "receiver `{receiver_label}` for `{method}` uses `{}` but the method requires `{}`.",
-                    effect.as_str(),
+                    (*effect).unwrap_or(DataEffect::Read).as_str(),
                     expected.as_str()
                 ),
                 call_span.clone(),
@@ -4509,7 +4509,7 @@ fn callee_display(callee: &Callee) -> String {
             receiver,
             method,
             effect,
-        } => format!("{} {}.{method}", effect.as_str(), call_expr_label(receiver)),
+        } => format!("{} {}.{method}", (*effect).unwrap_or(DataEffect::Read).as_str(), call_expr_label(receiver)),
     }
 }
 
@@ -4533,7 +4533,7 @@ fn expr_data_effect(expr: &HirExpr) -> Option<&'static str> {
         HirExpr::Call {
             callee:
                 Callee::ReceiverCall {
-                    effect: DataEffect::Read,
+                    effect: Some(DataEffect::Read) | None,
                     ..
                 },
             ..
diff --git a/crates/rsscript/src/checks/local.rs b/crates/rsscript/src/checks/local.rs
@@ -1441,7 +1441,7 @@ fn callee_display(callee: &Callee) -> String {
             effect,
         } => format!(
             "{} {}.{method}",
-            effect.as_str(),
+            (*effect).map(|e| e.as_str()).unwrap_or("read"),
             local_expr_label(receiver)
         ),
     }
diff --git a/crates/rsscript/src/formatter.rs b/crates/rsscript/src/formatter.rs
@@ -987,8 +987,8 @@ impl Formatter {
                 method,
                 effect,
             } => {
-                if *effect != DataEffect::Read {
-                    self.out.push_str(effect.as_str());
+                if (*effect).unwrap_or(DataEffect::Read) != DataEffect::Read {
+                    self.out.push_str((*effect).unwrap_or(DataEffect::Read).as_str());
                     self.out.push(' ');
                 }
                 self.expr_at(receiver, 0, indent);
@@ -1209,10 +1209,10 @@ fn inline_callee(callee: &Callee) -> Option<String> {
             method,
             effect,
         } => {
-            let prefix = if *effect == DataEffect::Read {
+            let prefix = if (*effect).unwrap_or(DataEffect::Read) == DataEffect::Read {
                 String::new()
             } else {
-                format!("{} ", effect.as_str())
+                format!("{} ", (*effect).unwrap_or(DataEffect::Read).as_str())
             };
             Some(format!("{prefix}{}.{}", inline_expr(receiver)?, method))
         }
@@ -1233,7 +1233,7 @@ fn receiver_call_segments(expr: &Expr) -> Option<ReceiverCallSegments<'_>> {
         ..
     } = current
     {
-        if *effect != DataEffect::Read {
+        if (*effect).unwrap_or(DataEffect::Read) != DataEffect::Read {
             return None;
         }
         segments.push((method.as_str(), args.as_slice()));
diff --git a/crates/rsscript/src/hir.rs b/crates/rsscript/src/hir.rs
@@ -1862,7 +1862,7 @@ fn lower_hir_expr(
                         receiver, effect, ..
                     } => Some(HirCallReceiver {
                         value: Box::new(lower_hir_expr(hir, function_name, receiver, value_types)),
-                        effect: param_effect_from_data_effect(*effect),
+                        effect: param_effect_from_data_effect((*effect).unwrap_or(DataEffect::Read)),
                         type_name: receiver_type,
                         resolved_namespace,
                     }),
@@ -3384,7 +3384,7 @@ fn callee_display(callee: &Callee) -> String {
             effect,
         } => format!(
             "{} {}.{method}",
-            effect.as_str(),
+            (*effect).map(|e| e.as_str()).unwrap_or("read"),
             receiver_call_label(receiver)
         ),
     }
diff --git a/crates/rsscript/src/package/review.rs b/crates/rsscript/src/package/review.rs
@@ -1821,7 +1821,7 @@ fn callee_label(callee: &Callee) -> String {
             effect,
         } => format!(
             "{} {}.{method}",
-            effect.as_str(),
+            (*effect).map(|e| e.as_str()).unwrap_or("read"),
             package_expr_label(receiver)
         ),
     }
diff --git a/crates/rsscript/src/review.rs b/crates/rsscript/src/review.rs
@@ -1451,21 +1451,21 @@ fn collect_review_map_facts_expr(
                             facts.receiver_calls.push(ReviewMapReceiverCall {
                                 line: span.line,
                                 column: span.column,
-                                source: format!("{} {receiver_label}.{method}", effect.as_str()),
+                                source: format!("{} {receiver_label}.{method}", (*effect).map(|e| e.as_str()).unwrap_or("read")),
                                 canonical_callee: namespace
                                     .map(|namespace| format!("{namespace}.{method}"))
                                     .unwrap_or_else(|| format!("<unresolved>.{method}")),
-                                self_effect: effect.as_str().to_string(),
+                                self_effect: (*effect).map(|e| e.as_str()).unwrap_or("read").to_string(),
                                 resolution: receiver_call_resolution_label(&resolution).to_string(),
                             });
                             resolution
                         } else {
                             facts.receiver_calls.push(ReviewMapReceiverCall {
                                 line: span.line,
                                 column: span.column,
-                                source: format!("{} {receiver_label}.{method}", effect.as_str()),
+                                source: format!("{} {receiver_label}.{method}", (*effect).map(|e| e.as_str()).unwrap_or("read")),
                                 canonical_callee: format!("<unresolved>.{method}"),
-                                self_effect: effect.as_str().to_string(),
+                                self_effect: (*effect).map(|e| e.as_str()).unwrap_or("read").to_string(),
                                 resolution: "unknown".to_string(),
                             });
                             CallResolution::Unknown
@@ -2443,7 +2443,7 @@ fn review_callee_display(callee: &Callee) -> String {
             effect,
         } => format!(
             "{} {}.{method}",
-            effect.as_str(),
+            (*effect).map(|e| e.as_str()).unwrap_or("read"),
             review_expr_label(receiver)
         ),
     }
diff --git a/crates/rsscript/src/rust_lower/helpers.rs b/crates/rsscript/src/rust_lower/helpers.rs
@@ -812,7 +812,7 @@ pub(super) fn collect_mutated_bindings_from_expr(expr: &Expr, names: &mut BTreeS
             if let Callee::ReceiverCall {
                 receiver, effect, ..
             } = callee
-                && *effect == DataEffect::Mut
+                && (*effect) == Some(DataEffect::Mut)
                 && let Some(root) = expr_root_name(receiver)
             {
                 names.insert(root.to_string());
diff --git a/crates/rsscript/src/rust_lower/lowerer.rs b/crates/rsscript/src/rust_lower/lowerer.rs
@@ -2650,7 +2650,7 @@ impl<'a> RustLowerer<'a> {
                         })
                         .unwrap_or(receiver_type_root);
                     let is_protocol = self.protocol_names.contains(&namespace);
-                    let lowered_receiver = match effect {
+                    let lowered_receiver = match (*effect).unwrap_or(DataEffect::Read) {
                         DataEffect::Mut
                             if let Expr::Ident(receiver_name, _) = receiver.as_ref()
                                 && self.param_effects.get(receiver_name)
@@ -3420,7 +3420,7 @@ impl<'a> RustLowerer<'a> {
             Expr::Call {
                 callee:
                     Callee::ReceiverCall {
-                        effect: DataEffect::Read,
+                        effect: Some(DataEffect::Read) | None,
                         method,
                         ..
                     },
diff --git a/crates/rsscript/src/syntax/ast.rs b/crates/rsscript/src/syntax/ast.rs
@@ -732,10 +732,16 @@ pub enum Callee {
     },
     /// Receiver-call shorthand: `<effect> receiver.method(args)`
     /// Desugars to `Type.method(self: <effect> receiver, args)`.
+    /// `effect` is `None` for a bare call (`x.m()`, defaulting to read of the
+    /// receiver) and `Some(..)` when an effect keyword was written explicitly
+    /// (`read x.m()` / `mut x.m()`). The distinction matters for module-qualified
+    /// calls: a bare `m.fn()` is a value call, while `read m.fn()` borrows the
+    /// result. Consumers that just need the receiver borrow use
+    /// `effect.unwrap_or(DataEffect::Read)`.
     ReceiverCall {
         receiver: Box<Expr>,
         method: String,
-        effect: DataEffect,
+        effect: Option<DataEffect>,
     },
 }
 
diff --git a/crates/rsscript/src/syntax/module_isolation.rs b/crates/rsscript/src/syntax/module_isolation.rs
@@ -611,18 +611,17 @@ impl Resolver {
                 }
             }
             Expr::Call { callee, args, span } => {
-                // `<effect> module.fn(args)` parses as a receiver call, but when the
-                // receiver names a module it is a module-qualified free call. The
-                // effect was written for the *call*, not the module, so convert it
-                // and re-apply the effect to the whole call: `read m.fn()` becomes
-                // `read (m__fn())`, identical to `read flat()` and `read (m.fn())`.
-                // A receiver call always carries an explicit effect (bare module
-                // calls parse as `Callee::Qualified`, handled below), so this never
-                // affects value-position calls.
+                // `read m.fn(args)` parses as a receiver call carrying an EXPLICIT
+                // effect; for a module free call (no receiver to borrow) the effect
+                // was meant for the call result, so convert and re-apply it:
+                // `read m.fn()` -> `read (m__fn())`. A BARE `m.fn()` has effect
+                // `None` (default read of a non-existent receiver) and must stay a
+                // VALUE call — wrapping it in `Expr::Effect` lowered it to a borrow
+                // `&T` and broke value-position calls like `let v = m.fn()`.
                 if let Callee::ReceiverCall {
                     receiver,
                     method,
-                    effect,
+                    effect: Some(effect),
                 } = callee
                     && let Some(prefix) = module_path_of_receiver(receiver, scope)
                     && self
@@ -648,6 +647,9 @@ impl Resolver {
                     };
                     return;
                 }
+                // Bare module receiver call (effect None) and everything else:
+                // `rewrite_callee` collapses a module receiver call to the plain
+                // mangled free call by value.
                 self.rewrite_callee(callee, file, scope);
                 for arg in args {
                     self.rewrite_expr(&mut arg.value, file, scope);
diff --git a/crates/rsscript/src/syntax/parser.rs b/crates/rsscript/src/syntax/parser.rs
@@ -2928,7 +2928,7 @@ fn parse_expr(tokens: &[Token], start: usize, end: usize) -> Option<Expr> {
     }
 
     if let Some(receiver_call) =
-        parse_receiver_call_expr_from_receiver(tokens, start, end, DataEffect::Read)
+        parse_receiver_call_expr_from_receiver(tokens, start, end, None)
     {
         return Some(receiver_call);
     }
@@ -3704,14 +3704,15 @@ fn parse_receiver_call_expr(
     end: usize,
     effect: DataEffect,
 ) -> Option<Expr> {
-    parse_receiver_call_expr_from_receiver(tokens, start + 1, end, effect)
+    // Reached only when an effect keyword was written explicitly.
+    parse_receiver_call_expr_from_receiver(tokens, start + 1, end, Some(effect))
 }
 
 fn parse_receiver_call_expr_from_receiver(
     tokens: &[Token],
     receiver_start: usize,
     end: usize,
-    effect: DataEffect,
+    effect: Option<DataEffect>,
 ) -> Option<Expr> {
     if tokens
         .get(receiver_start)
diff --git a/crates/rsscript/tests/checker_lowering/misc.rs b/crates/rsscript/tests/checker_lowering/misc.rs
@@ -1110,3 +1110,46 @@ native fn parse(text: read String) -> Result<fresh JsonValue, JsonError>
         diagnostic.code == "RS0015" && diagnostic.label == "unsupported top-level item"
     }));
 }
+
+#[test]
+fn value_position_module_call_lowers_by_value_not_borrow() {
+    // Regression: a bare `m.fn()` parses as a receiver call, but a module free
+    // call has no receiver to borrow. It must lower to a VALUE call so the result
+    // can be passed to a by-value parameter. Previously it was wrapped in an
+    // effect and lowered to `&(m__make(..))`, which rustc rejected (E0308,
+    // expected `i64` found `&i64`) only at `rss run` — `rss check` missed it.
+    let sources = vec![
+        (
+            "m.rss".to_string(),
+            "module m\n\nfn make(x: Int) -> Int {\n    return x + 1\n}\n".to_string(),
+        ),
+        (
+            "main.rss".to_string(),
+            concat!(
+                "module app\n\n",
+                "fn consume(y: Int) -> Int {\n    return y * 2\n}\n\n",
+                "fn use_it(a: Int) -> Int {\n",
+                "    let v = m.make(x: a)\n",
+                "    return consume(y: v)\n",
+                "}\n\n",
+                "fn main() -> Unit {\n    return Unit\n}\n",
+            )
+            .to_string(),
+        ),
+    ];
+    let package =
+        lower_sources_to_rust_package_with_options(&sources, "ns-modcall", "/rt", &[], &[])
+            .expect("value-position module call should lower");
+    // The module call collapses to the mangled free call...
+    assert!(
+        package.lib_rs.contains("m__make("),
+        "module call should lower to the mangled free symbol:\n{}",
+        package.lib_rs
+    );
+    // ...by VALUE: it must NOT be wrapped in a borrow.
+    assert!(
+        !package.lib_rs.contains("&(m__make("),
+        "value-position module call must not lower to a borrow `&(m__make(..))`:\n{}",
+        package.lib_rs
+    );
+}

Original file line number	Diff line number	Diff line change
`@@ -6250,7 +6250,7 @@ fn callee_display(callee: &Callee) -> String {`
`6250`	`6250`	`effect,`
`6251`	`6251`	`} => format!(`
`6252`	`6252`	`"{} {}.{method}",`
`6253`		`- effect.as_str(),`
	`6253`	`+ (*effect).unwrap_or(DataEffect::Read).as_str(),`
`6254`	`6254`	`analyzer_expr_label(receiver)`
`6255`	`6255`	`),`
`6256`	`6256`	`}`
Original file line number	Diff line number	Diff line change
`@@ -3355,7 +3355,7 @@ fn body_callee_display(callee: &Callee) -> String {`
`3355`	`3355`	`receiver,`
`3356`	`3356`	`method,`
`3357`	`3357`	`effect,`
`3358`		`- } => format!("{} {}.{method}", effect.as_str(), body_expr_label(receiver)),`
	`3358`	`+ } => format!("{} {}.{method}", (*effect).unwrap_or(DataEffect::Read).as_str(), body_expr_label(receiver)),`
`3359`	`3359`	`}`
`3360`	`3360`	`}`
`3361`	`3361`
Original file line number	Diff line number	Diff line change
`@@ -1441,7 +1441,7 @@ fn callee_display(callee: &Callee) -> String {`
`1441`	`1441`	`effect,`
`1442`	`1442`	`} => format!(`
`1443`	`1443`	`"{} {}.{method}",`
`1444`		`- effect.as_str(),`
	`1444`	`+ (*effect).map(\|e\| e.as_str()).unwrap_or("read"),`
`1445`	`1445`	`local_expr_label(receiver)`
`1446`	`1446`	`),`
`1447`	`1447`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1821,7 +1821,7 @@ fn callee_label(callee: &Callee) -> String {`
`1821`	`1821`	`effect,`
`1822`	`1822`	`} => format!(`
`1823`	`1823`	`"{} {}.{method}",`
`1824`		`- effect.as_str(),`
	`1824`	`+ (*effect).map(\|e\| e.as_str()).unwrap_or("read"),`
`1825`	`1825`	`package_expr_label(receiver)`
`1826`	`1826`	`),`
`1827`	`1827`	`}`
Original file line number	Diff line number	Diff line change
`@@ -812,7 +812,7 @@ pub(super) fn collect_mutated_bindings_from_expr(expr: &Expr, names: &mut BTreeS`
`812`	`812`	`if let Callee::ReceiverCall {`
`813`	`813`	`receiver, effect, ..`
`814`	`814`	`} = callee`
`815`		`- && *effect == DataEffect::Mut`
	`815`	`+ && (*effect) == Some(DataEffect::Mut)`
`816`	`816`	`&& let Some(root) = expr_root_name(receiver)`
`817`	`817`	`{`
`818`	`818`	`names.insert(root.to_string());`