Skip to content

Conversation

@Julow
Copy link
Collaborator

@Julow Julow commented Apr 15, 2021

Previously, @canonical tags were allowed to be attached to module, module type and type declarations and in the top-comment of compilation units.

This handles @canonical tags in the top-comment of signatures and structures, for example:

module M : sig
  (** @canonical Lib.M *)

  ...
end

$ compile test__x.mli test.ml
File "test.ml", line 15, characters 6-24:
Unexpected tag '@canonical' at this location.
$ compile test__x.mli test__y.ml test.ml
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The scope of this test increases and will conflict with #653. Perhaps at this point, we could test every canonical features in one larger test case ?

Copy link
Member

Choose a reason for hiding this comment

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

could do, though I'd still prefer to try to test canonical on it's own rather than in conjunction with hidden, as there is some overlap in how they both work. Obviously we want to test them together as well, just in a different test -- perhaps a 'namespaces' test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alright, I'll remove the "hidden" part of this test to turn it into a general canonical test. The combination with hidden modules is tested in the ocaml_stdlib.t, we might want to have a bigger "wrapped_library" test too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@jonludlam
Copy link
Member

This looks good, just needs a rebase.

Julow added 4 commits May 10, 2021 19:17
Previously, @canonical tags were allowed to be attached to module,
module type and type declarations and in the top-comment of compilation
units.

This handles @canonical tags in the top-comment of signatures and
structures.
@Julow Julow force-pushed the canonical-sig-topc branch from 0da6b56 to 1d7f92d Compare May 10, 2021 17:42
@Julow
Copy link
Collaborator Author

Julow commented May 10, 2021

Just rebased. I kept the "Cleanup canonical tests" commit but I'm not sure it makes sense since the recent changes on master.

@jonludlam jonludlam merged commit b427137 into ocaml:master May 19, 2021
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