-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Updating function and variable docs #1017
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressing comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely good to freshen all of this. Pulling in @wolffg explicitly here as he's been thinking about these high-level design concerns and may have wording or structure suggestions. It looks OK to me, but I also want to make sure this is working well for other readers as well. I have some comments inline too.
|
||
<!-- tocstop --> | ||
|
||
## Overview |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zygoloid to ensure we're doing a decent job of introducing type inference and not missing any implications or connections to other parts of the language that we should establish...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This basically looks good to me. Some minor suggestions inline. I've also pinged on one comment thread that I'd at least like to get an ACK on before this lands.
Co-authored-by: Chandler Carruth <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rephrasing return
statements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me. Some minor wording suggestions.
docs/design/functions.md
Outdated
A function declaration can be called, and the function definition may be | ||
provided later in code; for example, the `api` file of a library will often | ||
contain a function's declaration while the `impl` file contains the definition. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems a bit repetitive with things in the previous paragraph.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
Co-authored-by: Richard Smith <[email protected]>
Co-authored-by: Richard Smith <[email protected]>
Co-authored-by: Richard Smith <[email protected]>
- For example, `fn Sleep(seconds: i64);` is similar to | ||
`fn Sleep(seconds: i64) -> ();`. | ||
- `()` is similar to a `void` return type in C++. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps state that switching between fn F(...)
and fn F(...) -> ()
is a compatible change for all callers? (though the return statements in the function's definition would have to be updated)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest proposing this separately. There are two reasons I could see this not being true:
- It could be decided that
var x: auto = F()
is either required or disallowed in some cases. - Constraints could be allowed to detect the difference.
In any case, I think this is a non-trivial decision and may be more appropriate for a proposal or its own change (this doc edit has already taken longer than I expected, I'd rather not push it out another couple weeks).
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 <[email protected]> Co-authored-by: Richard Smith <[email protected]>
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). Feel free to comment on structure, I was trying to decide how best to approach this.