Add support for in operator

[DZakh]

No description provided.

[DZakh]

Ready for review

[cometkim]

The JavaScript in operator has the behavior of iterating over the entire prototype of an object.

I'm not 100% sure about its semantics, but we should avoid confusing it with hasOwnProperty.

If it intended to be matched with JS' in semantics, 'toString' in dict{} would return true. I'm not sure this helps in ReScript codebase, or if it's needed for JS FFI.

[cometkim]

If this is just for implementing stdlib, we can use it as an internal primitive (e.g. #has_in) and not expose it to users.

[cometkim]

I just saw #7313

If this is for record types, Object.hasOwn is better, and maybe with this runtime codes

@unboxed
type rec t = Any('a): t

let hasOwn: (t, string) => bool = %raw(
  "Object.hasOwn || Object.bind.call(Object.hasOwnProperty)"
)

let a = hasOwn(Any(dict{}), "toString")
Console.log(a) // => `false`

[DZakh]

If this is just for implementing stdlib, we can use it as an internal primitive (e.g. #has_in) and not expose it to users.

I think it might be useful to expose to users as well for JS compat. I can see cases where it might be useful. Also, the in operator might be unsafe for some cases, but it has the best performance compared to Object.hasOwn and Object.hasOwnProperty. This is important for me in ReScript Schema.

Ideally after this PR is merged to add %has, which will automatically choose between Object.hasOwn and in.

• For example, if prop is toString or __proto__, use Object.hasOwn otherwise use in

[cometkim]

The prototype concept does not exist in the ReScript, so the first-class syntax appears to be confusing. E.g.

type t = {
  toString?: ...
}

If you need it for the purpose of very specific reason, like perf, %raw is solving the problem quite well already, doesn't it?

[cometkim]

Also, the fact that in appears faster than Object.hasOwn may be temporary or host-dependent. The semantics of Object.hasOwn is actually simpler, making it easier to optimize.

The results may change over time. This commonly happens with relatively recently added features. I'd recommend testing it on other engines like Firefox and Safari

[cometkim]

IMO, the in semantics are incompatible with others in ReScript.

I'm fine with adding compiler primitives where they're needed to support some low-level JS codes, but I'd like to avoid adding first-class syntax or promoting it to users.

[DZakh]

@cometkim I've made changes based on your comments. It now uses in operator when we can guarantee safety in compile time and fallbacks to Object.hasOwn in other cases.

Regarding performance currently there's a huge difference in favor of the in operator
https://jsbenchmark.com/#eyJjYXNlcyI6W3siaWQiOiJWdEx6SzlYNG9fejF0WXQwSG43VWIiLCJjb2RlIjoicmV0dXJuIFwiZm9vXCIgaW4gREFUQSIsImRlcGVuZGVuY2llcyI6W10sIm5hbWUiOiJJTiJ9LHsiaWQiOiItcUwyZXh0b2JQTDNsR0lMNjVWQnYiLCJjb2RlIjoicmV0dXJuIE9iamVjdC5oYXNPd24oREFUQSwgXCJmb29cIikiLCJkZXBlbmRlbmNpZXMiOltdLCJuYW1lIjoiT2JqZWN0Lmhhc093biJ9LHsiaWQiOiI0X21oZnhKRUxlSk1HRm9CdzV1TmoiLCJjb2RlIjoicmV0dXJuIERBVEFbXCJmb29cIl0gIT09IHVuZGVmaW5lZCIsImRlcGVuZGVuY2llcyI6W10sIm5hbWUiOiJVbmRlZmluZWQifSx7ImlkIjoiV0Rnb0tSLTJkcnZFTVd3cnJrRGo5IiwiY29kZSI6InJldHVybiBEQVRBLmhhc093blByb3BlcnR5KFwiZm9vXCIpIiwiZGVwZW5kZW5jaWVzIjpbXSwibmFtZSI6Imhhc093blByb3BlcnR5In1dLCJjb25maWciOnsibmFtZSI6IkJhc2ljIGV4YW1wbGUiLCJwYXJhbGxlbCI6dHJ1ZSwiZ2xvYmFsVGVzdENvbmZpZyI6eyJkZXBlbmRlbmNpZXMiOltdfSwiZGF0YUNvZGUiOiJyZXR1cm4ge1xuICBmb286IFwic3RyaW5nXCJcbn0ifX0

In my opinion, Object.hasOwn will never be faster than in because it's a function call anyway, which is difficult to optimize because of the potential prototype polluting.

[cometkim]

Looks good to me.

Record semantics are close to Object.create(null) and it was actually used in the output of the earlier versions. As we're using object literals for compactness now, this would be a minimal safe guard for the codes with external types.

My previous opinion was that it is not general and safe enough to introduce as a syntax. Having an unsafe primitive for emitting in is perfectly. It might be better to prefix with "unsafe"

[cometkim]

constructor too? / __proto__ is so deprecated, no need to cover

[cometkim]

I think usinghasOwnProperty or pritmitive runtime is better to us. Introducing Object.hasOwn would significantly increase the target requirement.

[cometkim]

In my opinion, Object.hasOwn will never be faster than in because it's a function call anyway, which is difficult to optimize because of the potential prototype polluting.

Well, it is easier to optimize by simple inlining. compiler's choice IMO.

[DZakh]

@cometkim After a second thought I actually think that we shouldn't use Object.hasOwn/hasOwnProperty. This doesn't make things any safer, because all the other methods like Dict.get will still behave in the same unsafe way:

https://rescript-lang.org/try?version=v11.1.4&module=esmodule&code=DYUwLgBAhgXBAmBLAxmAPIgdmAfBAvBAFIDOAdACIphkgC2ADmAJ4AUAlAFCelnAD2Ac1ZQAtDl5VUZQeFYAiMPwDKYAE5ZB89lyA

The case:

type t = {
  toString?: ...
}

It is legit, but it's broken even now. And it's possible to fix it with the Object.hasOwn I added, but I think it should only applied to the records pattern matching and not to Dict.has. I strongly feel that Dict.has should always use in operator.

[DZakh]

If you need it for the purpose of very specific reason, like perf, %raw is solving the problem quite well already, doesn't it?

The problem with %raw that it adds a function call, which is critical when you try to optimise for jit compiler.

[DZakh]

My point is that it's meaningless to worry about in safety when Dict.get and other main methods are similarly unsafe.

[cometkim]

After a second thought I actually think that we shouldn't use Object.hasOwn/hasOwnProperty. This doesn't make things any safer, because all the other methods like Dict.get will still behave in the same unsafe way:

Although it may be unsafe if additional assumptions are provided by the user like in FFIs the compiler should produce output that is at least as safe as possible, assuming that the given types are true. If it weren't for that now, it would be treated as a bug and should be fixed.

The problem with the new operation is it could produce false positives with no custom assumptions.

Or it's not a bug, depending on the definitions.

Since we support dict as a first-class type, we should handle it in dict semantics. I think dicts and records may have different behavior on it. Their semantics should be clearer to determine "correct" output from pattern-matching, etc.

So my question is, is it for dicts or records? if it's only for dicts, how it should be defined in the dict semantics?

Regardless, adding a compiler primitive is fine. We can discuss it further when applying it to compiler internals.

[cometkim]

In my mind, any record type is closer to Object.create(null), more explicit, immutable, and fixed layout data. This doesn't share semantics with plain JS.

The dict is just an abstraction of the object-as-dictionary pattern. I'm fine with dict sharing semantics with plain JS objects, destructuring $.constructor.name would be a pretty legit use case. But I'd like to treat record types separately from dicts.

[DZakh]

Agree about dict and record needing to be handled separately. What I suggest is that:

• dict doesn't have any safeguards and should use in for fields check
• For record, we know the fields in compile time, and it should be handled in the safest way. So specifically for record fields check inside of pattern matching we will need to use hasOwnProperty as you suggested.

The PR introduces the in primitive to the compiler and exposes it via %has for dicts only (I'll remove Object.hasOwn fallback). In the next PRs, I should extend the logic to handle records as well and use it for pattern matching for optional fields.