lazy_t cleanup part 2 #7484
lazy_t cleanup part 2 #7484
Conversation
|
@cristianoc Also it causes this error (see CI failure): even when I make line 30 instead. |
| decl_abstr with | ||
| type_params = [tvar]; | ||
| type_arity = 1; | ||
| type_variance = [Variance.covariant]; |
There was a problem hiding this comment.
I think this is the difference: type lazy_t<+'a> with the +: covariant.
| @@ -66,16 +66,6 @@ let init_shape modl = | |||
| non_const = cstr_non_const; | |||
| attrs = []; | |||
| } ) | |||
| | {desc = Tconstr (p, _, _)} when Path.same p Predef.path_lazy_t -> | |||
There was a problem hiding this comment.
This is what would cause the recursive module test to fail.
I think that's fine, unless the recursive module example - with lazy - pattern is important to support.
| @@ -34,10 +34,10 @@ type t<'a> = { | |||
|
|
|||
| %%private(external fnToVal: (unit => 'a) => 'a = "%identity") | |||
| %%private(external valToFn: 'a => unit => 'a = "%identity") | |||
| %%private(external castToConcrete: lazy_t<'a> => t<'a> = "%identity") | |||
There was a problem hiding this comment.
I don't think we can make lazy_t and t the same here, because lazy_t needs to be covariant and t is mutable. Also, the naming might change for clarity: currently lazy_t represents the type Lazy.t while t represents an internal implementation-only type used to generate specific js code (and cannot be exported as it would be unsound to export the mutable part anyway).
There was a problem hiding this comment.
Overall, bringing lazy_t out of builtins seems to bring clarity, as it makes a few things more explicit.
|
Here's a quick set of changes, minus cleanup: |
|
@cristianoc Thanks a lot, PR updated. |
rescript
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/darwin-x64
@rescript/linux-x64
@rescript/win32-x64
commit: |
No description provided.