Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate implementation and interface in different (odoc) compilation units #1067

Merged
merged 8 commits into from
Feb 21, 2024

Conversation

panglesd
Copy link
Collaborator

This PR separate the compilation pipeline for interface and implementation. This is a backward incompatible change, but only for rendering source code. It introduces a new command, compile-src, which compiles to a new kind of odoc file: implementation files, named src-<name>.odoc(l).

A good way to see the changes is to look at the diff in the source rendering tests, but here is a list of the changes:

  • The compile command do not take the --source-name and --source-parent-file commands.
  • The new compile-src command take --source-path and --source-parent-file command instead. It can only take a <name>.cmt file as input, and creates a value of type Odoc_model.Lang.Implementation.t which is stored in src-<name>.odoc. Basically, such type is a record containing an identifier, the source information extracted from the typedtree, the implementation shape and the uid_to_id map.
  • When looking for a shape, we look in the src-....odoc files in the path.
  • When html-generating, the --source and --source-root are restricted to implementation odocl files.

A notable change is that --source-parent-file and --source-path is required for implementation files, which forces to have them even when we only want to count occurrences, not render them.

Another change is that --source and --source-root are not anymore specific to the html rendering process. The reason is that with the current organization of the code, only the renderer can depend on backend-specific flags, not the transformation model -> document. Of course it is possible to change that, but that require further modifications which I'm not sure we want.

The reason for this change is the following:

  • It separates two independent pipeline that were merged into one,
  • It avoids having a single .odocl generate files in very different output directories, which can be a problem for some build systems.

@panglesd
Copy link
Collaborator Author

Note: the driver needs to be rewritten to use the new pipeline...

@panglesd
Copy link
Collaborator Author

panglesd commented Jan 8, 2024

This is ready for review.

One notable change that can be discussed: the --count-occurrences (which was used for collecting occurrences information without the will to render source code) command has disappeared.

Instead, we just compile-src the implementation, give it to odoc count-occurrences, but do not call odoc html-generate on it.
However, this means that we still have to give a source parent, even when we do not intend to render it. I would make the source parent optional, and forbid the html-generateion of implementation without source parent. What do you think?

panglesd added a commit to panglesd/odoc that referenced this pull request Jan 11, 2024
Hidden modules were not rendered, since calling html-generate on them was used
to generate the associated source code.

However, this breaks dune rules in some cases (see issue ocaml#1013), as dune always
expect an output to the command.

This fix is related to the separation of implementation and interface
pipelines (see ocaml#1067).

Signed-off-by: Paul-Elliot <[email protected]>
@Julow Julow force-pushed the separate-intf-impl branch from 5cafdf3 to 830c4ed Compare January 18, 2024 16:42
Copy link
Collaborator

@Julow Julow left a comment

Choose a reason for hiding this comment

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

I did a full review and I think this is ready to merge!

Introduce a new command: `odoc compile-src`, which creates a new kind of odoc
file: implementation files.

Implementation files are used to render source code, as well as to collect
occurrences.

Signed-off-by: Paul-Elliot <[email protected]>
- Rename `--source-parent-file` into `parent`
- Check that source argument is empty in case of rendering source tree
- Removed now useless `syntax` argument from `extra_documents`
- Check `"src-"`prefix when loading units for counting occurrences to avoid
  loading for nothing
- fixed some typos and made some small refactoring

Signed-off-by: Paul-Elliot <[email protected]>
Julow and others added 2 commits February 16, 2024 12:22
This field now hold one location instead of two. The name is now more
explicit.
@panglesd
Copy link
Collaborator Author

I rebased and marked the commands as experimental, as they are likely to change in the future (likely before we release)!

@jonludlam jonludlam merged commit ff4c335 into ocaml:master Feb 21, 2024
10 checks passed
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.

3 participants