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.
| 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.
|
|
||
| %%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.