[4.14] C17 fixes (strict prototypes and others) #12577
Conversation
It is always possible to cast a function pointer to another function
pointer type, and cast again to the original type before calling the
pointed function. I pick the base
typedef value (*c_primitive)(void);
to store the functions in the array, and cast them back in the
interpreter according to the arity described by the opcode.
Some references:
- [Function pointers in C][1]
- [Function pointer casts][2]
[1]: https://frama-c.com/2013/08/24/Function-pointers-in-C.html
[2]: https://www.open-std.org/JTC1/SC22/WG14/www/docs/n1256.pdf
The primitives function pointers can be stored as simple void
pointers:
(Comment by Xavier Leroy: ocaml#11763 (comment))
> Note that ISO C forbids casting a function pointer to type void *,
> but we've been doing this for a very long time, and all C compilers
> that I know of are happy with this.
This is documented by C11 Standard J.5.7, Function pointer casts.
The prototypes in `prims.c` and the headers differ, but:
(Comment by Xavier Leroy: ocaml#11763 (comment))
> On the one hand, it's not legal C11 to declare value f(void) a
> function f that is defined in another compilation unit as value
> f(value), say; while I think it is correct to declare it value f(),
> like we currently do. On the other hand, this is no worse than the
> many other "not legal C11" things we already do.
(cherry picked from commit 8db1817)
Use strict prototypes on primitives in bytecomp
The bytecode compiler also extracts the list of primitives to a C
file, similarly as the Makefile. The Makefile was fixed in
8db1817 to account for stricter C
function prototypes, but the bytecomp problems were not discovered
back then.
As the startup code includes `caml/mlvalues.h`, two primitives are
declared twice with conflicting types. We prevent this with
preprocessor tricks before including the header.
The generated C code now also uses the same names as in the Makefile.
(cherry picked from commit e650dd9)
| for i = 0 to Array.length prim - 1 do | ||
| fprintf outchan " %s,\n" prim.(i) | ||
| done; | ||
| fprintf outchan " (primitive) 0 };\n"; | ||
| fprintf outchan " 0 };\n"; |
There was a problem hiding this comment.
Why lose this cast? Also, is there a reason we don't say "NULL"?
There was a problem hiding this comment.
These are backports, the goal is to align on the current trunk code.
To the best of my knowledge, the cast is unnecessary here. IIRC (strict) C only requires casting NULL to the correct pointer type as an argument to a variadic function. The code has always been this way, the version in this file was using casts, but the version in the makefiles wasn't. I chose to remove unnecessary information.
As for using 0 or NULL, they're also equivalent in this context. No strong opinion here.
| fprintf outchan "const char * caml_names_of_builtin_cprim[] = {\n"; | ||
| for i = 0 to Array.length prim - 1 do | ||
| fprintf outchan " \"%s\",\n" prim.(i) | ||
| done; | ||
| fprintf outchan " (char *) 0 };\n" | ||
| fprintf outchan " 0 };\n" |
There was a problem hiding this comment.
Why lose this cast? And why not "NULL"?
There was a problem hiding this comment.
Same arguments as above.
| @@ -1049,12 +1049,10 @@ void read_declarations(void) | |||
| void output_token_type(void) | |||
| { | |||
| bucket * bp; | |||
| int n; | |||
There was a problem hiding this comment.
Are these changes to suppress an unused variable warning?
There was a problem hiding this comment.
Yes, it's activated by default in clang, and fails the build. This code has been rewritten at some point in trunk, so the problem had evaporated on its own.
|
The fun thing is that the transition from |
|
I'm running a round of "CI precheck" with this PR, just to make sure. Will merge after this. |
On 4.14 branch: C17 fixes (strict prototypes and others)
On 4.14 branch: C17 fixes (strict prototypes and others)
On 4.14 branch: C17 fixes (strict prototypes and others)
On 4.14 branch: C17 fixes (strict prototypes and others)
On 4.14 branch: C17 fixes (strict prototypes and others)
On 4.14 branch: C17 fixes (strict prototypes and others)
On 4.14 branch: C17 fixes (strict prototypes and others)
On 4.14 branch: C17 fixes (strict prototypes and others)
Changes backported from trunk to fix strict prototypes, old style definition, unused variables warnings. The warnings are raised by clang (by default in GNU C17 mode), failing the build of the compiler (with the warnings-as-error
-Werrorflag), and headers with non-strict prototypes leak into user code, adding warnings when compiling C stubs or when using the bytecode compiler.I've regrouped the commits by topic, since the issues took multiple PRs to get fixed in trunk, and that code can be slightly different between 4.14 and 5.x ;)