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

Supports mode annotation on function body #3327

Merged
merged 6 commits into from
Dec 12, 2024
Merged

Conversation

riaqn
Copy link
Contributor

@riaqn riaqn commented Nov 29, 2024

This PR adds the support for the following syntax:

let foo a b @ modes = ..
let foo a b : ty @@ modes = ..
fun a b @ modes -> ..
fun a b : ty @@ modes -> ..

Please review by commits.

Please do not merge until I finish the internal migration for the new parsetree.

@riaqn riaqn marked this pull request as ready for review November 29, 2024 14:42
Copy link

github-actions bot commented Nov 29, 2024

Parser Change Checklist

This PR modifies the parser. Please check that the following tests are updated:

  • parsetree/source_jane_street.ml

This test should have examples of every new bit of syntax you are adding. Feel free to just check the box if your PR does not actually change the syntax (because it is refactoring the parser, say).

@riaqn riaqn added typing lexer/parser Changes to the lexer and parser modes Work on modes. There's some overlap with the `multicore` label, but not strictly so. labels Nov 29, 2024
Copy link
Contributor

@tdelvecchio-jsc tdelvecchio-jsc left a comment

Choose a reason for hiding this comment

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

Liam to do further review since I realized I don't have context on typing/

printer/printast_with_mappings.ml Show resolved Hide resolved
parsing/printast.ml Show resolved Hide resolved
@riaqn riaqn requested a review from liam923 December 3, 2024 10:22
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.

(I did not review the changes in parser.mly under the premise that @tdelvecchio-jsc already reviewed it.)

I'd like to see a test that shows that this allows a stronger interface to be inferred for a ml-only module.

testsuite/tests/typing-modes/modes.ml Show resolved Hide resolved
typing/untypeast.ml Outdated Show resolved Hide resolved
typing/typecore.ml Outdated Show resolved Hide resolved
typing/untypeast.ml Outdated Show resolved Hide resolved
@riaqn riaqn requested a review from liam923 December 10, 2024 12:05
@riaqn riaqn force-pushed the modes-on-function-body branch from a3c6677 to 8fd10b9 Compare December 11, 2024 09:54
@tdelvecchio-jsc tdelvecchio-jsc self-requested a review December 12, 2024 16:49
@riaqn riaqn merged commit 1274f35 into main Dec 12, 2024
21 checks passed
@riaqn riaqn deleted the modes-on-function-body branch December 12, 2024 16:52
@riaqn riaqn mentioned this pull request Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lexer/parser Changes to the lexer and parser modes Work on modes. There's some overlap with the `multicore` label, but not strictly so. typing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants