WIP: Drop Caml runtimes and primitives

[cometkim]

Resolves #5779

This PR contains many breaking changes

Completely dropped

• [x] BukleScript compile time contexts
• OCaml utils
  • [ ] parser
  • [ ] lexer
  • [ ] arg parser
  • [ ] digest
  • [ ] formatter
  • [ ] printer
• OCaml structures
  • [ ] Array
  • [ ] List
  • [ ] Hashtbl
  • [ ] Set
  • [ ] Queue
  • [ ] Complex
  • [x] Stream
• [x] OCaml IO
  • [ ] Sys
• OCaml primitives
  • [ ] String
  • [ ] Char
  • [ ] Int32
  • [ ] Int64

TBD list here what changed

[DZakh]

Like it

[cometkim]

Maybe it's better to merge #6979 first, then I'll rebase

[cristianoc]

Maybe it's better to merge #6979 first, then I'll rebase

Definitely. Given the size of this amazing work we can't really review it but we can quickly observe if all the generated code is simply identical.

[cometkim]

Actually, it was a bit improved like https://github.com/rescript-lang/rescript-compiler/pull/6984/commits/03fe31086dccfe98ca91fe66e92983df95bb26b6#diff-ef282bde01036b4b8ce35133e81733df0530c2e4192992862d2ef200c114e5e0

Having dedicated variants rather than just Pccall makes analyzing side effects /specialization much easier.

[cknitt]

I tested against one of our projects and found that the array indexing now does not return option anymore, even if Core is open.

I.e. this compiles:

open RescriptCore

let a = [1, 2, 3]

let x: int = a[0]
let y: option<int> = a->Array.get(0)

[cknitt]

Maybe it's better to merge https://github.com/rescript-lang/rescript-compiler/pull/6979 first, then I'll rebase

Not sure as #6979 is currently blocked by the binary incompatibility issue described in https://github.com/rescript-lang/rescript-compiler/pull/6979/files#r1738735381, so we would need to wait for that to be resolved.

[zth]

Maybe it's better to merge #6979 first, then I'll rebase

Not sure as #6979 is currently blocked by the binary incompatibility issue described in https://github.com/rescript-lang/rescript-compiler/pull/6979/files#r1738735381, so we would need to wait for that to be resolved.

I'm going to see if we can figure out a quick fix ASAP for just this situation so that can get merged, so that and this can move along. Will get back here when I got something.

[zth]

Maybe it's better to merge #6979 first, then I'll rebase

Not sure as #6979 is currently blocked by the binary incompatibility issue described in https://github.com/rescript-lang/rescript-compiler/pull/6979/files#r1738735381, so we would need to wait for that to be resolved.

I'm going to see if we can figure out a quick fix ASAP for just this situation so that can get merged, so that and this can move along. Will get back here when I got something.

Actually seems like the binary compat was already broken here: https://github.com/rescript-lang/rescript-compiler/pull/6943

Me and @cristianoc discussed a bit, and we think it's fine to merge that PR as is, because the editor extension doesn't touch any of these things where we've extended the AST. However, we need to figure out a real solution for this mid to long term.

The real solution is likely to be that the extension ships its own compiler libs vendored that's compatible with <=11.x, and we start shipping the language server runnable via the compiler for v12+ going forward, so that changes made to the AST is fine to make.

However, that's going to be some work, and I don't have time for that right now. The hard "deadline" for this work is when we introduce changes that break the binary compat in ways that matter for the editor extension. So, it'll be fine for now.

[cometkim]

I tested against one of our projects and found that the array indexing now does not return option anymore, even if Core is open.

I.e. this compiles:

open RescriptCore

let a = [1, 2, 3]

let x: int = a[0]
let y: option<int> = a->Array.get(0)

Fixed in https://github.com/rescript-lang/rescript-compiler/pull/6984/commits/613970b2ce1d70a37f3b68059cc9f231f073e55e

This is related to the behavior I mentioned in Discord. It's quite confusing that open MyStd or module Array = ... can break types of primitive syntax.

[cknitt]

I tested again against one of our projects. Array indexing now works as expected.

I ran into some things like mod_float and string_of_int missing and had to patch some dependencies, but once I got it to build, there were no diffs in the output compared to alpha.3! 🎉

So I guess we should keep at least the more widely used functions like mod_float and string_of_int, but mark them as deprecated in v12.

[cometkim]

Added compatibility/deprecations for Pervasives, Array, List, Char, and String.

Additionally, I modified the ninja.js to make runtime dir include only primitives and others dir include only stdlib. Pervasives now belong solely to primitives, and Js should be part of the stdlib. No-more copy-pasting and ordering hacks

The dependency graph is now others -> runtime clearly

[cknitt]

string_of_int and mod_float are still missing.

For the deprecations, I would suggest to only have "Use Core instead", not "Use Core or Js instead", as Js is also legacy.

Hmm, actually, once Core is in the compiler, we should refer to the matching function without mentioning Core at all, e.g., "Use Float.toString instead."

[cometkim]

I didn't make all functions compatible. I removed functions and OCaml primitives that were already deprecated. AFAIK we've been seeing messages like "use Belt.Int.toString instead" for a long time now.

Also note: Those modules should eventually be removed from the runtime; the "compiler primitives" path, because they are standard library, not primitives. Core is the new stdlib, but it already has some different signature than OCaml. Breaking changes will be inevitable if we add -open Core by default.

To be honest, I don't want to be forced to use Core. I want to have my own stdlib and I don't want to be in a situation where I have to depend on it or shadow it. we can avoid most of the problems if we make using Core as optional.

[zth]

Core will not be optional, as in it will always be available and on by default. This is because one of the big goals with Core is to provide a great default, and we don't want to give the impression that it's "Core or these other available things" . Core won't be a separate concept anymore when it's integrated in the compiler, it'll just be a part of the language.

However, for advanced users it'll work exactly like today - you can shadow it easily with your own stdlib if you want, just like you can shadow the OCaml and/or the Js stuff today.

[cometkim]

Core will not be optional, as in it will always be available and on by default. This is because one of the big goals with Core is to provide a great default, and we don't want to give the impression that it's "Core or these other available things" . Core won't be a separate concept anymore when it's integrated in the compiler, it'll just be a part of the language.

However, for advanced users it'll work exactly like today - you can shadow it easily with your own stdlib if you want, just like you can shadow the OCaml and/or the Js stuff today.

ok I don't argue with that goal.

Anyway, the role of primitive is clear, so we have to be careful not to mix it up with the API surface of Core again.

Primitive is basically "used by the compiler" and should be referenced as an external binding as needed, not as a module, except a few edge cases.

If we open Core as the default, we have to deal with compatibility issues anyway. The most noticible one is that the return type of array[0] syntax changes from 'a to option<'a>. This is why I tried to change the behavior to a Primitive_array.get instead of Array.get earlier.

[zth]

@cometkim IMO you don't need to spend time on getting the error messages right here, we'll have to adjust them anyway as Core is brought in.

Yeah, that was a somewhat calculated breaking change. The positive side is that anyone who's moved to Core already (which I hope is the majority) will already have dealt with that.

Anyway, again, absolutely fantastic job on this PR. A pretty insane feat changing this amount of code and causing no major breakages!

[cknitt]

@cometkim Skimmed through the rest of the changes. Looks good to me! 👍

Could you just bring back the "cargo clean" in the Makefile in some form or other a discussed above? Then good to go from my point of view.

[cometkim]

Could you just bring back the "cargo clean" in the Makefile in some form or other a discussed above? Then good to go from my point of view.

Added clean-rewatch, and rebased

[zth]

Merge! Merge! Merge!

😁

[cknitt]

Merged! 🎉