Skip to content

type specialisation for options does not work with externals #4930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bobzhang opened this issue Feb 1, 2021 · 2 comments
Closed

type specialisation for options does not work with externals #4930

bobzhang opened this issue Feb 1, 2021 · 2 comments

Comments

@bobzhang
Copy link
Member

bobzhang commented Feb 1, 2021

let f ?x y = 
  (match  x with 
  | None -> 1 
  | Some (_:string) -> 2 ) + y


type t = {
  y : int [@optional]
}[@@deriving abstract]


let u ?y () = t ?y ()
function f(x, y) {
  return (
          x !== undefined ? 2 : 1
        ) + y | 0;
}

function u(y, param) {
  var tmp = {};
  if (y !== undefined) {
    tmp.y = Caml_option.valFromOption(y);
  }
  return tmp;
}

The second one does not get specialized.
Raised here https://forum.rescript-lang.org/t/how-can-i-get-rid-of-the-runtime-libraries/1001/2
Code snippet:

[@react.component]
let make = (
  ~href: string,
  ~className: option(string)=?,
  ~onClick: option(clickHandler)=?,
  ~children: React.element,
) => {
  let onClick = Belt.Option.getWithDefault(onClick, makeDefaultClickHandler(href));

  <a href onClick ?className> children </a>
}

Expanded code:

external makeProps :
  href:string ->
    ?className:string ->
      ?onClick:clickHandler ->
        children:React.element ->
          ?key:string ->
            unit ->
              <
                href: string  ;className: string option  ;onClick: clickHandler
                                                                    option  ;
                children: React.element   >  Js.t = ""
@bobzhang
Copy link
Member Author

bobzhang commented Feb 2, 2021

The difficulty comes from the fact that the snippet as below is instrumented during the runtime to achieve a special semantics

  if (y !== undefined) {
    tmp.y = Caml_option.valFromOption(y);
  }

The metadata of external is collected during the parsing, while it needs type information.
Even if we collect the type information, it is difficult to get it connected to the runtime code generation.

Proposal 1:
Add some attributes like immediate and verify it during type checking, rely on such information to do the code optimisation.
This approach requires more user input and more work from compiler.

Proposal 2:
Ad-hoc solution, assuming the built-in string type can not be re-defined -- seems to be fair and easy to guarantee in the parsing stage. So we know such type can not be option and do such code optimizations. This makes sense if jsx has lot of string optional arguments

@bobzhang
Copy link
Member Author

fixed in #4960

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant