Function return type inference #826

jonmeow · 2021-09-13T22:28:40Z

Allow auto as a return type for functions. Only support functions with one return statement for now (open question).

This explicitly suggests removing the executable semantics fn name(args) => expression syntax, and is motivated by reconciling executable semantics with approved Carbon state. example This aspect is a decision that may be affected by lambda syntax, but we might also choose to keep lambda syntax and function syntax separate -- I don't think there's enough benefit to providing the alternate function syntax right now.

proposals/p0826.md

Co-authored-by: Geoff Romer <gromer@google.com>

chandlerc

Generally makes sense. Would like to specifically know if @zygoloid has anything we should ensure to avoid here and whether this seems reasonable to go in at this stage.

chandlerc · 2021-09-14T17:55:45Z

proposals/p0826.md

+For example, this is valid because `CallAdd` can see the `Add` definition:
+
+```
+fn Add(x: i32, y: i32) -> auto;
+
+fn Add(x: i32, y: i32) -> auto { return x + y; }


I don't think we should even allow this. While the caller is fine, we can't type check the first declaration. I'd suggest requiring a definition to accompany any declaration with auto.

I think disallowing this makes sense. A forward declaration involving auto is usually not useful -- you can't actually call it, so it doesn't break any cycles, and you can't use it to move the implementation out of an api file. Perhaps it's useful to allow you to declare a member function inside a class and then define it outside, though I'm not sure how valuable that is. If we're deducing a return type, I'd expect the function body to be small.

Updated proposal and added alternative corresponding.

zygoloid · 2021-09-20T23:58:50Z

proposals/p0826.md

+For example, this is valid because `CallAdd` can see the `Add` definition:
+
+```
+fn Add(x: i32, y: i32) -> auto;
+
+fn Add(x: i32, y: i32) -> auto { return x + y; }


I think disallowing this makes sense. A forward declaration involving auto is usually not useful -- you can't actually call it, so it doesn't break any cycles, and you can't use it to move the implementation out of an api file. Perhaps it's useful to allow you to declare a member function inside a class and then define it outside, though I'm not sure how valuable that is. If we're deducing a return type, I'd expect the function body to be small.

zygoloid · 2021-09-21T00:02:31Z

proposals/p0826.md

+
+## Proposal
+
+Support automatic type inference in Carbon with `auto`, as in:


FWIW, I think this is our first allowance for the use of auto anywhere. I don't think that's a problem in and of itself, but it may be worth saying that auto will be a keyword.

Added notes to the background, and a very brief note here. (TBH, I had thought #339 included it, but obviously not -- maybe it's worth a proposal for a starting point)

jonmeow

Addressing comments

jonmeow · 2021-09-21T17:01:32Z

proposals/p0826.md

+
+## Proposal
+
+Support automatic type inference in Carbon with `auto`, as in:


Added notes to the background, and a very brief note here. (TBH, I had thought #339 included it, but obviously not -- maybe it's worth a proposal for a starting point)

jonmeow · 2021-09-21T17:08:43Z

proposals/p0826.md

+For example, this is valid because `CallAdd` can see the `Add` definition:
+
+```
+fn Add(x: i32, y: i32) -> auto;
+
+fn Add(x: i32, y: i32) -> auto { return x + y; }


Updated proposal and added alternative corresponding.

zygoloid · 2021-09-21T22:50:25Z

Generally makes sense. Would like to specifically know if @zygoloid has anything we should ensure to avoid here and whether this seems reasonable to go in at this stage.

I think this proposal has bounded the scope enough to avoid most problems here: we require exactly one return statement, so there's no need to consider how to merge multiple return types, and we're not allowing auto as a component of a more complex type so there's no need to specify how that works.

I think there are two more issues from C++ that we should consider, though I think at least one of them is out of scope for now:

What happens if an attempt is made to call the function
i. before itsreturn statement,
ii. within its return statement, or
iii. after its return statement but before the end of its definition?
I think (ii) is easy: we basically must reject, because the return type is defined in terms of itself. I think we should reject in cases (i) and (iii) too, even though we could in principle support (iii) and the subset of (i) where the expression in the return statement doesn't depend on the return type. This would diverge from C++, which permits recursive calls after the (first) return statement (ie, C++ permits (iii) but not (i) nor (ii)).
What happens if the return value is const qualified (or more generally is of some facet type)? In C++, we initially said that return type deduction for lambdas just copied the type of the return expression, as we do here, and this preserved const qualifiers on the return type. Then with generalized return type deduction we switched to using the auto rules, which are defined in terms of template argument deduction and discard const qualifiers, resulting in some subtle behavior changes. I think this is probably out of scope for now, given that we haven't decided on exactly how to model const yet and in particular I don't think we have reason to believe that making a copy would drop constness in Carbon like it does in C++, but this seems like something we should keep in mind.

In terms of what to do with the above in this document, I think adding something saying we don't permit recursive calls in functions with a deduced return type at least for now (and that this intentionally diverges from C++) would be sufficient.

chandlerc · 2021-09-22T10:09:02Z

Generally makes sense. Would like to specifically know if @zygoloid has anything we should ensure to avoid here and whether this seems reasonable to go in at this stage.

I think this proposal has bounded the scope enough to avoid most problems here: we require exactly one return statement, so there's no need to consider how to merge multiple return types, and we're not allowing auto as a component of a more complex type so there's no need to specify how that works.

I think there are two more issues from C++ that we should consider, though I think at least one of them is out of scope for now:

What happens if an attempt is made to call the function
i. before itsreturn statement,
ii. within its return statement, or
iii. after its return statement but before the end of its definition?
I think (ii) is easy: we basically must reject, because the return type is defined in terms of itself. I think we should reject in cases (i) and (iii) too, even though we could in principle support (iii) and the subset of (i) where the expression in the return statement doesn't depend on the return type. This would diverge from C++, which permits recursive calls after the (first) return statement (ie, C++ permits (iii) but not (i) nor (ii)).

Strongly agree with just rejecting all recursion here. Seems much simpler and more obvious.

What happens if the return value is const qualified (or more generally is of some facet type)? In C++, we initially said that return type deduction for lambdas just copied the type of the return expression, as we do here, and this preserved const qualifiers on the return type. Then with generalized return type deduction we switched to using the auto rules, which are defined in terms of template argument deduction and discard const qualifiers, resulting in some subtle behavior changes. I think this is probably out of scope for now, given that we haven't decided on exactly how to model const yet and in particular I don't think we have reason to believe that making a copy would drop constness in Carbon like it does in C++, but this seems like something we should keep in mind.

Yeah, I agree this is a bit out of scope for now. I'd revisit this (and any other auto we add) when we are getting much more precise with rules around deduction for templates.

jonmeow · 2021-09-22T21:03:50Z

https://github.com/carbon-language/carbon-lang/pull/826/files/be3f11ee0fb54e3bf336a64ef4f10c8967de3ba6..aa305e36838ef0f62fa2a8b985a17073f6995231 should address both points about direct recursion and const

chandlerc

Some minor wording suggestions below to just make things read a bit easier for me. Generally, LGTM. Feel free to merge with these or similar. Also happy to look if the wording suggestions aren't making sense or are changing anything.

proposals/p0826.md

chandlerc · 2021-09-23T09:00:25Z

proposals/p0826.md

+However, this would be invalid because `CallAdd` can only see the `Add`
+declaration (even though the definition may be in the same file), and it would
+need the definition to determine the return type:


Just trying to make "this" a bit less ambiguous:

Suggested change

However, this would be invalid because `CallAdd` can only see the `Add`

declaration (even though the definition may be in the same file), and it would

need the definition to determine the return type:

However, the example below would be invalid because `CallAdd` can only see the `Add`

declaration (even though the definition may be in the same file), and it would

need the definition to determine the return type:

FYI re: below, more specific words are preferred: https://developers.google.com/style/word-list#letter-b

I'll probably do something like "following example"

Co-authored-by: Chandler Carruth <chandlerc@gmail.com>

…ow/carbon-lang into proposal-fn-returning-auto

This implements #826, I think covering everything important there. Regarding ReturnTypeContext, I broke that out because it started feeling like a significant number of args to be passing around, and I think this makes the association inside type checking clearer. Co-authored-by: Geoff Romer <gromer@google.com>

I was doing this for #851 initially, but I think #438 and #826 hadn't made it in (possibly intentionally due to #851? I don't recall). Co-authored-by: Chandler Carruth <chandlerc@gmail.com> Co-authored-by: Richard Smith <richard@metafoo.co.uk>

Allow `auto` as a return type for functions. Only support functions with one `return` statement for now (open question). This explicitly suggests removing the executable semantics `fn name(args) => expression` syntax, and is motivated by reconciling executable semantics with approved Carbon state. [example](https://github.com/carbon-language/carbon-lang/blob/3d1716f6c692a840d8b4b513ddfb8119432a5150/executable_semantics/testdata/fun_named_params.carbon) This aspect is a decision that may be affected by lambda syntax, but we might also choose to keep lambda syntax and function syntax separate -- I don't think there's enough benefit to providing the alternate function syntax right now. Co-authored-by: Geoff Romer <gromer@google.com> Co-authored-by: Chandler Carruth <chandlerc@gmail.com>

This implements #826, I think covering everything important there. Regarding ReturnTypeContext, I broke that out because it started feeling like a significant number of args to be passing around, and I think this makes the association inside type checking clearer. Co-authored-by: Geoff Romer <gromer@google.com>

I was doing this for #851 initially, but I think #438 and #826 hadn't made it in (possibly intentionally due to #851? I don't recall). Co-authored-by: Chandler Carruth <chandlerc@gmail.com> Co-authored-by: Richard Smith <richard@metafoo.co.uk>

jonmeow added the proposal A proposal label Sep 13, 2021

jonmeow requested a review from a team September 13, 2021 22:28

google-cla bot added the cla: yes PR meets CLA requirements according to bot. label Sep 13, 2021

jonmeow added 3 commits September 13, 2021 22:28

Filling out template with PR 826

8236766

Working on proposal

febb4bc

merge

854deac

jonmeow changed the title ~~fn returning auto~~ Function return type inference Sep 14, 2021

jonmeow marked this pull request as ready for review September 14, 2021 17:51

github-actions bot added the proposal rfc Proposal with request-for-comment sent out label Sep 14, 2021

geoffromer reviewed Sep 14, 2021

View reviewed changes

proposals/p0826.md Outdated Show resolved Hide resolved

jonmeow and others added 2 commits September 14, 2021 15:05

Update proposals/p0826.md

19e4491

Co-authored-by: Geoff Romer <gromer@google.com>

Remove brackets

b2b0053

chandlerc reviewed Sep 16, 2021

View reviewed changes

zygoloid reviewed Sep 21, 2021

View reviewed changes

jonmeow added 3 commits September 21, 2021 16:31

Merge branch 'trunk' into proposal-fn-returning-auto

c6c785b

merge

664d20d

Addressing comments

c274a6f

jonmeow commented Sep 21, 2021

View reviewed changes

jonmeow requested review from chandlerc, zygoloid and geoffromer September 21, 2021 17:11

jonmeow added 2 commits September 21, 2021 17:12

Clarify why no separate declaration

e7f8c19

more words

0307649

jonmeow added 3 commits September 22, 2021 20:32

merge

be3f11e

Reject recursion, note const

511d75e

Fix format

aa305e3

chandlerc approved these changes Sep 23, 2021

View reviewed changes

github-actions bot added proposal accepted Decision made, proposal accepted and removed proposal rfc Proposal with request-for-comment sent out labels Sep 23, 2021

jonmeow and others added 7 commits September 23, 2021 09:11

Update proposals/p0826.md

b56176d

Co-authored-by: Chandler Carruth <chandlerc@gmail.com>

Update proposals/p0826.md

c79c372

Co-authored-by: Chandler Carruth <chandlerc@gmail.com>

Merge branch 'trunk' into proposal-fn-returning-auto

fcce7d9

Rephrase and prettier

6e0c41d

Merge branch 'proposal-fn-returning-auto' of https://github.com/jonme…

8b4b032

…ow/carbon-lang into proposal-fn-returning-auto

prettier

82d5bc7

rephrase

18e7d6b

jonmeow merged commit 0eb78c7 into carbon-language:trunk Sep 23, 2021

jonmeow deleted the proposal-fn-returning-auto branch September 23, 2021 17:10

jonmeow mentioned this pull request Sep 23, 2021

Implement auto return types, removing => returns #850

Merged

chandlerc mentioned this pull request Jan 5, 2022

How much complexity should we invest in deduced/inferred return types? #1008

Closed

jonmeow mentioned this pull request Jan 12, 2022

Updating function and variable docs #1017

Merged

jonmeow mentioned this pull request Feb 23, 2022

Implement the current set of rules for precedence partial ordering #1096

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Function return type inference #826

Function return type inference #826

jonmeow commented Sep 13, 2021 •

edited

Loading

chandlerc left a comment

chandlerc Sep 14, 2021

zygoloid Sep 20, 2021

jonmeow Sep 21, 2021

zygoloid Sep 20, 2021

zygoloid Sep 21, 2021

jonmeow Sep 21, 2021

jonmeow left a comment

jonmeow Sep 21, 2021

jonmeow Sep 21, 2021

zygoloid commented Sep 21, 2021 •

edited

Loading

chandlerc commented Sep 22, 2021

jonmeow commented Sep 22, 2021

chandlerc left a comment

chandlerc Sep 23, 2021

jonmeow Sep 23, 2021


		## Proposal

		Support automatic type inference in Carbon with `auto`, as in:

Function return type inference #826

Function return type inference #826

Conversation

jonmeow commented Sep 13, 2021 • edited Loading

chandlerc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonmeow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zygoloid commented Sep 21, 2021 • edited Loading

chandlerc commented Sep 22, 2021

jonmeow commented Sep 22, 2021

chandlerc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonmeow commented Sep 13, 2021 •

edited

Loading

zygoloid commented Sep 21, 2021 •

edited

Loading