Constify C constructors, arrays, and flags tables (take 2)

[MisterDA]

Constify more constructors, arrays, and flags tables in C code. Now these tables will go in the readonly segment, where they belong.
This PR accounts for missed opportunities from #12223. I believe I've missed these because I searched with regexes that were too strong (starting with static, using int but ignoring DWORD, …).
I've tentatively modified the changes entry under the 5.2 section.

[Octachron]

This is not a bug fix but merely a second round of optimization, isn't it? If I am not mistaken, the changelog entry belongs to the "Working version" section.

[dra27]

I'm afraid f87f3ed will non-trivially break Jane Street's core-unix; cf. core_unix/src/core_unix_stubs.c#L1331-L1340:

#if OCAML_VERSION_MAJOR < 5
#define caml_unix_getsockopt_aux unix_getsockopt_aux
#define caml_unix_setsockopt_aux unix_setsockopt_aux
#endif

extern value caml_unix_getsockopt_aux(char *name, enum option_type ty,
                                      int level, int option, value v_socket);
extern value caml_unix_setsockopt_aux(char *name, enum option_type ty,
                                      int level, int option, value v_socket,
                                      value v_status);

It's worth an opam-grep / GitHub code search when de-prefixing a symbol (cf. #10926)

[MisterDA]

This is not a bug fix but merely a second round of optimization, isn't it? If I am not mistaken, the changelog entry belongs to the "Working version" section.

Yes, the changes are independent from the previous PR. I'll move to the working version.

I'm afraid f87f3ed will non-trivially break Jane Street's core-unix;

Nice catch! Thanks.
Should I keep the change adding the const to the name parameter, or does that entail too much breakage for a minor gain?

It's worth an opam-grep / GitHub code search when de-prefixing a symbol

Thanks, I'll keep that in mind! I couldn't remember on which machine I had downloaded the whole opam archive… the GitHub code search is definitely handy.

[dra27]

Should I keep the change adding the const to the name parameter, or does that entail too much breakage for a minor gain?

I think that's a good change - it is an internal detail, so we shouldn't necessarily feel that we can't improve things because that might break external users (after all, I renamed the functions in 5.0!). The issue before was that there was no clear workaround if those functions became static other than copying the code.

What I would suggest doing if this goes in is then opening parallel PRs to fix those packages pre-emptively (as in #10926, which of course usefully lists the packages which were affected by the renaming of those functions already.)

[MisterDA]

I have found two packages impacted by this change, and submitted pull-requests. One has already been merged.

• Constify C constructors and flags tables ocsigen/lwt#1007
• Constify C constructors and flags tables janestreet/core_unix#10

Is there anything that could block this PR?

[shindere]

Antonin Décimo (2024/05/28 03:49 -0700):

Is there anything that could block this PR?

Should we wait for janestreet/core_unix#10 to be merged, maybe?

[MisterDA]

Antonin Décimo (2024/05/28 03:49 -0700):
Is there anything that could block this PR?
Should we wait for janestreet/core_unix#10 to be merged, maybe?

Seems backwards to have to wait for downstream projects to merge these kind of PRs, acting on faith that this one will eventually be merged…

[shindere]

I ended up reviewign this and I believe that this PR is an improvement
over the current state and that, even in the unlikely case where it
would break something, it's better to just go ahead and improve from there
than to delay this even more.

The last commit is not strictly speaking related to the PR, I think,
but at least it is in its own commit and it does not look worth a
PR in itself.

As a consequence, I have just approved and will now merge.