Skip to content

Commit ce7f09c

Browse files
zthtsnobipCopilot
authored
PoC of let? (#7582)
* PoC of let.unwrap * support let unwrap syntax (`let?`) (#7586) * support let unwrap syntax (`let?`) * fix printing of let? * fix loc and put error case first for better errors * changed test output * fix * support let? on Error and None as well * add feature for shipping experimental features * error test for not enabled feature * fix * error handling * move to more correct place * comment * tweaks * Update rewatch/src/config.rs Co-authored-by: Copilot <[email protected]> * hint about when appropriate * fix * add another error message test * fix lint * try to improve error message * more work on error messages * changelog --------- Co-authored-by: Paul Tsnobiladzé <[email protected]> Co-authored-by: Copilot <[email protected]>
1 parent 42dfb71 commit ce7f09c

File tree

54 files changed

+1048
-38
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+1048
-38
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
#### :rocket: New Feature
2222

2323
- Add support for ArrayBuffer and typed arrays to `@unboxed`. https://github.com/rescript-lang/rescript/pull/7788
24+
- Experimental: Add `let?` syntax for unwrapping and propagating errors/none as early returns for option/result types. https://github.com/rescript-lang/rescript/pull/7582
25+
- Add support for shipping features as experimental, including configuring what experimental features are enabled in `rescript.json`. https://github.com/rescript-lang/rescript/pull/7582
2426

2527
#### :bug: Bug fix
2628

compiler/bsc/rescript_compiler_main.ml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,10 @@ let command_line_flags : (string * Bsc_args.spec * string) array =
393393
( "-absname",
394394
set absname,
395395
"*internal* Show absolute filenames in error messages" );
396+
( "-enable-experimental",
397+
string_call Experimental_features.enable_from_string,
398+
"Enable experimental features: repeatable, e.g. -enable-experimental \
399+
LetUnwrap" );
396400
(* Not used, the build system did the expansion *)
397401
( "-bs-no-bin-annot",
398402
clear Clflags.binary_annotations,

compiler/frontend/ast_attributes.ml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,12 @@ let has_bs_optional (attrs : t) : bool =
199199
true
200200
| _ -> false)
201201

202+
let has_unwrap_attr (attrs : t) : bool =
203+
Ext_list.exists attrs (fun ({txt}, _) ->
204+
match txt with
205+
| "let.unwrap" -> true
206+
| _ -> false)
207+
202208
let iter_process_bs_int_as (attrs : t) =
203209
let st = ref None in
204210
Ext_list.iter attrs (fun (({txt; loc}, payload) as attr) ->

compiler/frontend/ast_attributes.mli

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ val iter_process_bs_string_as : t -> string option
4646

4747
val has_bs_optional : t -> bool
4848

49+
val has_unwrap_attr : t -> bool
50+
4951
val iter_process_bs_int_as : t -> int option
5052

5153
type as_const_payload = Int of int | Str of string * External_arg_spec.delim

compiler/frontend/bs_builtin_ppx.ml

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,124 @@ let expr_mapper ~async_context ~in_function_def (self : mapper)
143143
] ) ->
144144
default_expr_mapper self
145145
{e with pexp_desc = Pexp_ifthenelse (b, t_exp, Some f_exp)}
146+
(* Transform:
147+
- `@let.unwrap let Ok(inner_pat) = expr`
148+
- `@let.unwrap let Error(inner_pat) = expr`
149+
- `@let.unwrap let Some(inner_pat) = expr`
150+
- `@let.unwrap let None = expr`
151+
...into switches *)
152+
| Pexp_let
153+
( Nonrecursive,
154+
[
155+
{
156+
pvb_pat =
157+
{
158+
ppat_desc =
159+
( Ppat_construct
160+
({txt = Lident ("Ok" as variant_name)}, Some _)
161+
| Ppat_construct
162+
({txt = Lident ("Error" as variant_name)}, Some _)
163+
| Ppat_construct
164+
({txt = Lident ("Some" as variant_name)}, Some _)
165+
| Ppat_construct
166+
({txt = Lident ("None" as variant_name)}, None) );
167+
} as pvb_pat;
168+
pvb_expr;
169+
pvb_attributes;
170+
};
171+
],
172+
body )
173+
when Ast_attributes.has_unwrap_attr pvb_attributes -> (
174+
if not (Experimental_features.is_enabled Experimental_features.LetUnwrap)
175+
then
176+
Bs_syntaxerr.err pvb_pat.ppat_loc
177+
(Experimental_feature_not_enabled LetUnwrap);
178+
let variant : [`Result_Ok | `Result_Error | `Option_Some | `Option_None] =
179+
match variant_name with
180+
| "Ok" -> `Result_Ok
181+
| "Error" -> `Result_Error
182+
| "Some" -> `Option_Some
183+
| _ -> `Option_None
184+
in
185+
match pvb_expr.pexp_desc with
186+
| Pexp_pack _ -> default_expr_mapper self e
187+
| _ ->
188+
let cont_case =
189+
{
190+
Parsetree.pc_bar = None;
191+
pc_lhs = pvb_pat;
192+
pc_guard = None;
193+
pc_rhs = body;
194+
}
195+
in
196+
let loc = {pvb_pat.ppat_loc with loc_ghost = true} in
197+
let early_case =
198+
match variant with
199+
(* Result: continue on Ok(_), early-return on Error(e) *)
200+
| `Result_Ok ->
201+
{
202+
Parsetree.pc_bar = None;
203+
pc_lhs =
204+
Ast_helper.Pat.alias
205+
(Ast_helper.Pat.construct ~loc
206+
{txt = Lident "Error"; loc}
207+
(Some (Ast_helper.Pat.any ~loc ())))
208+
{txt = "e"; loc};
209+
pc_guard = None;
210+
pc_rhs = Ast_helper.Exp.ident ~loc {txt = Lident "e"; loc};
211+
}
212+
(* Result: continue on Error(_), early-return on Ok(x) *)
213+
| `Result_Error ->
214+
{
215+
Parsetree.pc_bar = None;
216+
pc_lhs =
217+
Ast_helper.Pat.alias
218+
(Ast_helper.Pat.construct ~loc {txt = Lident "Ok"; loc}
219+
(Some (Ast_helper.Pat.any ~loc ())))
220+
{txt = "x"; loc};
221+
pc_guard = None;
222+
pc_rhs = Ast_helper.Exp.ident ~loc {txt = Lident "x"; loc};
223+
}
224+
(* Option: continue on Some(_), early-return on None *)
225+
| `Option_Some ->
226+
{
227+
Parsetree.pc_bar = None;
228+
pc_lhs =
229+
Ast_helper.Pat.alias
230+
(Ast_helper.Pat.construct ~loc {txt = Lident "None"; loc} None)
231+
{txt = "x"; loc};
232+
pc_guard = None;
233+
pc_rhs = Ast_helper.Exp.ident ~loc {txt = Lident "x"; loc};
234+
}
235+
(* Option: continue on None, early-return on Some(x) *)
236+
| `Option_None ->
237+
{
238+
Parsetree.pc_bar = None;
239+
pc_lhs =
240+
Ast_helper.Pat.alias
241+
(Ast_helper.Pat.construct ~loc {txt = Lident "Some"; loc}
242+
(Some (Ast_helper.Pat.any ~loc ())))
243+
{txt = "x"; loc};
244+
pc_guard = None;
245+
pc_rhs = Ast_helper.Exp.ident ~loc {txt = Lident "x"; loc};
246+
}
247+
in
248+
default_expr_mapper self
249+
{
250+
e with
251+
pexp_desc = Pexp_match (pvb_expr, [early_case; cont_case]);
252+
pexp_attributes = e.pexp_attributes @ pvb_attributes;
253+
})
254+
| Pexp_let (_, [{pvb_pat; pvb_attributes}], _)
255+
when Ast_attributes.has_unwrap_attr pvb_attributes ->
256+
(* Catch all unsupported cases for `let?` *)
257+
if not (Experimental_features.is_enabled Experimental_features.LetUnwrap)
258+
then
259+
Bs_syntaxerr.err pvb_pat.ppat_loc
260+
(Experimental_feature_not_enabled LetUnwrap)
261+
else
262+
Bs_syntaxerr.err pvb_pat.ppat_loc
263+
(LetUnwrap_not_supported_in_position `Unsupported_type)
146264
| Pexp_let
147265
( Nonrecursive,
148266
[
@@ -333,6 +451,24 @@ let signature_item_mapper (self : mapper) (sigi : Parsetree.signature_item) :
333451
let structure_item_mapper (self : mapper) (str : Parsetree.structure_item) :
334452
Parsetree.structure_item =
335453
match str.pstr_desc with
454+
| Pstr_value (_, vbs)
455+
when List.exists
456+
(fun (vb : Parsetree.value_binding) ->
457+
Ast_attributes.has_unwrap_attr vb.pvb_attributes)
458+
vbs ->
459+
let vb =
460+
List.find
461+
(fun (vb : Parsetree.value_binding) ->
462+
Ast_attributes.has_unwrap_attr vb.pvb_attributes)
463+
vbs
464+
in
465+
if not (Experimental_features.is_enabled Experimental_features.LetUnwrap)
466+
then
467+
Bs_syntaxerr.err vb.pvb_pat.ppat_loc
468+
(Experimental_feature_not_enabled LetUnwrap)
469+
else
470+
Bs_syntaxerr.err vb.pvb_pat.ppat_loc
471+
(LetUnwrap_not_supported_in_position `Toplevel)
336472
| Pstr_type (rf, tdcls) (* [ {ptype_attributes} as tdcl ] *) ->
337473
Ast_tdcls.handle_tdcls_in_stru self str rf tdcls
338474
| Pstr_primitive prim

compiler/frontend/bs_syntaxerr.ml

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ type error =
4747
| Misplaced_label_syntax
4848
| Optional_in_uncurried_bs_attribute
4949
| Bs_this_simple_pattern
50+
| Experimental_feature_not_enabled of Experimental_features.feature
51+
| LetUnwrap_not_supported_in_position of [`Toplevel | `Unsupported_type]
5052

5153
let pp_error fmt err =
5254
Format.pp_print_string fmt
@@ -82,7 +84,19 @@ let pp_error fmt err =
8284
each constructor must have an argument."
8385
| Conflict_ffi_attribute str -> "Conflicting attributes: " ^ str
8486
| Bs_this_simple_pattern ->
85-
"%@this expect its pattern variable to be simple form")
87+
"%@this expect its pattern variable to be simple form"
88+
| Experimental_feature_not_enabled feature ->
89+
Printf.sprintf
90+
"Experimental feature not enabled: %s. Enable it by setting \"%s\" to \
91+
true under \"experimentalFeatures\" in rescript.json."
92+
(Experimental_features.to_string feature)
93+
(Experimental_features.to_string feature)
94+
| LetUnwrap_not_supported_in_position hint -> (
95+
match hint with
96+
| `Toplevel -> "`let?` is not allowed for top-level bindings."
97+
| `Unsupported_type ->
98+
"`let?` is only supported in let bindings targeting the `result` or \
99+
`option` type."))
86100

87101
type exn += Error of Location.t * error
88102

compiler/frontend/bs_syntaxerr.mli

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ type error =
4747
| Misplaced_label_syntax
4848
| Optional_in_uncurried_bs_attribute
4949
| Bs_this_simple_pattern
50+
| Experimental_feature_not_enabled of Experimental_features.feature
51+
| LetUnwrap_not_supported_in_position of [`Toplevel | `Unsupported_type]
5052

5153
val err : Location.t -> error -> 'a
5254

compiler/ml/error_message_utils.ml

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ type type_clash_context =
9696
| IfReturn
9797
| TernaryReturn
9898
| SwitchReturn
99+
| LetUnwrapReturn
99100
| TryReturn
100101
| StringConcat
101102
| ComparisonOperator
@@ -131,6 +132,7 @@ let context_to_string = function
131132
| Some TernaryReturn -> "TernaryReturn"
132133
| Some Await -> "Await"
133134
| Some BracedIdent -> "BracedIdent"
135+
| Some LetUnwrapReturn -> "LetUnwrapReturn"
134136
| None -> "None"
135137
136138
let fprintf = Format.fprintf
@@ -163,6 +165,8 @@ let error_expected_type_text ppf type_clash_context =
163165
| Some ComparisonOperator ->
164166
fprintf ppf "But it's being compared to something of type:"
165167
| Some SwitchReturn -> fprintf ppf "But this switch is expected to return:"
168+
| Some LetUnwrapReturn ->
169+
fprintf ppf "But this @{<info>let?@} is used where this type is expected:"
166170
| Some TryReturn -> fprintf ppf "But this try/catch is expected to return:"
167171
| Some WhileCondition ->
168172
fprintf ppf "But a @{<info>while@} loop condition must always be of type:"
@@ -314,6 +318,65 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env loc ppf
314318
"\n\n\
315319
\ All branches in a @{<info>switch@} must return the same type.@,\
316320
To fix this, change your branch to return the expected type."
321+
| Some LetUnwrapReturn, bottom_aliases -> (
322+
let kind =
323+
match bottom_aliases with
324+
| Some ({Types.desc = Tconstr (p, _, _)}, _)
325+
when Path.same p Predef.path_option ->
326+
`Option
327+
| Some (_, {Types.desc = Tconstr (p, _, _)})
328+
when Path.same p Predef.path_option ->
329+
`Option
330+
| Some ({Types.desc = Tconstr (p, _, _)}, _)
331+
when Path.same p Predef.path_result ->
332+
`Result
333+
| Some (_, {Types.desc = Tconstr (p, _, _)})
334+
when Path.same p Predef.path_result ->
335+
`Result
336+
| _ -> `Unknown
337+
in
338+
match kind with
339+
| `Option ->
340+
fprintf ppf
341+
"\n\n\
342+
\ This @{<info>let?@} unwraps an @{<info>option@}; use it where the \
343+
enclosing function or let binding returns an @{<info>option@} so \
344+
@{<info>None@} can propagate.\n\n\
345+
\ Possible solutions:\n\
346+
\ - Change the enclosing function or let binding to return \
347+
@{<info>option<'t>@} and use @{<info>Some@} for success; \
348+
@{<info>let?@} will propagate @{<info>None@}.\n\
349+
\ - Replace @{<info>let?@} with a @{<info>switch@} and handle the \
350+
@{<info>None@} case explicitly.\n\
351+
\ - If you want a default value instead of early return, unwrap using \
352+
@{<info>Option.getOr(default)@}."
353+
| `Result ->
354+
fprintf ppf
355+
"\n\n\
356+
\ This @{<info>let?@} unwraps a @{<info>result@}; use it where the \
357+
enclosing function or let binding returns a @{<info>result@} so \
358+
@{<info>Error@} can propagate.\n\n\
359+
\ Possible solutions:\n\
360+
\ - Change the enclosing function or let binding to return \
361+
@{<info>result<'ok, 'error>@}; use @{<info>Ok@} for success, and \
362+
@{<info>let?@} will propagate @{<info>Error@}.\n\
363+
\ - Replace @{<info>let?@} with a @{<info>switch@} and handle the \
364+
@{<info>Error@} case explicitly.\n\
365+
\ - If you want a default value instead of early return, unwrap using \
366+
@{<info>Result.getOr(default)@}."
367+
| `Unknown ->
368+
fprintf ppf
369+
"\n\n\
370+
\ @{<info>let?@} can only be used in a context that expects \
371+
@{<info>option@} or @{<info>result@}.\n\n\
372+
\ Possible solutions:\n\
373+
\ - Change the enclosing function or let binding to return an \
374+
@{<info>option<'t>@} or @{<info>result<'ok, 'error>@} and propagate \
375+
with @{<info>Some/Ok@}.\n\
376+
\ - Replace @{<info>let?@} with a @{<info>switch@} and handle the \
377+
@{<info>None/Error@} case explicitly.\n\
378+
\ - If you want a default value instead of early return, unwrap using \
379+
@{<info>Option.getOr(default)@} or @{<info>Result.getOr(default)@}.")
317380
| Some TryReturn, _ ->
318381
fprintf ppf
319382
"\n\n\

compiler/ml/experimental_features.ml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
type feature = LetUnwrap
2+
3+
let to_string (f : feature) : string =
4+
match f with
5+
| LetUnwrap -> "LetUnwrap"
6+
7+
let from_string (s : string) : feature option =
8+
match s with
9+
| "LetUnwrap" -> Some LetUnwrap
10+
| _ -> None
11+
12+
module FeatureSet = Set.Make (struct
13+
type t = feature
14+
let compare = compare
15+
end)
16+
17+
let enabled_features : FeatureSet.t ref = ref FeatureSet.empty
18+
let enable_from_string (s : string) =
19+
match from_string s with
20+
| Some f -> enabled_features := FeatureSet.add f !enabled_features
21+
| None -> ()
22+
23+
let is_enabled (f : feature) = FeatureSet.mem f !enabled_features

compiler/ml/experimental_features.mli

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
type feature = LetUnwrap
2+
3+
val enable_from_string : string -> unit
4+
val is_enabled : feature -> bool
5+
val to_string : feature -> string

0 commit comments

Comments
 (0)