search

ocaml-semver / ocaml-api-watch

Libraries and tools to keep watch on you OCaml lib's API changes
ISC License
21 stars 15 forks source link

Fixed implementation for abstract types #48

Closed Siddhi-agg closed 5 months ago

Siddhi-agg commented 5 months ago

Fixes #44.

I have modified the current signature to reflect the type equality for abstract types with the reference signature. I have also added an equality test case for abstract types and it works as required.

Siddhi-agg commented 5 months ago

It looks great! Out of curiosity, the test was indeed failing before the fix right?

Yes @NathanReb. The last test was failing before I implemented this fix. I also noticed that the last test was only failing when we used two different cmi. When I used the same variable two times, the test passed. So, it was indeed creating two signature types for type t with different identifiers there.

NathanReb commented 5 months ago

When I used the same variable two times, the test passed. So, it was indeed creating two signature types for type t with different identifiers there.

yes that's expected, if you reuse the same signature without parsing it, the type are strictly identical. In practice that will never happen so our tests should always parse signatures separately, even if it means parsing the same string twice but we can't share the parsed cmi value.