Skip to content

Conversation

@MisterDA
Copy link
Contributor

@MisterDA MisterDA commented Apr 4, 2025

bindings.c:55:3: warning: use of GNU old-style field designator extension [-Wgnu-designator]
  identifier : "tree handling",
  ^~~~~~~~~~~~
  .identifier =

Security

  • Change has no security implications (otherwise, ping the security team)

    bindings.c:55:3: warning: use of GNU old-style field designator extension [-Wgnu-designator]
      identifier : "tree handling",
      ^~~~~~~~~~~~
      .identifier =
#endif
#ifdef custom_fixed_length_default
.fixed_length = custom_fixed_length_default,
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do the new fields under ifdefs come from? I expect this code to be copied from the original tree-sitter bindings for Reason. The project I found is oni2 and doesn't have the ifdefs: https://github.com/onivim/oni2/blob/161a92c2226728b37cd9a6f554b7397f4beb3d5e/src/reason-tree-sitter/bindings.c#L45-L61

Can you please include an authoritative source URL at the beginning of this file?

Copy link
Member

@mjambon mjambon Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also https://github.com/onivim/reason-tree-sitter/blob/master/src/bindings.c but the project was archived (and it also doesn't have the ifdefs).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They were introduced in OCaml 5.0, see caml/custom.h. Using the #ifdef allows the code to remain compatible with OCaml 4 (one could also use the OCAML_VERSION macro, but I find this pattern rather elegant). Some compilers in some settings complain if some fields are left uninitialized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants