Incorrect polymorphic variant -> string conversion

[yawaramin]

Input

external ice_cream :
  ?flavour:([`vanilla | `chocolate] [@bs.string]) ->
  num_scoops:int ->
  _ =
  "" [@@bs.obj]
  
let my_scoop = ice_cream ~flavour:`vanilla ~num_scoops:1

Output

var my_scoop = {
  flavour: /* vanilla */256918907,
  num: 1
};

Expected Output

flavour: "vanilla"

Further Comments

Also, ice_cream named parameter num_scoops got converted into output object key num. This may be out of scope for this issue.

Finally, if we make flavour non-optional, we get an error Line 1, 0: bs.obj label flavour does not support such arg type.

[bobzhang]

@yawaramin about num_scoops -> num, see discussions in #999 (we may choose __ for name mangling in the future, _ might be too confusing )

[OvermindDL1]

__ should be used for name mangling yeah, _ is too often used as a separator.

[bobzhang]

the reason why we disable the combination of bs.obj and bs.string is that the result object type does not make sense unless we make it abstract type.
So here

• either we do the same thing as label argument, disable the combination

• we allow such combination but force the result object type to be abstract type
  why is it necessary?

    external mk : ~a:([`a|`b] [@bs.obj]) -> unit -> _ = "" [@@bs.obj]
    let u = mk ~a:`a  
  

  if u is not an abstract type, it would be < a : [a|b] [@bs.obj] > Js.t
  then we can access u##a which is a string in runtime, but the type system tells
  it is a variant.
  If we do a sanity check to ensure that the result object type is abstract type, then
  it is sound, since the user has to check all externals are sound

[bobzhang]

think twice, I think we can achieve this by changing the algorithm of how to synthesize the output type

[bobzhang]

fixed in master, so you can use bs.string with bs.obj now

[yawaramin]

Hi Bob, the problem seems to persist even after upgrading to 1.4.1.

I am declaring a Intl.Date_time_format.Options.t abstract type and a make function for it: https://github.com/yawaramin/bucklescript-cyclejs-test/blob/f8074409794bb0d8d3952ef8d5fa857add7ce692/src/intl_date_time_format.ml#L10

Calling the Intl.Date_time_format.Options.make function: https://github.com/yawaramin/bucklescript-cyclejs-test/blob/f8074409794bb0d8d3952ef8d5fa857add7ce692/src/comment.ml#L28

And getting the same kind of numeric output as before: https://github.com/yawaramin/bucklescript-cyclejs-test/blob/f8074409794bb0d8d3952ef8d5fa857add7ce692/lib/js/src/comment.js#L21

This time no warnings about any unused [@bs.string] attribute though.

[bobzhang]

Hi, can you double check the version? It's fixed in master, 1.4.2, but I did not make a release yet, due to some bugs on windows

[yawaramin]

Oh! I misunderstood. Will wait for the release and check again then :-)