-
Notifications
You must be signed in to change notification settings - Fork 1.2k
runtime/array.c: rename make-array functions #13003
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
runtime/array.c: rename make-array functions #13003
Conversation
d72a265 to
ed48566
Compare
OlivierNicole
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me.
ed48566 to
e0bb2ac
Compare
|
Is this good to merge once there is a Changes entry? (I guess before merging, the maintainer doing it should also verify the bytecode blobs of the bootstrap commits?) |
|
These PRs usually generate a fair amount of bike-shedding. I don't know if nobody cares or if it slipped by un-noticed because everyone was busy. Let me send an email to ensure that people with opinions had the opportunity to give some of them. |
Do we need the distinction between |
Historically, |
|
Thanks for the clarification @nojb. |
|
@hhugo I see that js_of_ocaml would be affected by the part of this PR that uses the new primitives in the code produced by the compiler -- of course. Does it make your life easier if this part of the change is delayed to a future release of OCaml? |
Any release after 5.2 would be fine. |
|
To be clear, this PR -- if merged -- would go to trunk, to be released in future 5.3. I'm not sure if you mean that it does not matter when the change to compiler-produced output happens, or if you mean that it is better if the change happens at least one release after the new names are introduced. |
I say after 5.2 is better because I've already released a version of jsoo compatible with 5.2 and I'd prefer to not have to do another one. But in general, I don't really care when such change happen. |
e0bb2ac to
4bb1965
Compare
|
I just pushed a rebase that adds a Changes entry and removes the bootstrap commit: we are only extending the runtime with new functions, which are mentioned in the code generated by the new compiler, it looks like this does not require a bootstrap. |
This PR introduces new, more consistent names for the C functions in the OCaml runtime that create arrays.
Current situation
The implementation of most array primitives has a fairly regular naming structure. Consider
getfor example, we have:and on the C side:
On the other hand, the naming structure of array creation functions is much less regular due to historical baggage. At the OCaml level, we have the following functions to create arrays:
but on the C side we have the following:
There is no obvious naming convention here, in particular
makeis used to mean three different things:float arraywith uninitialized values'a arraywith an initial value'a arrayWhy change?
Improving this situation is a preliminary step towards
uniform_arrayprimitives in the runtime -- see ocaml/RFCs#37 . (One should be able to review new primitives without having to get confused about names at the same time.)Proposition
I propose to keep the existing names for backward-compatibility reasons (removing primitives is a pain), but also to introduce more regular names to use in the future and (after a bootstrap) in the compiler codebase.