search

yallop / ocaml-ctypes

Library for binding to C libraries using pure OCaml
MIT License
363 stars 95 forks source link

Expose issue with multiple anonymous struct. #688

Open Et7f3 opened 2 years ago

Et7f3 commented 2 years ago

I expect this test to pass since it is copy pasted from a previous valid test. The generated code trigger ocaml warning 11 and I think it should be related. I don't know how to fix if you give me pointer I can try.

I get:

File "tests/test-structs/generated_struct_bindings.ml", line 16, characters 4-39:
16 |   | Struct ({ tag = ""; _} as s'), "v2" ->
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning 11 [redundant-case]: this match case is unused.
File "tests/test-structs/generated_struct_bindings.ml", line 19, characters 4-39:
19 |   | Struct ({ tag = ""; _} as s'), "v1" ->
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning 11 [redundant-case]: this match case is unused.
File "tests/test-structs/generated_struct_bindings.ml", line 57, characters 4-55:
57 |   | Struct ({ tag = ""; spec = Incomplete _; _ } as s') ->
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning 11 [redundant-case]: this match case is unused.

I also tried to reproduce with union and got same warning.

File "tests/test-unions/generated_struct_bindings.ml", line 16, characters 4-39:
16 |   | Union ({ utag = ""; _} as s'), "t2" ->
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning 11 [redundant-case]: this match case is unused.
File "tests/test-unions/generated_struct_bindings.ml", line 19, characters 4-39:
19 |   | Union ({ utag = ""; _} as s'), "t1" ->
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning 11 [redundant-case]: this match case is unused.
File "tests/test-unions/generated_struct_bindings.ml", line 33, characters 4-48:
33 |   | Union ({ utag = ""; uspec = None; _ } as s') ->
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning 11 [redundant-case]: this match case is unused.

So this seem to be same issue

I don't see test for anonymous enum and I think we can trigger this defect in some unusual compiler like chibicc. See on https://godbolt.org with:

typedef enum { A } a;
typedef enum { A, B } b;

Since gcc and clang, tcc emit an error this should be a faulty compiler. However for struct this pattern can be seen on skia https://github.com/google/skia/blob/1f193df9b393d50da39570dab77a0bb5d28ec8ef/include/c/sk_types.h#L77-L89

yallop commented 2 years ago

The problem is that, without the tag, there's not always enough information available to retrieve the layout. As the generated code displayed in the error message suggests:

16 |   | Struct ({ tag = ""; _} as s'), "v2" ->

ctypes retrieves the layout information for each field using uses a combination of the tag and the field name. If this combination doesn't uniquely identify the field (as in your example, where multiple untagged structs share a field name), then this scheme won't work.

Et7f3 commented 2 years ago

I tried to add some stamp as workaround. The stamps are generated for each new structure. This break even more test so I think I have missed some place that need to be updated.