Skip to content

Use type annotations from arguments in let rec#12315

Merged
stedolan merged 1 commit intoocaml:trunkfrom
stedolan:type-args-in-let-rec
Jun 21, 2023
Merged

Use type annotations from arguments in let rec#12315
stedolan merged 1 commit intoocaml:trunkfrom
stedolan:type-args-in-let-rec

Conversation

@stedolan
Copy link
Copy Markdown
Contributor

When typing let rec expressions, the compiler 'approximates' the type of each binding, allowing it to use information about each binding during typechecking. For instance, in a let rec of functions, it knows the arity of each definition early, allowing it to produce partial application warnings for mutually recursive functions.

Currently, type annotations on function parameters are ignored during this approximation. This delays typechecking errors, often in a confusing way. This patch makes approximation use these type annotations. As well as improving errors, this also makes type-based disambiguation work in more cases, for instance:

module M = struct type t = A | B end

let rec f () = g A
and g (x : M.t) = f ()

With this patch, the above typechecks, because the compiler knows that g accepts M.t before typing the body of f.

Copy link
Copy Markdown
Member

@gasche gasche left a comment

Choose a reason for hiding this comment

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

The patch looks correct to me, and I agree that it is a reasonable extension of the current behavior.

(Is the absence of a Changes entry intentional?)

Error: This expression has type int but an expression was expected of type
string
|}]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you include a case of optional argument, to make sure that this is handled correctly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@stedolan
Copy link
Copy Markdown
Contributor Author

(Is the absence of a Changes entry intentional?)

Nope, just hadn't written it yet.

@stedolan stedolan force-pushed the type-args-in-let-rec branch from e93a382 to d6f3c6c Compare June 20, 2023 16:19
@Octachron
Copy link
Copy Markdown
Member

I fear that the merge of #12210 broke the new test. Sorry about this collision.

@gasche gasche force-pushed the type-args-in-let-rec branch from d6f3c6c to 155690a Compare June 20, 2023 19:08
@gasche
Copy link
Copy Markdown
Member

gasche commented Jun 20, 2023

I rebased the PR (by force-pushing to the remote branch) to the new testsuite output.

@stedolan
Copy link
Copy Markdown
Contributor Author

Thanks for rebasing!

@stedolan stedolan merged commit 5babf9b into ocaml:trunk Jun 21, 2023
stedolan added a commit to stedolan/ocaml that referenced this pull request Jul 25, 2023
@stedolan stedolan mentioned this pull request Jul 25, 2023
stedolan added a commit to stedolan/ocaml that referenced this pull request Jul 25, 2023
gasche added a commit that referenced this pull request Jul 25, 2023
voodoos added a commit to voodoos/ocaml that referenced this pull request Feb 12, 2025
voodoos added a commit to voodoos/ocaml that referenced this pull request Feb 17, 2025
voodoos added a commit to voodoos/ocaml that referenced this pull request Feb 17, 2025
@voodoos voodoos mentioned this pull request Feb 17, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants