search

links-lang / links

Links: Linking Theory to Practice for the Web
http://www.links-lang.org
Other
332 stars 42 forks source link

Better workaround for issue #1187 (warning 40 due to @@deriving sexp) #1188

Closed frank-emrich closed 1 year ago

frank-emrich commented 1 year ago

This PR implements a better workaround for #1187.

The previous workaround was to force using an older version of the ppx_sexp_conv, which did not exhibit issue #1187. As such a version constraint may inconvenience users, this PR removes the version constraint and touches the problematic uses of @@deriving sexp directly.

Unforutnately, locally suppressing the warning code 40 triggered by the code generated for @@deriving sexp is somewhat tricky, as there is no AST node in the original source file to attach an ocaml.warning attribute to.

This PR circumvents this problem by wrapping the affected type definitions in an anonymous module, disabling warning 40 within that whole module, and then immediately includeing said module.

This limits the scope in which we actually disable warning 40 to exactly where it is needed.

frank-emrich commented 1 year ago

I'm planning to find a better solution for #1187 eventually (e.g., looking into the generated code in detail and reporting a bug with the ppx_sexp_conv authors). In the meantine, this updated workaround is friendlier for end users and should be part of the upcoming Links release (as it avoids forcing users to install an outdated version of ppx_sexp_conv).