Skip to content

Commit 5d06a7b

Browse files
authored
Fix let? unwrap to use actual variable names instead of hardcoded ones (#8091)
When using `let? Some(myVar) = ...`, the variable name was hardcoded as "x" instead of using the actual pattern variable name "myVar". This fix extracts the variable name from the pattern and uses it in the early return cases, ensuring proper variable propagation and avoiding unnecessary runtime allocations. Fixes #8085 Signed-off-by: [Huijung Yoon] <[[email protected]]>
1 parent 3c2f87e commit 5d06a7b

File tree

3 files changed

+53
-43
lines changed

3 files changed

+53
-43
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
- Fix `@scope` shadowing (rewrite using `globalThis`). https://github.com/rescript-lang/rescript/pull/8100
2929
- Formatter: normalize underscore placeholders in pipe expressions to canonical form (e.g., `a->map2(_, fn)` formats to `a->map2(fn)`). https://github.com/rescript-lang/rescript/pull/8033
3030
- Fix rewatch panic on duplicate module name. https://github.com/rescript-lang/rescript/pull/8102
31+
- Fix `let?` unwrap to use actual variable names from pattern instead of hardcoded "x"/"e". When using `let? Some(myVar) = ...`, the variable name `myVar` is now properly propagated in early returns. https://github.com/rescript-lang/rescript/issues/8085
3132

3233
#### :memo: Documentation
3334

compiler/frontend/bs_builtin_ppx.ml

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,15 @@ let expr_mapper ~async_context ~in_function_def (self : mapper)
216216
}
217217
in
218218
let loc = {pvb_pat.ppat_loc with loc_ghost = true} in
219+
(* Extract the variable name from the pattern (e.g., myVar from Some(myVar)) *)
220+
let var_name =
221+
match pvb_pat.ppat_desc with
222+
| Ppat_construct (_, Some inner_pat) -> (
223+
match Ast_pat.is_single_variable_pattern_conservative inner_pat with
224+
| Some name when name <> "" -> name
225+
| _ -> "x")
226+
| _ -> "x"
227+
in
219228
let early_case =
220229
match variant with
221230
(* Result: continue on Ok(_), early-return on Error(e) *)
@@ -227,9 +236,9 @@ let expr_mapper ~async_context ~in_function_def (self : mapper)
227236
(Ast_helper.Pat.construct ~loc
228237
{txt = Lident "Error"; loc}
229238
(Some (Ast_helper.Pat.any ~loc ())))
230-
{txt = "e"; loc};
239+
{txt = var_name; loc};
231240
pc_guard = None;
232-
pc_rhs = Ast_helper.Exp.ident ~loc {txt = Lident "e"; loc};
241+
pc_rhs = Ast_helper.Exp.ident ~loc {txt = Lident var_name; loc};
233242
}
234243
(* Result: continue on Error(_), early-return on Ok(x) *)
235244
| `Result_Error ->
@@ -239,9 +248,9 @@ let expr_mapper ~async_context ~in_function_def (self : mapper)
239248
Ast_helper.Pat.alias
240249
(Ast_helper.Pat.construct ~loc {txt = Lident "Ok"; loc}
241250
(Some (Ast_helper.Pat.any ~loc ())))
242-
{txt = "x"; loc};
251+
{txt = var_name; loc};
243252
pc_guard = None;
244-
pc_rhs = Ast_helper.Exp.ident ~loc {txt = Lident "x"; loc};
253+
pc_rhs = Ast_helper.Exp.ident ~loc {txt = Lident var_name; loc};
245254
}
246255
(* Option: continue on Some(_), early-return on None *)
247256
| `Option_Some ->
@@ -250,9 +259,9 @@ let expr_mapper ~async_context ~in_function_def (self : mapper)
250259
pc_lhs =
251260
Ast_helper.Pat.alias
252261
(Ast_helper.Pat.construct ~loc {txt = Lident "None"; loc} None)
253-
{txt = "x"; loc};
262+
{txt = var_name; loc};
254263
pc_guard = None;
255-
pc_rhs = Ast_helper.Exp.ident ~loc {txt = Lident "x"; loc};
264+
pc_rhs = Ast_helper.Exp.ident ~loc {txt = Lident var_name; loc};
256265
}
257266
(* Option: continue on None, early-return on Some(x) *)
258267
| `Option_None ->
@@ -262,9 +271,9 @@ let expr_mapper ~async_context ~in_function_def (self : mapper)
262271
Ast_helper.Pat.alias
263272
(Ast_helper.Pat.construct ~loc {txt = Lident "Some"; loc}
264273
(Some (Ast_helper.Pat.any ~loc ())))
265-
{txt = "x"; loc};
274+
{txt = var_name; loc};
266275
pc_guard = None;
267-
pc_rhs = Ast_helper.Exp.ident ~loc {txt = Lident "x"; loc};
276+
pc_rhs = Ast_helper.Exp.ident ~loc {txt = Lident var_name; loc};
268277
}
269278
in
270279
default_expr_mapper self

tests/tests/src/LetUnwrap.mjs

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,19 @@ function doNextStuffWithResult(s) {
3030
}
3131

3232
function getXWithResult(s) {
33-
let e = doStuffWithResult(s);
34-
if (e.TAG !== "Ok") {
35-
return e;
33+
let y = doStuffWithResult(s);
34+
if (y.TAG !== "Ok") {
35+
return y;
3636
}
37-
let y = e._0;
38-
let e$1 = doNextStuffWithResult(y);
39-
if (e$1.TAG === "Ok") {
37+
let y$1 = y._0;
38+
let x = doNextStuffWithResult(y$1);
39+
if (x.TAG === "Ok") {
4040
return {
4141
TAG: "Ok",
42-
_0: e$1._0 + y
42+
_0: x._0 + y$1
4343
};
4444
} else {
45-
return e$1;
45+
return x;
4646
}
4747
}
4848

@@ -67,15 +67,15 @@ function doNextStuffWithOption(s) {
6767
}
6868

6969
function getXWithOption(s) {
70-
let x = doStuffWithOption(s);
71-
if (x === undefined) {
72-
return x;
70+
let y = doStuffWithOption(s);
71+
if (y === undefined) {
72+
return y;
7373
}
74-
let x$1 = doNextStuffWithOption(x);
75-
if (x$1 !== undefined) {
76-
return x$1 + x;
74+
let x = doNextStuffWithOption(y);
75+
if (x !== undefined) {
76+
return x + y;
7777
} else {
78-
return x$1;
78+
return x;
7979
}
8080
}
8181

@@ -115,55 +115,55 @@ async function decodeResAsync(res) {
115115
}
116116

117117
async function getXWithResultAsync(s) {
118-
let e = await doStuffResultAsync(s);
119-
if (e.TAG !== "Ok") {
120-
return e;
118+
let x = await doStuffResultAsync(s);
119+
if (x.TAG !== "Ok") {
120+
return x;
121121
}
122-
let res = e._0;
122+
let res = x._0;
123123
console.log(res.s);
124-
let e$1 = await decodeResAsync(res);
125-
if (e$1.TAG === "Ok") {
124+
let x$1 = await decodeResAsync(res);
125+
if (x$1.TAG === "Ok") {
126126
return {
127127
TAG: "Ok",
128-
_0: e$1._0
128+
_0: x$1._0
129129
};
130130
} else {
131-
return e$1;
131+
return x$1;
132132
}
133133
}
134134

135135
function returnsAliasOnFirstError(s) {
136-
let e = doStuffWithResult(s);
137-
if (e.TAG === "Ok") {
136+
let _y = doStuffWithResult(s);
137+
if (_y.TAG === "Ok") {
138138
return {
139139
TAG: "Ok",
140140
_0: "ok"
141141
};
142142
} else {
143-
return e;
143+
return _y;
144144
}
145145
}
146146

147147
function returnsAliasOnSecondError(s) {
148-
let e = doStuffWithResult(s);
149-
if (e.TAG !== "Ok") {
150-
return e;
148+
let y = doStuffWithResult(s);
149+
if (y.TAG !== "Ok") {
150+
return y;
151151
}
152-
let e$1 = doNextStuffWithResult(e._0);
153-
if (e$1.TAG === "Ok") {
152+
let _x = doNextStuffWithResult(y._0);
153+
if (_x.TAG === "Ok") {
154154
return {
155155
TAG: "Ok",
156156
_0: "ok"
157157
};
158158
} else {
159-
return e$1;
159+
return _x;
160160
}
161161
}
162162

163163
function returnsAliasOnOk(s) {
164-
let x = doStuffWithResult(s);
165-
if (x.TAG === "Ok") {
166-
return x;
164+
let _e = doStuffWithResult(s);
165+
if (_e.TAG === "Ok") {
166+
return _e;
167167
} else {
168168
return {
169169
TAG: "Error",

0 commit comments

Comments
 (0)