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

Design overview update part 5: Names #1347

Merged
merged 14 commits into from
Jul 6, 2022
Merged

Conversation

josh11b
Copy link
Contributor

@josh11b josh11b commented Jun 28, 2022

This follows #1274 , #1325 , #1328 , and #1336 . It fills in the "Names" section.

@josh11b josh11b requested review from a team as code owners June 28, 2022 23:54
@josh11b josh11b requested a review from zygoloid June 28, 2022 23:54
josh11b added a commit to josh11b/carbon-lang that referenced this pull request Jun 30, 2022
Copy link
Contributor

@zygoloid zygoloid left a comment

Choose a reason for hiding this comment

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

This looks good to me. Some suggestions but I don't think anything really major.

Comment on lines 568 to 569
there should be exactly one definition for the name, but there can be additional
forward declarations that introduce the name before it is defined. Forward
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we say that there can be at most one forward declaration? Given the other constraints we've put on forward declarations, it doesn't seem like there's any utility in having more than one forward declaration. (The #875 proposal mentioned this but didn't actually make a decision on it.)

The one exception I can think of is impls, where a declaration inside a match_first is meaningful even if there's another declaration elsewhere, but I think we can meaningfully distinguish forward declarations from match_first declarations given that match_first declarations have additional semantics. (friend declarations might also behave like match_first, depending on what rules we want for them.)

Perhaps the rule could be that there can be at most one forward declaration, any number of declarations inside match_first blocks, and at most one defintiion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm fine either way. I agree there is not much value beyond the first, other than the match_first case. We did say that a friend declaration does not declare the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I switched to say at most one here.

docs/design/README.md Outdated Show resolved Hide resolved
docs/design/README.md Outdated Show resolved Hide resolved
docs/design/README.md Outdated Show resolved Hide resolved
docs/design/README.md Outdated Show resolved Hide resolved
docs/design/README.md Outdated Show resolved Hide resolved
docs/design/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

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

Really nice. Some comments inline. Happy to look again w/ updates, but don't block on me if @zygoloid (or anyone else) LGTMs.

docs/design/README.md Outdated Show resolved Hide resolved
docs/design/README.md Outdated Show resolved Hide resolved
docs/design/README.md Outdated Show resolved Hide resolved
docs/design/README.md Outdated Show resolved Hide resolved
Comment on lines 1991 to 1992
this is as if there was a special "prelude" package that was imported
automatically into every `api` file that declared these names.
Copy link
Contributor

@zygoloid zygoloid Jul 6, 2022

Choose a reason for hiding this comment

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

Suggested change
this is as if there was a special "prelude" package that was imported
automatically into every `api` file that declared these names.
this is as if there was a special "prelude" package which defines
these types that is imported automatically into every `api` file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to go with Chandler's rewrite. We can fix forward if it is still lacking.

docs/design/README.md Outdated Show resolved Hide resolved
@chandlerc
Copy link
Contributor

To be clear -- I'm in favor of landing with whichever works best for now. We can always follow-up to better word stuff. (This goes for several of the points, just want to be explicit here, and not just directly in chats...)

@josh11b josh11b merged commit 5535f5e into carbon-language:trunk Jul 6, 2022
@josh11b josh11b deleted the names branch July 6, 2022 23:57
josh11b added a commit that referenced this pull request Jul 8, 2022
This follows #1274 , #1325 , #1328 , #1336 , and #1347 . This has miscellaneous changes to the design overview without a particular focus.

Also adds some missing keywords to our list of keywords.

Co-authored-by: Jon Ross-Perkins <[email protected]>
josh11b added a commit that referenced this pull request Jul 15, 2022
This follows #1274 , #1325 , #1328 , #1336 , #1347 , and #1368 . This fills in details about how values work, value categories, parameter passing, unformed state, and so on.

Co-authored-by: Geoff Romer <[email protected]>
Co-authored-by: Richard Smith <[email protected]>
Co-authored-by: Chandler Carruth <[email protected]>
@chandlerc chandlerc added the documentation An issue or proposed change to our documentation label Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation An issue or proposed change to our documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants