Skip to content

Conversation

@voodoos
Copy link
Collaborator

@voodoos voodoos commented Nov 6, 2025

@liam923 not what you expected, but at least the use of the cache is clearer now. I don't know why I opted for the imperative stamped hashtbl in the initial version. This version relies on sharing and integrates much more naturally with the typer cache.

If the next step is to have "lazy" indexing, I think the most sensible approch would be to add a pipeline phase after typing.

@voodoos voodoos added the no changelog Turn off CI changelog-check label Nov 6, 2025
Copy link
Contributor

@liam923 liam923 left a comment

Choose a reason for hiding this comment

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

Thanks, this change definitely seems like an improvement and makes the cache a lot easier to reason about.

@liam923
Copy link
Contributor

liam923 commented Nov 6, 2025

If the next step is to have "lazy" indexing, I think the most sensible approch would be to add a pipeline phase after typing.

Maybe this discussion belongs elsewhere, but could you elaborate on this? It seems to me that making part_index in item be a index Lazy.t and then having get_index force it would be sufficient.

@voodoos
Copy link
Collaborator Author

voodoos commented Nov 6, 2025

Thanks for the review.

It seems to me that making part_index in item be a index Lazy.t and then having get_index force it would be sufficient.

That sounds reasonable too. I will think about it !

@voodoos voodoos merged commit f708466 into ocaml:main Nov 6, 2025
12 of 15 checks passed
@liam923 liam923 mentioned this pull request Nov 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog Turn off CI changelog-check

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants